* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
@ 2013-09-29 19:15 ` jakub at gcc dot gnu.org
2013-09-29 20:18 ` mpolacek at gcc dot gnu.org
` (16 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2013-09-29 19:15 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |4.8.2
Summary|[4.9 Regression] Wrong code |[4.8/4.9 Regression] Wrong
|due to s390x machine reorg |code due to s390x machine
|pass |reorg pass
--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Reproduced with 4.8 branch too.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
2013-09-29 19:15 ` [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass jakub at gcc dot gnu.org
@ 2013-09-29 20:18 ` mpolacek at gcc dot gnu.org
2013-09-30 11:02 ` jakub at gcc dot gnu.org
` (15 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: mpolacek at gcc dot gnu.org @ 2013-09-29 20:18 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
Marek Polacek <mpolacek at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Last reconfirmed| |2013-09-29
CC| |mpolacek at gcc dot gnu.org
Ever confirmed|0 |1
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
2013-09-29 19:15 ` [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass jakub at gcc dot gnu.org
2013-09-29 20:18 ` mpolacek at gcc dot gnu.org
@ 2013-09-30 11:02 ` jakub at gcc dot gnu.org
2013-09-30 11:15 ` jakub at gcc dot gnu.org
` (14 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2013-09-30 11:02 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I'd say the bug is in s390_chunkify_start:
if (GET_CODE (pat) == PARALLEL && XVECLEN (pat, 0) > 2)
pat = XVECEXP (pat, 0, 0);
Dunno what exactly the > 2 condition has been added for, JUMP_INSNs with
PARALLEL with XVECLEN (pat, 0) == 2 are:
1) casesi_jump
2) *cmp_and_br_signed_<mode>, *cmp_and_br_unsigned_<mode>,
*icmp_and_br_signed_<mode>, *icmp_and_br_unsigned_<mode>
3) *ccraw_to_int
I can understand why you wouldn't want to handle 1) in there, and supposedly
3) should be split before machine reorg or worst case at the start of it. But
2), this case, it looks wrong not to do anything here.
Note that similar condition is in s390_split_branches, no idea what do you want
to do there. But perhaps TARGET_Z10 implies TARGET_ZARCH and thus
s390_split_branches would be never called.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (2 preceding siblings ...)
2013-09-30 11:02 ` jakub at gcc dot gnu.org
@ 2013-09-30 11:15 ` jakub at gcc dot gnu.org
2013-09-30 11:17 ` jakub at gcc dot gnu.org
` (13 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2013-09-30 11:15 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 30934
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30934&action=edit
Possible fix
Possible fix.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (3 preceding siblings ...)
2013-09-30 11:15 ` jakub at gcc dot gnu.org
@ 2013-09-30 11:17 ` jakub at gcc dot gnu.org
2013-09-30 11:58 ` krebbel at gcc dot gnu.org
` (12 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2013-09-30 11:17 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 30935
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30935&action=edit
Alternate fix
Another possibility. If the check is there really just to prevent handling
tablejumps, I wonder why we can't do the tablejump handling first and only if
it didn't do anything, handle all other jumps.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (4 preceding siblings ...)
2013-09-30 11:17 ` jakub at gcc dot gnu.org
@ 2013-09-30 11:58 ` krebbel at gcc dot gnu.org
2013-09-30 12:00 ` krebbel at gcc dot gnu.org
` (11 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: krebbel at gcc dot gnu.org @ 2013-09-30 11:58 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #5 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
Thanks for tracking this down!
(In reply to Jakub Jelinek from comment #2)
> I'd say the bug is in s390_chunkify_start:
> if (GET_CODE (pat) == PARALLEL && XVECLEN (pat, 0) > 2)
> pat = XVECEXP (pat, 0, 0);
> Dunno what exactly the > 2 condition has been added for, JUMP_INSNs with
The more complex loop jumps might have more than 2 I think.
> PARALLEL with XVECLEN (pat, 0) == 2 are:
> 1) casesi_jump
> 2) *cmp_and_br_signed_<mode>, *cmp_and_br_unsigned_<mode>,
> *icmp_and_br_signed_<mode>, *icmp_and_br_unsigned_<mode>
> 3) *ccraw_to_int
> I can understand why you wouldn't want to handle 1) in there, and supposedly
> 3) should be split before machine reorg or worst case at the start of it.
> But 2), this case, it looks wrong not to do anything here.
>
> Note that similar condition is in s390_split_branches, no idea what do you
> want to do there. But perhaps TARGET_Z10 implies TARGET_ZARCH and thus
> s390_split_branches would be never called.
Exactly.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (5 preceding siblings ...)
2013-09-30 11:58 ` krebbel at gcc dot gnu.org
@ 2013-09-30 12:00 ` krebbel at gcc dot gnu.org
2013-09-30 13:11 ` krebbel at gcc dot gnu.org
` (10 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: krebbel at gcc dot gnu.org @ 2013-09-30 12:00 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #6 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #4)
> Created attachment 30935 [details]
> Alternate fix
>
> Another possibility. If the check is there really just to prevent handling
> tablejumps, I wonder why we can't do the tablejump handling first and only
> if it didn't do anything, handle all other jumps.
I agree with that patch. Since tablejump_p is available it is easier to read
the other way around.
Thanks!
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (6 preceding siblings ...)
2013-09-30 12:00 ` krebbel at gcc dot gnu.org
@ 2013-09-30 13:11 ` krebbel at gcc dot gnu.org
2013-09-30 13:17 ` jakub at gcc dot gnu.org
` (9 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: krebbel at gcc dot gnu.org @ 2013-09-30 13:11 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #7 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
Created attachment 30938
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30938&action=edit
Alternate fix - v2
Since tablejump_p is checking for JUMP_P anyway we could move the check even
outside the jump insn check.
I've also changed s390_split_branches to exit early for table jumps.
Untested so far.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (7 preceding siblings ...)
2013-09-30 13:11 ` krebbel at gcc dot gnu.org
@ 2013-09-30 13:17 ` jakub at gcc dot gnu.org
2013-09-30 13:33 ` jakub at gcc dot gnu.org
` (8 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2013-09-30 13:17 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 30939
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30939&action=edit
gcc48-pr58574.patch
Yeah, perhaps. I'm attaching 4.8 version of the patch, which wasn't using
tablejump_p at all. The patch also contains a testcase (which should be added
to the 4.9 version of the patch too).
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (8 preceding siblings ...)
2013-09-30 13:17 ` jakub at gcc dot gnu.org
@ 2013-09-30 13:33 ` jakub at gcc dot gnu.org
2013-10-01 6:58 ` jakub at gcc dot gnu.org
` (7 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2013-09-30 13:33 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Note that I can't test this easily on the trunk, the partition I have access to
has just 2 CPUs and 1GB of RAM, bootstrap/regtest would be very slow if it
worked at all, but am testing the 4.8 version of the patch (via a build system
scratch build).
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (9 preceding siblings ...)
2013-09-30 13:33 ` jakub at gcc dot gnu.org
@ 2013-10-01 6:58 ` jakub at gcc dot gnu.org
2013-10-01 12:28 ` krebbel at gcc dot gnu.org
` (6 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2013-10-01 6:58 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The 4.8 version of the patch bootstrapped/regtested fine on both s390x-linux
and s390-linux, both configured with --with-arch=z10 --with-tune=zEC12.
Andreas, could you please bootstrap/regtest some 4.9 version of the patch?
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (10 preceding siblings ...)
2013-10-01 6:58 ` jakub at gcc dot gnu.org
@ 2013-10-01 12:28 ` krebbel at gcc dot gnu.org
2013-10-01 12:38 ` jakub at gcc dot gnu.org
` (5 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: krebbel at gcc dot gnu.org @ 2013-10-01 12:28 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
Andreas Krebbel <krebbel at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #30938|0 |1
is obsolete| |
--- Comment #11 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
Created attachment 30943
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30943&action=edit
Upstream patch
I've tested this patch on s390 and s390x with --with-arch=z10 --with-tune=zEC12
and the default options on GCC head.
No regressions.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (11 preceding siblings ...)
2013-10-01 12:28 ` krebbel at gcc dot gnu.org
@ 2013-10-01 12:38 ` jakub at gcc dot gnu.org
2013-10-01 13:33 ` krebbel at gcc dot gnu.org
` (4 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2013-10-01 12:38 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Thanks, are you going to post it to gcc-patches and commit then? Can I post
the 4.8 patch there afterwards, or do you prefer some different alternative for
4.8?
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (12 preceding siblings ...)
2013-10-01 12:38 ` jakub at gcc dot gnu.org
@ 2013-10-01 13:33 ` krebbel at gcc dot gnu.org
2013-10-01 13:46 ` krebbel at gcc dot gnu.org
` (3 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: krebbel at gcc dot gnu.org @ 2013-10-01 13:33 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #13 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
Author: krebbel
Date: Tue Oct 1 13:33:02 2013
New Revision: 203060
URL: http://gcc.gnu.org/viewcvs?rev=203060&root=gcc&view=rev
Log:
2013-10-01 Jakub Jelinek <jakub@redhat.com>
Andreas Krebbel <Andreas.Krebbel@de.ibm.com>
PR target/58574
* config/s390/s390.c (s390_split_branches): Modify check for table
jump insns.
(s390_chunkify_start): Rearrange table jump insn check in order to
deal with compare and branch insns correctly.
2013-10-01 Jakub Jelinek <jakub@redhat.com>
PR target/58574
* gcc.c-torture/execute/pr58574.c: New testcase.
Added:
trunk/gcc/testsuite/gcc.c-torture/execute/pr58574.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/s390/s390.c
trunk/gcc/testsuite/ChangeLog
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (13 preceding siblings ...)
2013-10-01 13:33 ` krebbel at gcc dot gnu.org
@ 2013-10-01 13:46 ` krebbel at gcc dot gnu.org
2013-10-01 13:50 ` jakub at gcc dot gnu.org
` (2 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: krebbel at gcc dot gnu.org @ 2013-10-01 13:46 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #14 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #12)
> Thanks, are you going to post it to gcc-patches and commit then? Can I post
> the 4.8 patch there afterwards, or do you prefer some different alternative
> for 4.8?
Upstream version committed.
Feel free to commit your version to 4.8 branch. Thanks!
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.8/4.9 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (14 preceding siblings ...)
2013-10-01 13:46 ` krebbel at gcc dot gnu.org
@ 2013-10-01 13:50 ` jakub at gcc dot gnu.org
2013-10-01 14:00 ` [Bug target/58574] [4.7 " jakub at gcc dot gnu.org
2014-06-12 13:30 ` rguenth at gcc dot gnu.org
17 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2013-10-01 13:50 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Tue Oct 1 13:50:30 2013
New Revision: 203062
URL: http://gcc.gnu.org/viewcvs?rev=203062&root=gcc&view=rev
Log:
PR target/58574
* config/s390/s390.c (s390_chunkify_start): Handle tablejump_p first,
continue when done, for other jumps look through PARALLEL
unconditionally.
* gcc.c-torture/execute/pr58574.c: New test.
Added:
branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/pr58574.c
Modified:
branches/gcc-4_8-branch/gcc/ChangeLog
branches/gcc-4_8-branch/gcc/config/s390/s390.c
branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.7 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (15 preceding siblings ...)
2013-10-01 13:50 ` jakub at gcc dot gnu.org
@ 2013-10-01 14:00 ` jakub at gcc dot gnu.org
2014-06-12 13:30 ` rguenth at gcc dot gnu.org
17 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2013-10-01 14:00 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|4.8.2 |4.7.4
Summary|[4.8/4.9 Regression] Wrong |[4.7 Regression] Wrong code
|code due to s390x machine |due to s390x machine reorg
|reorg pass |pass
--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 4.8.2+ so far. From code inspection it could affect 4.7.x too, can't
it? No time to test it there though.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug target/58574] [4.7 Regression] Wrong code due to s390x machine reorg pass
[not found] <bug-58574-4@http.gcc.gnu.org/bugzilla/>
` (16 preceding siblings ...)
2013-10-01 14:00 ` [Bug target/58574] [4.7 " jakub at gcc dot gnu.org
@ 2014-06-12 13:30 ` rguenth at gcc dot gnu.org
17 siblings, 0 replies; 18+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-06-12 13:30 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58574
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Known to work| |4.8.2
Resolution|--- |FIXED
Target Milestone|4.7.4 |4.8.2
Known to fail| |4.7.4
--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed for 4.8.2.
^ permalink raw reply [flat|nested] 18+ messages in thread