[Bug c/80640] New: Missing memory side effect

2017-05-05 Thread nico...@morey-chaisemartin.com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80640

Bug ID: 80640
   Summary: Missing memory side effect
   Product: gcc
   Version: 6.3.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: nico...@morey-chaisemartin.com
  Target Milestone: ---

I found what seems to be a GCC issue while building/running the OpenMPI 2.1.0
testsuite using GCC 6.3.1 on i586 (Suse Tumbleweed)

I attached the tarball with the preprocessed source file and the minimum
libraries.
It compiles with:

 gcc  -o opal_fifo opal_fifo.c  libopen-pal.so.20 -ldl libsupport.a -lrt -lm
-lutil -pthread -Wl,-rpath -Wl,$(pwd) 


When compiling witrh O0, or O1 the test works. It stalls with O2 and O3.

From what I could find, the program get stuck here:

while (&fifo->opal_fifo_ghost == item->opal_list_next) {
opal_atomic_rmb ();
}


opal_atomic_rmb is defined like this

static inline void opal_atomic_rmb(void)
{
__atomic_thread_fence (2);
}


The assembly for this loop looks like this:

=> 0x080495a5 <+325>:   cmp%edi,%eax
   0x080495a7 <+327>:   je 0x80495a5 


I'm a little rusty on x86 assembly but for me it means GCC cached the value to
compare and never reloads them from memory.

I would expect the __atomic_thread_fence atomic to have some kind of memory
side effect that means these values should be reloaded.

Adding a simple

__asm__ __volatile__("": : :"memory");

to opal_atomic_rmb seems to fix the issue.

[Bug c/80640] Missing memory side effect

2017-05-05 Thread nico...@morey-chaisemartin.com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80640

--- Comment #2 from Nicolas Morey-Chaisemartin  
---
Created attachment 41325
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=41325&action=edit
Test case

Previous tarball was too big. I stripped all debug info from the lib and it
should work now :)

[Bug c/80640] Missing memory side effect

2017-05-05 Thread nico...@morey-chaisemartin.com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80640

--- Comment #4 from Nicolas Morey-Chaisemartin  
---
I agree the volatile shoud fix thing> I'll have to see with the ompi guys to
fix that.

But shouldn't __atomic_thread_fence () have a side effect here and force the
memory to be reloaded ?
If it has no impact, what's the point ?

[Bug c/80640] Missing memory side effect

2017-05-05 Thread nico...@morey-chaisemartin.com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80640

--- Comment #6 from Nicolas Morey-Chaisemartin  
---
Ok. So there's something wrong :)
I'll make a work around for SUSE while waiting for a fix in GCC