0
votes

I have the original code:

min = INT_MAX;
for (i=0;i<N;i++)
  if (A[i]<min) 
    min = A[i];
for (i=0;i<N;i++)
  A[i]=A[i]-min;

I want to get the parallel version of this and I did this:

min = INT_MAX;
#pragma omp parallel private(i){
minl = INT_MAX;
#pragma omp for
for (i=0;i<N;i++)
  if (A[i]<minl)
    minl=A[i];
#pragma omp critical{
if (minl<min)
  min=minl;
}
#pragma omp for
for (i=0;i<N;i++)
  A[i]=A[i]-min;
}

Is the parallel code right? I was wondering if it is necessary to write #pragma omp barrier before #pragma omp critical so that I make sure that all the minimums are calculated before calculating the global minimum.

1
No, it's not right. Although critical may be used, #pragma omp for reduction(min: ....) was added in OpenMP 4.0 for this purpose. Almost enough reason there to use an up to date implementation.tim18
And what would I need to change if I want to do it without reduction?Unknown
You appear to vacillate on whether you wish to set up an inner simd reduction and outer parallel threaded reduction (a good method, if the problem is large enough). The single thread simd reduction is enough maybe up to N < 80000 or so. Unfortunately, omp for simd reduction doesn't work the same on all compilers (again, not at all on Microsoft).tim18
Full working code would help; for example you don't tell anything about data types vs. identity of your compiler.tim18

1 Answers

2
votes

The code is correct. There is no necessity to add a #pragma omp barrier because there is no need for all min_l to be computed when one thread enters the critical section. There is also an implicit barrier at the end of the loop region.

Further, you do not necessarily need to explicitly declare the loop iteration variable i private.

You can improve the code by using a reduction instead of your manual merging of minl:

#pragma omp for reduction(min:min)
for (i=0;i<N;i++)
  if (A[i]<min)
    min=A[i];

Note: The min operator for reduction is available since OpenMP 3.1.