0

I have to snippets, one of which is standard, and one other which uses OpenMP. This is the first:

#include <iostream>

int main(){
double a = 0;
for(int i =0; i<= 100000; i++){
    a+=1;
}

std::cout << a << std::endl;
return 0;
}

This code gives me: 100001, so it is OK; there is no problem.

The problem is that, for the second snippet:

#include <iostream>

int main(){

double a = 0;
#pragma omp parallel for
for(int i = 0; i<=100000; i++){

    a+=1;

}
std::cout << a << std::endl;
return 0;
}

The result of this code is: 55246, and I don't know why I get this result instead of 100001.

TylerH
  • 20,799
  • 66
  • 75
  • 101
  • 1
    Looks like it is because you print before parallel iterations complete. Do you get same result each time? – t.m. Mar 23 '17 at 15:27
  • No i get a different result each time i don't understand why –  Mar 23 '17 at 15:29
  • 1
    change `double a = 0;` to `std::atomic a = 0;` `a` is being updated by more than 1 thread without any synchronisation, this is Undefined Behaviour. – Richard Critten Mar 23 '17 at 15:30
  • i tried but i get 'atomic' is not a member of 'std' –  Mar 23 '17 at 15:32
  • Have you included `` ? It's been in the standard since C++11 http://en.cppreference.com/w/cpp/atomic/atomic – Richard Critten Mar 23 '17 at 15:34
  • Well, I am no expert on openmp, but looks like you are doing each iteration in parallel, that means your program won't wait all parallel operations to complete before executing next line, where you print the variable. Also as @RichardCritten pointed out, you need to handle parallel operations changing a common variable. – t.m. Mar 23 '17 at 15:36
  • How can i do to include ? Thank you –  Mar 23 '17 at 15:58
  • 2
    The same way you included `` This is such a basic question I feel you should be reading some C++ introductory books. See here: http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list – Richard Critten Mar 23 '17 at 16:00
  • Have you tried setting `a` to be `atomic` as previously mentioned and setting a barrier to wait for all threads to finish? `#pragma omp barrier` `std::cout << a << std::endl;`? – Ingenioushax Mar 23 '17 at 16:27
  • You could also decorate the compound assignment operator with the following OpenMP directive to make the assignment atomic. `#pragma omp atomic` – Ingenioushax Mar 23 '17 at 16:43
  • 1
    Possible duplicate of [OpenMP - Why does the number of comparisons decrease?](http://stackoverflow.com/questions/42395568/openmp-why-does-the-number-of-comparisons-decrease) – Zulan Mar 23 '17 at 17:41
  • @RichardCritten note that `std::atomic` and OpenMP are unfortunately [not officially interoperable](http://stackoverflow.com/a/41316118/620382). While it might work in practice, I wouldn't outright recommend mixing C++11 multithreading concepts and OpenMP. – Zulan Mar 23 '17 at 17:44
  • 4
    What you need here is `#pragma omp parallel for reduction (+:a)` to solve the race condition on `a`. All suggestions based on `atomic` are either wrong if refering to `std::atomic` or terribly suboptimal if refering to `omp atomic`. – Gilles Mar 24 '17 at 04:53
  • 1
    To me, Gilles' answer is the most appropriate in this case. I'd suggest him to make this comment as an answer. – Harald Mar 24 '17 at 11:04
  • As Gilles pointed out, undeclared reduction is the reason for wrong result. However, parallel reduction without simd reduction can't be considered optimum (not that optimization is relevant for the quoted snippet). – tim18 Mar 24 '17 at 12:36

0 Answers0