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 )
#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)
follow-up: 2 comment:1 by , 17 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 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
comment:3 by , 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++)
follow-up: 5 comment:4 by , 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.
comment:5 by , 17 years ago
Replying to hyd_danmar:
I have seen a few variations on this:
#pragma omp parallel forI 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:7 by , 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 , 14 years ago
| Cc: | added |
|---|---|
| Priority: | → Normal |
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?