Opened 17 years ago

Last modified 14 years ago

#663 new enhancement

race condition when using openmp for parallel loops

Reported by: ettlmartin Owned by: noone
Priority: Normal Milestone:
Component: New check Version:
Keywords: opnmp for loop Cc: amai

Description (last modified by hyd_danmar)

#include <omp.h>
int main()
{
	int a[50],i;

	a[0] = 1;
	#pragma omp parallel for 
	for(i=1; i<50; i++) 
	{
		a[i] = i + a[i-1]; 
	}
}


In the code a above, there is a race condition.
There are dependencies between loop iterations.
Sections of loops split between threads will not necessarily execute in order
Out of order loop execution will result in undefined behavior

In order to compile this sample you have to link against -lgomp e.g:

g++ -o test test.cpp -lgomp

Best regards

Martin

Change History (8)

comment:1 by hyd_danmar, 17 years ago

Description: modified (diff)

I'd like to be able to detect race conditions.

It's not so easy to detect this problem. It require changes in the preprocessor and/or tokenizer.

Could you elaborate about what is happening? Is the for loop executed by separate threads? Are these threads using the same variables?

in reply to:  1 comment:2 by ettlmartin, 17 years ago

Replying to hyd_danmar:

I'd like to be able to detect race conditions.

It's not so easy to detect this problem. It require changes in the preprocessor and/or tokenizer.

Could you elaborate about what is happening?

The for loop is executed by serveral threads. The number of threads can also determined, but here the number is undetermined. This means, a default number of threads executing the for loop.

Is the for loop executed by separate threads?

Yes, the for loop intervall is splitted up. Eeach thread gets its own intervall. Suppose there are two threads, the the first thread executes for(i=0;i<25;i++) and the second the rest of the intervall.

Are these threads using the same variables?

Yes

http://openmp.org/wp/
http://de.wikipedia.org/wiki/OpenMP

comment:3 by hyd_danmar, 17 years ago

One issue is that a[i] depends on a[i-1]. That was the issue this ticket is about isn't it?

If I am not mistaken there is also an issue with "i". If both threads use the same variable "i" they can't loop both from 0 to 25 and from 25 to 50 at the same time.

So what is the correct way to create a for loop? Is it like this?

    #pragma omp parallel for 
    for(int i=0; i<50; i++)

comment:4 by hyd_danmar, 17 years ago

I have seen a few variations on this:

    #pragma omp parallel for 

I have seen the keywords "private" and "shared"

So I think such keywords should also be handled.

in reply to:  4 comment:5 by ettlmartin, 17 years ago

Replying to hyd_danmar:

I have seen a few variations on this:

    #pragma omp parallel for 

I have seen the keywords "private" and "shared"

So I think such keywords should also be handled.

# shared: the data within a parallel region is shared, which means visible and accessible by all threads simultaneously. By default, all variables in the work sharing region are shared except the loop iteration counter.

# private: the data within a parallel region is private to each thread, which means each thread will have a local copy and use it as a temporary variable. A private variable is not initialized and the value is not maintained for use outside the parallel region. By default, the loop iteration counters in the OpenMP loop constructs are private

comment:6 by judas_iscariote, 16 years ago

note that with g++ you must use -fopenmp and NOT -lgomp

comment:7 by tehuser, 15 years ago

Hi All,

Just importing some sample openMP errors, and their description, as discussed on the forum:
http://sourceforge.net/apps/phpbb/cppcheck/viewtopic.php?f=4&t=268

This is from "Common Mistakes in OpenMP and How To Avoid Them A Collection of Best Practices",
http://www.michaelsuess.net/michaelsuess/publications/suess_leopold_common_mistakes_06.pdf

Looking at table 1:

  • 6: Declare loop variable in "#pragma omp parallel for" as "shared"
    //This code is very wrong -- shared(i) should not be written, as 
    //i is the loop iterator.
    int i;
    int a[100];
    #pragma omp parallel for shared(i)
    for(i=0;i<100;i++)
       a[i]=i;

No warnings with gcc (gcc 4.5.0)
$ g++ test.cpp -o test -Wall -Wextra -fopenmp
$

  • 8: Try to change num of threads in parallel region after start of region
        int a[100];
    
        #pragma omp parallel 
        {
        int numThreads=omp_get_num_threads();
        numThreads=numThreads*2; //Lets use more threads for whatever reason
        omp_set_num_threads(numThreads); //Wrong
    
        
        #pragma omp for
        for(int i=0;i<100;i++)
            a[i] = i;
    
        
        } 
    

$ g++ test.cpp -o test -fopenmp -Wall -Wextra
$

Or:

    int a[100];

    int numThreads=omp_get_num_threads();
    numThreads=numThreads*2; //Lets use more threads for whatever reason
      
    #pragma omp parallel for
    for(int i=0;i<100;i++)
    {
       omp_set_num_threads(numThreads); //Wrong
       a[i] = i;

    
    } 
  • 10 : Attempt to change loop variable in "#pragma omp for"
    void dostuff(int *buffer, int size)
    {
         #pragma omp parallel for
         for(int i=0;i<size;i++)
         {
              //I think i am being tricky here, 'cause openmP does not allow break
              // but actually, this is wrong.
              if(buffer[i] < 100)
              {
                 //Set i to beyond buffer, to force for to exit
                 i=size; //Wrong.
              }
         }
    }
    

$ g++ test.cpp -o test -fopenmp -Wall -Wextra
$

# 13: Orphaned construct outside parallel region (this one may be hard to detect... as if the function is called from within a parallel region, then this can be OK)

    void func(int *a,int size)
    {
          //Meant to write "parallel for" but forgot. 
          //Created a looping construct with no parallel region
          #pragma omp for
         for(int i=0;i<size; i++)
              a[i]=i;

    }

comment:8 by amai, 14 years ago

Cc: amai added
Priority: Normal
Note: See TracTickets for help on using tickets.