public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
@ 2015-02-03 23:08 schwab@linux-m68k.org
2015-02-09 0:07 ` [Bug target/64930] [5 " pinskia at gcc dot gnu.org
` (12 more replies)
0 siblings, 13 replies; 14+ messages in thread
From: schwab@linux-m68k.org @ 2015-02-03 23:08 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
Bug ID: 64930
Summary: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c
scan-assembler-times isync 12
Product: gcc
Version: 5.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: schwab@linux-m68k.org
Target: powerpc64-*-*
$ gcc/xgcc -Bgcc/ ../gcc/testsuite/gcc.target/powerpc/atomic-p7.c -mcpu=power7
-O2 -S -m64 -o atomic-p7.s
$ grep -c isync atomic-p7.s
16
$ gcc/xgcc -Bgcc/ ../gcc/testsuite/gcc.target/powerpc/atomic-p8.c -mcpu=power8
-O2 -S -m64 -o atomic-p8.s
$ grep -c isync atomic-p8.s
25
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug target/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
@ 2015-02-09 0:07 ` pinskia at gcc dot gnu.org
2015-02-09 14:28 ` rguenth at gcc dot gnu.org
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: pinskia at gcc dot gnu.org @ 2015-02-09 0:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |5.0
Summary|[5.0 regression] FAIL: |[5 regression] FAIL:
|gcc.target/powerpc/atomic-p |gcc.target/powerpc/atomic-p
|7.c scan-assembler-times |7.c scan-assembler-times
|isync 12 |isync 12
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug target/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
2015-02-09 0:07 ` [Bug target/64930] [5 " pinskia at gcc dot gnu.org
@ 2015-02-09 14:28 ` rguenth at gcc dot gnu.org
2015-02-11 17:32 ` jakub at gcc dot gnu.org
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2015-02-09 14:28 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |wrong-code
Priority|P3 |P1
--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
smells like wrong-code?
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug target/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
2015-02-09 0:07 ` [Bug target/64930] [5 " pinskia at gcc dot gnu.org
2015-02-09 14:28 ` rguenth at gcc dot gnu.org
@ 2015-02-11 17:32 ` jakub at gcc dot gnu.org
2015-02-11 17:44 ` [Bug testsuite/64930] " jakub at gcc dot gnu.org
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2015-02-11 17:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Last reconfirmed| |2015-02-11
CC| |amacleod at redhat dot com,
| |amodra at gcc dot gnu.org,
| |jakub at gcc dot gnu.org,
| |torvald at gcc dot gnu.org
Ever confirmed|0 |1
--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Bisected to r219601 aka PR59448. I believe that change was intentional, thus
perhaps the testcases should be adjusted for that?
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug testsuite/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
` (2 preceding siblings ...)
2015-02-11 17:32 ` jakub at gcc dot gnu.org
@ 2015-02-11 17:44 ` jakub at gcc dot gnu.org
2015-02-11 18:14 ` jgreenhalgh at gcc dot gnu.org
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2015-02-11 17:44 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org
--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 34731
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34731&action=edit
gcc5-pr64930.patch
Thus I'm proposing this untested patch.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug testsuite/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
` (3 preceding siblings ...)
2015-02-11 17:44 ` [Bug testsuite/64930] " jakub at gcc dot gnu.org
@ 2015-02-11 18:14 ` jgreenhalgh at gcc dot gnu.org
2015-02-11 20:20 ` torvald at gcc dot gnu.org
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: jgreenhalgh at gcc dot gnu.org @ 2015-02-11 18:14 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
James Greenhalgh <jgreenhalgh at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jgreenhalgh at gcc dot gnu.org
--- Comment #4 from James Greenhalgh <jgreenhalgh at gcc dot gnu.org> ---
This is just the PowerPC equivalent of the FAILs we started seeing on
AArch64/ARM for the same reason (consume promoted to acquire).
Over on the AArch64/ARM side, Mike Stump mentioned that he would prefer the
now-failing search for consume semantics to be marked XFAIL, rather than for us
to update the testcase to reflect the promotion.
https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02517.html
I'm not sure if that changes the approach you want to take for PowerPC,
personally I don't have an opinion either way.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug testsuite/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
` (4 preceding siblings ...)
2015-02-11 18:14 ` jgreenhalgh at gcc dot gnu.org
@ 2015-02-11 20:20 ` torvald at gcc dot gnu.org
2015-02-11 22:49 ` jakub at gcc dot gnu.org
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: torvald at gcc dot gnu.org @ 2015-02-11 20:20 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
--- Comment #5 from torvald at gcc dot gnu.org ---
(In reply to Jakub Jelinek from comment #3)
> Created attachment 34731 [details]
> gcc5-pr64930.patch
>
> Thus I'm proposing this untested patch.
I think expecting the consume-to-acquire promotion is the right thing to do (I
haven't tested the patch though). The consume specification in the standard
has issues, and based on the discussions in ISO C++ SG1, it seems unlikely that
it can or will be fixed. Thus, I believe the promotion will stay with us
forever.
Therefore, using an XFAIL doesn't seem right to me.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug testsuite/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
` (5 preceding siblings ...)
2015-02-11 20:20 ` torvald at gcc dot gnu.org
@ 2015-02-11 22:49 ` jakub at gcc dot gnu.org
2015-02-11 22:57 ` amodra at gmail dot com
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2015-02-11 22:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Plus xfailing the isync count test would mean we don't test isync count at all,
even for all the other constructs in the testcase.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug testsuite/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
` (6 preceding siblings ...)
2015-02-11 22:49 ` jakub at gcc dot gnu.org
@ 2015-02-11 22:57 ` amodra at gmail dot com
2015-02-11 23:02 ` jakub at gcc dot gnu.org
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: amodra at gmail dot com @ 2015-02-11 22:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
Alan Modra <amodra at gmail dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |amodra at gmail dot com
--- Comment #7 from Alan Modra <amodra at gmail dot com> ---
My opinion, FWIW, is that adjusting the expected count of isync is plain wrong.
The pr59448 "fix" effectively means that gcc doesn't support atomic consume, so
I think the testcase should remain failing (maybe add a comment saying why) or
#if 0 out all the consume tests.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug testsuite/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
` (7 preceding siblings ...)
2015-02-11 22:57 ` amodra at gmail dot com
@ 2015-02-11 23:02 ` jakub at gcc dot gnu.org
2015-02-12 0:31 ` amodra at gmail dot com
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2015-02-11 23:02 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Well, it does support it, but as an alias to acquire.
Keeping the test FAILing is just wrong, and if the promotion of consume to
acquire is going to be permanent or at least for a couple of years, xfail
doesn't make sense either.
We can say in comment that if consume is not promoted to acquire, the count
would be different and how big of course, so if that is ever changed, the test
can be adjusted again.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug testsuite/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
` (8 preceding siblings ...)
2015-02-11 23:02 ` jakub at gcc dot gnu.org
@ 2015-02-12 0:31 ` amodra at gmail dot com
2015-02-12 13:15 ` jakub at gcc dot gnu.org
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: amodra at gmail dot com @ 2015-02-12 0:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
--- Comment #9 from Alan Modra <amodra at gmail dot com> ---
My point was that if you write a testcase that specifically tests for consume
and get acquire code then that is a fail. The code generated is using a bigger
hammer than necessary. Imagine for a moment if gcc promoted everything to
seq_cst. That would also be "supporting" the atomics, but for powerpc we'd be
quite unhappy and would want to know about such a change with a testsuite
failure..
I recognize that the testcase doesn't properly test consume semantics, and that
teaching gcc to track dependencies is not simple.
Huh, I just checked and it appears powerpc emits unnecessary isyncs for
atomic_load consumes prior to the pr59448 change, probably in recognition that
dependency tracking isn't done. So I withdraw my objection to adjusting the
isync counts.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug testsuite/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
` (10 preceding siblings ...)
2015-02-12 13:15 ` jakub at gcc dot gnu.org
@ 2015-02-12 13:15 ` jakub at gcc dot gnu.org
2015-02-12 16:03 ` torvald at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2015-02-12 13:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Thu Feb 12 13:14:47 2015
New Revision: 220646
URL: https://gcc.gnu.org/viewcvs?rev=220646&root=gcc&view=rev
Log:
PR testsuite/64930
* gcc.target/powerpc/atomic-p7.c: Adjust expected count of isync
instructions for 2015-01-14 get_memmodel changes.
* gcc.target/powerpc/atomic-p8.c: Likewise.
Modified:
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.target/powerpc/atomic-p7.c
trunk/gcc/testsuite/gcc.target/powerpc/atomic-p8.c
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug testsuite/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
` (9 preceding siblings ...)
2015-02-12 0:31 ` amodra at gmail dot com
@ 2015-02-12 13:15 ` jakub at gcc dot gnu.org
2015-02-12 13:15 ` jakub at gcc dot gnu.org
2015-02-12 16:03 ` torvald at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2015-02-12 13:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug testsuite/64930] [5 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
` (11 preceding siblings ...)
2015-02-12 13:15 ` jakub at gcc dot gnu.org
@ 2015-02-12 16:03 ` torvald at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: torvald at gcc dot gnu.org @ 2015-02-12 16:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64930
--- Comment #12 from torvald at gcc dot gnu.org ---
(In reply to Alan Modra from comment #9)
> My point was that if you write a testcase that specifically tests for
> consume and get acquire code then that is a fail. The code generated is
> using a bigger hammer than necessary.
The consensus in ISO C++ SG1 (though we had no straw poll on this particular
thing, so this is my conclusion from the discussions and the opinions voiced by
other compiler vendors) is that implementing C/C++ memory_order_consume means,
in practice, promoting to memory_order_acquire. This is not a GCC-specific
solution or deficiency. It is rather the realization that the standard's
intent can't be implemented in practice without too much costs elsewhere (e.g.,
because of how tracking dependencies properly would require points-to analysis
or conservatively adding barriers in likely many places in the code).
See this paper for more background:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4321.pdf
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-02-12 16:03 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-03 23:08 [Bug target/64930] New: [5.0 regression] FAIL: gcc.target/powerpc/atomic-p7.c scan-assembler-times isync 12 schwab@linux-m68k.org
2015-02-09 0:07 ` [Bug target/64930] [5 " pinskia at gcc dot gnu.org
2015-02-09 14:28 ` rguenth at gcc dot gnu.org
2015-02-11 17:32 ` jakub at gcc dot gnu.org
2015-02-11 17:44 ` [Bug testsuite/64930] " jakub at gcc dot gnu.org
2015-02-11 18:14 ` jgreenhalgh at gcc dot gnu.org
2015-02-11 20:20 ` torvald at gcc dot gnu.org
2015-02-11 22:49 ` jakub at gcc dot gnu.org
2015-02-11 22:57 ` amodra at gmail dot com
2015-02-11 23:02 ` jakub at gcc dot gnu.org
2015-02-12 0:31 ` amodra at gmail dot com
2015-02-12 13:15 ` jakub at gcc dot gnu.org
2015-02-12 13:15 ` jakub at gcc dot gnu.org
2015-02-12 16:03 ` torvald at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).