* [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
2013-11-19 17:51 [Bug libgomp/59194] New: tsan detects race for real variables in an OMP reduction clause Joost.VandeVondele at mat dot ethz.ch
@ 2013-11-19 18:25 ` pinskia at gcc dot gnu.org
2013-11-19 18:46 ` Joost.VandeVondele at mat dot ethz.ch
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: pinskia at gcc dot gnu.org @ 2013-11-19 18:25 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I think this is still a dup of bug 40362.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
2013-11-19 17:51 [Bug libgomp/59194] New: tsan detects race for real variables in an OMP reduction clause Joost.VandeVondele at mat dot ethz.ch
2013-11-19 18:25 ` [Bug libgomp/59194] " pinskia at gcc dot gnu.org
@ 2013-11-19 18:46 ` Joost.VandeVondele at mat dot ethz.ch
2013-11-20 13:43 ` Joost.VandeVondele at mat dot ethz.ch
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Joost.VandeVondele at mat dot ethz.ch @ 2013-11-19 18:46 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194
Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |Joost.VandeVondele at mat dot ethz
| |.ch
--- Comment #2 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(In reply to Andrew Pinski from comment #1)
> I think this is still a dup of bug 40362.
I think the linux-futex case should be fixed by
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561#c38
PR40362 is still relevant for --disable-linux-futex
However, admittedly, I don't know this topic well enough to figure this out
from the code myself.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
2013-11-19 17:51 [Bug libgomp/59194] New: tsan detects race for real variables in an OMP reduction clause Joost.VandeVondele at mat dot ethz.ch
2013-11-19 18:25 ` [Bug libgomp/59194] " pinskia at gcc dot gnu.org
2013-11-19 18:46 ` Joost.VandeVondele at mat dot ethz.ch
@ 2013-11-20 13:43 ` Joost.VandeVondele at mat dot ethz.ch
2013-11-20 13:51 ` jakub at gcc dot gnu.org
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Joost.VandeVondele at mat dot ethz.ch @ 2013-11-20 13:43 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194
--- Comment #3 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
actually it seems more general an issue, the following:
SUBROUTINE S1(m)
REAL :: m
!$OMP ATOMIC
m=m+1.0
END
REAL :: m
m=0.0
!$OMP PARALLEL
CALL S1(m)
!$OMP END PARALLEL
END
flags race for the atomic update of m. Again, does work for integers but not
for reals.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
2013-11-19 17:51 [Bug libgomp/59194] New: tsan detects race for real variables in an OMP reduction clause Joost.VandeVondele at mat dot ethz.ch
` (2 preceding siblings ...)
2013-11-20 13:43 ` Joost.VandeVondele at mat dot ethz.ch
@ 2013-11-20 13:51 ` jakub at gcc dot gnu.org
2013-11-20 17:26 ` Joost.VandeVondele at mat dot ethz.ch
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2013-11-20 13:51 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194
--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Because CPUs obviously don't have floating point atomic instructions, what the
compiler does is just load it as an integer, view convert to floating point,
perform arithmetics, view convert result back to integer, and compare and swap
(if unsuccessful loop). I bet tsan complains because the load is not atomic,
but does it really matter? If we read garbage there, compare and swap will
fail and next time we'll have hopefully correct value already from what compare
and swap said was the previous value.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
2013-11-19 17:51 [Bug libgomp/59194] New: tsan detects race for real variables in an OMP reduction clause Joost.VandeVondele at mat dot ethz.ch
` (3 preceding siblings ...)
2013-11-20 13:51 ` jakub at gcc dot gnu.org
@ 2013-11-20 17:26 ` Joost.VandeVondele at mat dot ethz.ch
2013-11-20 17:54 ` Joost.VandeVondele at mat dot ethz.ch
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Joost.VandeVondele at mat dot ethz.ch @ 2013-11-20 17:26 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194
--- Comment #5 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(In reply to Jakub Jelinek from comment #4)
> I bet tsan complains because the load is
> not atomic, but does it really matter?
I think there are (at least) two possible answers to this.
1) No, it doesn't matter, because we get the right result. This actually is
great, already and your and other gcc-ers effort is much appreciated.
2) But yes, it does. For users, having a tool to help verify/debug an OMP
program is very valuable. tsan is pretty close to doing being exactly that, and
the gomp+tsan combination would be really a plus for gcc, increasing in
importance as OMP (or related variants) becomes more and more common. Such
false positives reduce the value of the gomp+tsan tool significantly.
Therefore, if these issues could be fixed easily without significant downside,
I think this should be done. I'd happy to try patches, and do some tsan+omp
testing.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
2013-11-19 17:51 [Bug libgomp/59194] New: tsan detects race for real variables in an OMP reduction clause Joost.VandeVondele at mat dot ethz.ch
` (4 preceding siblings ...)
2013-11-20 17:26 ` Joost.VandeVondele at mat dot ethz.ch
@ 2013-11-20 17:54 ` Joost.VandeVondele at mat dot ethz.ch
2013-11-20 17:59 ` jakub at gcc dot gnu.org
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Joost.VandeVondele at mat dot ethz.ch @ 2013-11-20 17:54 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194
--- Comment #6 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(In reply to Jakub Jelinek from comment #4)
> I bet tsan complains because the load is
> not atomic, but does it really matter? If we read garbage there, compare
> and swap will fail and next time we'll have hopefully correct value already
> from what compare and swap said was the previous value.
Actually, I believe this might fail. If the garbage read happens to be equal to
what a 3rd thread writes in the meanwhile (admittedly, small change), the
compare and swap will succeed and not fail.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
2013-11-19 17:51 [Bug libgomp/59194] New: tsan detects race for real variables in an OMP reduction clause Joost.VandeVondele at mat dot ethz.ch
` (5 preceding siblings ...)
2013-11-20 17:54 ` Joost.VandeVondele at mat dot ethz.ch
@ 2013-11-20 17:59 ` jakub at gcc dot gnu.org
2013-11-20 18:12 ` Joost.VandeVondele at mat dot ethz.ch
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2013-11-20 17:59 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194
--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
And the problem with that is? Because the arithmetics is based on the value
we've read, it shouldn't be a problem.
That said, during stage3 I'll look at how costly would be to use there
__atomic_load_n with MEMMODEL_RELAXED.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
2013-11-19 17:51 [Bug libgomp/59194] New: tsan detects race for real variables in an OMP reduction clause Joost.VandeVondele at mat dot ethz.ch
` (6 preceding siblings ...)
2013-11-20 17:59 ` jakub at gcc dot gnu.org
@ 2013-11-20 18:12 ` Joost.VandeVondele at mat dot ethz.ch
2013-11-21 12:34 ` dvyukov at google dot com
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Joost.VandeVondele at mat dot ethz.ch @ 2013-11-20 18:12 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194
--- Comment #8 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(In reply to Jakub Jelinek from comment #7)
> And the problem with that is? Because the arithmetics is based on the value
> we've read, it shouldn't be a problem.
Ah...right.
> That said, during stage3 I'll look at how costly would be to use there
> __atomic_load_n with MEMMODEL_RELAXED.
Thanks!
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
2013-11-19 17:51 [Bug libgomp/59194] New: tsan detects race for real variables in an OMP reduction clause Joost.VandeVondele at mat dot ethz.ch
` (7 preceding siblings ...)
2013-11-20 18:12 ` Joost.VandeVondele at mat dot ethz.ch
@ 2013-11-21 12:34 ` dvyukov at google dot com
2014-01-09 16:12 ` Joost.VandeVondele at mat dot ethz.ch
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: dvyukov at google dot com @ 2013-11-21 12:34 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194
Dmitry Vyukov <dvyukov at google dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |dvyukov at google dot com
--- Comment #9 from Dmitry Vyukov <dvyukov at google dot com> ---
(In reply to Jakub Jelinek from comment #4)
> Because CPUs obviously don't have floating point atomic instructions, what
> the compiler does is just load it as an integer, view convert to floating
> point, perform arithmetics, view convert result back to integer, and compare
> and swap (if unsuccessful loop). I bet tsan complains because the load is
> not atomic, but does it really matter? If we read garbage there, compare
> and swap will fail and next time we'll have hopefully correct value already
> from what compare and swap said was the previous value.
Data races do not work this way. It's undefined behavior:
http://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong
The code must use relaxed atomic load. If the code becomes slower due to that
then:
(1) the current generated code is incorrect
or (2) there is a performance bug in gcc handling of atomic operations
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
2013-11-19 17:51 [Bug libgomp/59194] New: tsan detects race for real variables in an OMP reduction clause Joost.VandeVondele at mat dot ethz.ch
` (8 preceding siblings ...)
2013-11-21 12:34 ` dvyukov at google dot com
@ 2014-01-09 16:12 ` Joost.VandeVondele at mat dot ethz.ch
2014-01-13 7:57 ` jakub at gcc dot gnu.org
2014-01-13 10:54 ` Joost.VandeVondele at mat dot ethz.ch
11 siblings, 0 replies; 13+ messages in thread
From: Joost.VandeVondele at mat dot ethz.ch @ 2014-01-09 16:12 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194
--- Comment #10 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(In reply to Jakub Jelinek from comment #7)
> That said, during stage3 I'll look at how costly would be to use there
> __atomic_load_n with MEMMODEL_RELAXED.
any chance this could be looked at in the remaining part of stage3? I can
benchmark the patch on our application if this makes sense.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
2013-11-19 17:51 [Bug libgomp/59194] New: tsan detects race for real variables in an OMP reduction clause Joost.VandeVondele at mat dot ethz.ch
` (9 preceding siblings ...)
2014-01-09 16:12 ` Joost.VandeVondele at mat dot ethz.ch
@ 2014-01-13 7:57 ` jakub at gcc dot gnu.org
2014-01-13 10:54 ` Joost.VandeVondele at mat dot ethz.ch
11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2014-01-13 7:57 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194
--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Mon Jan 13 07:56:40 2014
New Revision: 206572
URL: http://gcc.gnu.org/viewcvs?rev=206572&root=gcc&view=rev
Log:
PR libgomp/59194
* omp-low.c (expand_omp_atomic_pipeline): Expand the initial
load as __atomic_load_N if possible.
Modified:
trunk/gcc/ChangeLog
trunk/gcc/omp-low.c
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
2013-11-19 17:51 [Bug libgomp/59194] New: tsan detects race for real variables in an OMP reduction clause Joost.VandeVondele at mat dot ethz.ch
` (10 preceding siblings ...)
2014-01-13 7:57 ` jakub at gcc dot gnu.org
@ 2014-01-13 10:54 ` Joost.VandeVondele at mat dot ethz.ch
11 siblings, 0 replies; 13+ messages in thread
From: Joost.VandeVondele at mat dot ethz.ch @ 2014-01-13 10:54 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194
--- Comment #12 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(In reply to Jakub Jelinek from comment #11)
> Author: jakub
> Date: Mon Jan 13 07:56:40 2014
> New Revision: 206572
>
> URL: http://gcc.gnu.org/viewcvs?rev=206572&root=gcc&view=rev
> Log:
> PR libgomp/59194
> * omp-low.c (expand_omp_atomic_pipeline): Expand the initial
> load as __atomic_load_N if possible.
>
> Modified:
> trunk/gcc/ChangeLog
> trunk/gcc/omp-low.c
I have verified that this patch fixes the reported issues. From my point of
view, the PR can be closed as fixed. Thanks!
>From gcc-bugs-return-440196-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Jan 13 10:56:25 2014
Return-Path: <gcc-bugs-return-440196-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 22396 invoked by alias); 13 Jan 2014 10:56:25 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 22353 invoked by uid 48); 13 Jan 2014 10:56:21 -0000
From: "jakub at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug libgomp/59194] tsan detects race for real variables in an OMP reduction clause
Date: Mon, 13 Jan 2014 10:56:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: libgomp
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: jakub at gcc dot gnu.org
X-Bugzilla-Status: RESOLVED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_status resolution
Message-ID: <bug-59194-4-IfuD9AcB3p@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-59194-4@http.gcc.gnu.org/bugzilla/>
References: <bug-59194-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-01/txt/msg01338.txt.bz2
Content-length: 427
http://gcc.gnu.org/bugzilla/show_bug.cgi?idY194
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |FIXED
--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed then.
^ permalink raw reply [flat|nested] 13+ messages in thread