[Bug tree-optimization/53436] New: Volatile behaves strange with OpenMP

[Bug tree-optimization/53436] New: Volatile behaves strange with OpenMP

[o.mangold at googlemail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53436

             Bug #: 53436
           Summary: Volatile behaves strange with OpenMP
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: o.mangold@googlemail.com


When I compile the program below (gcc 4.7.0)

> gcc -g -O3 -std=c99 -fopenmp volatile.c

I find, that the volatile is ignored (when optimizing with -O or higher) for
the statement

  while(!x);

>From objdump I see that x is not reread for each loop

> objdump -S a.out
...
      while(!x);
      sleep(1);
      printf("b finished\n");
  4007bf:    bf 9c 08 40 00           mov    $0x40089c,%edi
  4007c4:    e9 37 fe ff ff           jmpq   400600 <puts@plt>
  4007c9:    eb fe                    jmp    4007c9 <main._omp_fn.0+0x29>
  4007cb:    0f 1f 44 00 00           nopl   0x0(%rax,%rax,1)
  volatile bool x=false;
  #pragma omp parallel num_threads(2) shared(x)
  {
    if (omp_get_thread_num()==0)
    {
      sleep(1);
  4007d0:    bf 01 00 00 00           mov    $0x1,%edi
  4007d5:    e8 66 fe ff ff           callq  400640 <sleep@plt>
...

At 4007c9 an unconditional (endless) loop is generated. I don't know if it is
valid to use volatile this way in combination with OpenMP (maybe the standard
doesn't cover it), but I guess that kind of optimization is at least a
dangerous thing to do.

--- volatile.c ---
#include <stdio.h>
#include <unistd.h>
#include <stdbool.h>
#include <omp.h>

int main()
{
  volatile bool x=false;
  #pragma omp parallel num_threads(2)
  {
    if (omp_get_thread_num()==0)
    {
      sleep(1);
      x=true;
      printf("a finished\n");
    }
    else
    {
      while(!x);
      sleep(1);
      printf("b finished\n");
    }
  }
}

[Bug tree-optimization/53436] Volatile behaves strange with OpenMP

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53436

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-21 21:05:39 UTC ---
Author: jakub
Date: Mon May 21 21:05:33 2012
New Revision: 187741

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187741
Log:
    PR tree-optimization/53436
    * omp-low.c (omp_build_component_ref): New function.
    (build_receiver_ref, build_sender_ref, create_task_copyfn): Use it.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/omp-low.c

[Bug tree-optimization/53436] Volatile behaves strange with OpenMP

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53436

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-21 21:06:19 UTC ---
Author: jakub
Date: Mon May 21 21:06:13 2012
New Revision: 187742

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187742
Log:
    PR tree-optimization/53436
    * omp-low.c (omp_build_component_ref): New function.
    (build_receiver_ref, build_sender_ref, create_task_copyfn): Use it.

Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/omp-low.c

[Bug tree-optimization/53436] Volatile behaves strange with OpenMP

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53436

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |jakub at gcc dot gnu.org
         Resolution|                            |FIXED

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-22 07:37:14 UTC ---
The testcase is not valid OpenMP, there is no flush operation in between the
store and reads, that said, there was a bug on the GCC side that I've fixed.

[Bug tree-optimization/53436] Volatile behaves strange with OpenMP

[o.mangold at googlemail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53436

--- Comment #4 from o.mangold at googlemail dot com 2012-05-22 07:45:46 UTC ---
(In reply to comment #3)
> The testcase is not valid OpenMP, there is no flush operation in between the
> store and reads, 

Is that also needed with volatile variables? Would be quite counterintuitive.

> that said, there was a bug on the GCC side that I've fixed.

Great, thanks.

[Bug tree-optimization/53436] Volatile behaves strange with OpenMP

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53436

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-22 07:59:01 UTC ---
(In reply to comment #4)
> (In reply to comment #3)
> > The testcase is not valid OpenMP, there is no flush operation in between the
> > store and reads, 
> 
> Is that also needed with volatile variables? Would be quite counterintuitive.

I believe so.  Volatile only makes sure the compiler emits a store of the
variable to memory, but the CPU can just store it into a cache.  There is
nothing that orders that store compared to other stores/loads.
So, just use the appropriate OpenMP directives for synchronization, rather than
bad attempts at busy waiting using volatile.

[Bug tree-optimization/53436] Volatile behaves strange with OpenMP

[o.mangold at googlemail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53436

--- Comment #6 from o.mangold at googlemail dot com 2012-05-22 08:32:03 UTC ---
Yes, I get, that it's not a good way to do things, as (among other reasons) a
volatile access is no memory fence. So accesses to other locations may not be
ordered. But just for the sake of correctness, accesses should be ordered, if
they all go to volatile variables, no? From the C99-standard, section 5.1.2.3:

> At sequence points, volatile objects are stable in the sense that previous accesses are complete and subsequent accesses have not yet occurred.

This means buffering the reads to the volatile variable over multiple
iterations of the while loop is not allowed, or do I get this wrong?

[Bug tree-optimization/53436] Volatile behaves strange with OpenMP

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53436

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-22 10:01:11 UTC ---
What GCC did was wrong.  But your testcase is clearly invalid as per
OpenMP 3.1, 1.4.1:
"Similarly, if at least one thread reads from a memory unit and at least one
thread writes without synchronization to that same memory unit, including cases
due to atomicity considerations as described above, then a data race occurs. If
a data race occurs then the result of the program is unspecified."

[Bug tree-optimization/53436] Volatile behaves strange with OpenMP

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53436

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot
                   |                            |gnu.org

--- Comment #8 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-05-24 22:31:58 UTC ---
> Yes, I get, that it's not a good way to do things, as (among other reasons) a
> volatile access is no memory fence. So accesses to other locations may not be
> ordered. But just for the sake of correctness, accesses should be ordered, if
> they all go to volatile variables, no? From the C99-standard, section 5.1.2.3:
> 
> > At sequence points, volatile objects are stable in the sense that previous accesses are complete and subsequent accesses have not yet occurred.
> 
> This means buffering the reads to the volatile variable over multiple
> iterations of the while loop is not allowed, or do I get this wrong?

You cannot invoke the C99 standard when there is concurrency in your program,
as the standard doesn't deal with it at all.

[Bug tree-optimization/53436] Volatile behaves strange with OpenMP

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53436

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-05-24 22:34:30 UTC ---
(In reply to comment #8)
> You cannot invoke the C99 standard when there is concurrency in your program,
> as the standard doesn't deal with it at all.

Though C++11/C11 does but we don't have it implemented yet.