Bug 81535 - [8 regression] gcc.target/powerpc/pr79439.c fails starting with r250442
Summary: [8 regression] gcc.target/powerpc/pr79439.c fails starting with r250442
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 8.0
: P1 normal
Target Milestone: 8.0
Assignee: Yuri Gribov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-24 16:23 UTC by seurer
Modified: 2018-02-16 23:41 UTC (History)
4 users (show)

See Also:
Host: powerpc64le-unknown-linux-gnu
Target: powerpc64le-unknown-linux-gnu
Build: powerpc64le-unknown-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2017-07-24 00:00:00


Attachments
Patch to disable test on PowerPC (274 bytes, patch)
2017-07-24 20:47 UTC, Yury Gribov
Details | Diff
Update pattern for Power (341 bytes, patch)
2017-07-25 04:28 UTC, Yury Gribov
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description seurer 2017-07-24 16:23:40 UTC
I noticed this on a powerpc64le power8 system:

spawn /home/seurer/gcc/build/gcc-test/gcc/xgcc -B/home/seurer/gcc/build/gcc-test/gcc/ /home/seurer/gcc/gcc-test/gcc/testsuite/gcc.target/powerpc/pr79439.c -fno-diagnostics-show-caret -fdiagnostics-color=never -O2 -fpic -ffat-lto-objects -S -o pr79439.s
PASS: gcc.target/powerpc/pr79439.c (test for excess errors)
PASS: gcc.target/powerpc/pr79439.c scan-assembler-times \\mbl f\\M 1
PASS: gcc.target/powerpc/pr79439.c scan-assembler-times \\mbl g\\M 1
PASS: gcc.target/powerpc/pr79439.c scan-assembler-times \\mbl rec\\M 1
FAIL: gcc.target/powerpc/pr79439.c scan-assembler-times \\mnop\\M 3

seurer@genoa:~/gcc/build/gcc-test$ diff pr79439.s.r250441 pr79439.s.r250442
46,47c46
< 	bl rec
< 	nop
---
> 	bl rec.localalias.0
54c53,54
< 	.ident	"GCC: (GNU) 8.0.0 20170721 (experimental) [trunk revision 250441]"
---
> 	.set	rec.localalias.0,rec
> 	.ident	"GCC: (GNU) 8.0.0 20170721 (experimental) [trunk revision 250442]"
Comment 1 seurer 2017-07-24 18:43:18 UTC
The failures also occur on powerpc64 BE.


This test that was added in this revision also fails:

spawn /home/seurer/gcc/build/gcc-test/gcc/xgcc -B/home/seurer/gcc/build/gcc-test/gcc/ /home/seurer/gcc/gcc-test/gcc/testsuite/gcc.dg/pr56727-2.c -fno-diagnostics-show-caret -fdiagnostics-color=never -O2 -fPIC -ffat-lto-objects -S -o pr56727-2.s
PASS: gcc.dg/pr56727-2.c (test for excess errors)
FAIL: gcc.dg/pr56727-2.c scan-assembler @(PLT|plt)

seurer@makalu-lp1:~/gcc/build/gcc-test$ /home/seurer/gcc/build/gcc-test/gcc/xgcc -B/home/seurer/gcc/build/gcc-test/gcc/ /home/seurer/gcc/gcc-test/gcc/testsuite/gcc.dg/pr56727-2.c -fno-diagnostics-show-caret -fdiagnostics-color=never -O2 -fPIC -ffat-lto-objects -S -o pr56727-2.s
seurer@makalu-lp1:~/gcc/build/gcc-test$ cat pr56727-2.s 
	.file	"pr56727-2.c"
	.section	".text"
	.align 2
	.p2align 4,,15
	.globl f
	.section	".opd","aw"
	.align 3
f:
	.quad	.L.f,.TOC.@tocbase,0
	.previous
	.type	f, @function
.L.f:
	.p2align 4,,15
.L2:
	b .L2
	.long 0
	.byte 0,0,0,0,0,0,0,0
	.size	f,.-.L.f
	.set	g,f
	.align 2
	.p2align 4,,15
	.globl h
	.section	".opd","aw"
	.align 3
h:
	.quad	.L.h,.TOC.@tocbase,0
	.previous
	.type	h, @function
.L.h:
	li 3,0
	b g
	.long 0
	.byte 0,0,0,0,0,0,0,0
	.size	h,.-.L.h
	.ident	"GCC: (GNU) 8.0.0 20170721 (experimental) [trunk revision 250442]"
Comment 2 Yury Gribov 2017-07-24 20:03:01 UTC
Thanks for detailed report.

For pr79439.c take a look at discussion in bug 56727 - the general agreement there was that it should be valid to replace recursive PLT calls with direct calls as long as function does not have aliases.  GCC has already been doing such sort of optimization before.  So perhaps we need to update test (by adding alias there)?

pr56727-2.c is interesting, it does not repro on x86.  I'll be back on Thursday and will take a look (hopefully it's not urgent?).
Comment 3 Yury Gribov 2017-07-24 20:47:06 UTC
Created attachment 41822 [details]
Patch to disable test on PowerPC

(In reply to Yury Gribov from comment #2)
> pr56727-2.c is interesting, it does not repro on x86.

Ok, pr56727-2 fails because it gets optimized by tailcall pass (which is even more agressive at optimizing recursive calls).  I worked around this (by prohibiting tailcalls) but generated assembly is too different from x86.  So perhaps simply disable this test on Power?  Patch attached.
Comment 4 Yury Gribov 2017-07-25 04:28:00 UTC
Created attachment 41824 [details]
Update pattern for Power

Another option is to adapt test for Power (attached but not tested).
Comment 5 Jakub Jelinek 2017-11-24 11:27:51 UTC
Any progress with this?
Comment 6 Yury Gribov 2017-11-24 11:38:39 UTC
(In reply to Jakub Jelinek from comment #5)
> Any progress with this?

I filed patch back then (https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01873.html) and missed reply from Segher.  I'll reply to his comments in gcc-patches today.
Comment 7 Jakub Jelinek 2018-01-10 15:33:01 UTC
Any further progress?  I see you've posted something, but no further follow-ups from you nor Segher.
Comment 8 Yury Gribov 2018-01-10 19:25:40 UTC
(In reply to Jakub Jelinek from comment #7)
> Any further progress?  I see you've posted something, but no further
> follow-ups from you nor Segher.

Jakub, I didn't contribute for few months now due to health issues.  I checked with Segher offline and he said we can push this to February (if noone else claims otherwise).
Comment 9 seurer 2018-01-24 16:22:02 UTC
Just an FYI that the output of this test case changed a bit somewhere in the range r256987 to r257017:

Now it gets this:

FAIL: gcc.target/powerpc/pr79439.c scan-assembler-times \\mbl g\\M 1 (found 2 times)

previously it failed with this:

FAIL: gcc.target/powerpc/pr79439.c scan-assembler-times \\mnop\\M 3 (found 2 times)
Comment 10 Will Schmidt 2018-02-13 15:38:11 UTC
(In reply to seurer from comment #9)
> Just an FYI that the output of this test case changed a bit somewhere in the
> range r256987 to r257017:
> 
> Now it gets this:
> 
> FAIL: gcc.target/powerpc/pr79439.c scan-assembler-times \\mbl g\\M 1 (found
> 2 times)
> 
> previously it failed with this:
> 
> FAIL: gcc.target/powerpc/pr79439.c scan-assembler-times \\mnop\\M 3 (found 2
> times)

I looked briefly.  Per the generated assembly, the "bl g" check is failing because the branch to 'g' now occurs twice in the generated code.  (occurs with -O2, unless -fdisable-tree-early_optimizations is specified)  The " scan-assembler-times 'nop' 3 " now passes, but that is only due to the additional nop associated with the extra branch to 'g'.
Comment 11 Yury Gribov 2018-02-16 20:38:45 UTC
Author: ygribov
Date: Fri Feb 16 20:38:14 2018
New Revision: 257760

URL: https://gcc.gnu.org/viewcvs?rev=257760&root=gcc&view=rev
Log:
Fix PowerPC tests in PR 81535.

gcc/testsuite/

2018-02-16  Yury Gribov  <tetra2005@gmail.com>

	PR target/81535
	* gcc.dg/pr56727-1.c: Prevent tailcalls and update for powerpc*-*-*.
	* gcc.dg/pr56727-2.c: Ditto.
	* gcc.target/powerpc/pr79439.c: Renamed to...
	* gcc.target/powerpc/pr79439-1.c: ...this.
	* gcc.target/powerpc/pr79439-2.c: New test.
	* gcc.target/powerpc/pr79439-3.c: New test.

Added:
    trunk/gcc/testsuite/gcc.target/powerpc/pr79439-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/pr79439-2.c
    trunk/gcc/testsuite/gcc.target/powerpc/pr79439-3.c
Removed:
    trunk/gcc/testsuite/gcc.target/powerpc/pr79439.c
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/pr56727-1.c
    trunk/gcc/testsuite/gcc.dg/pr56727-2.c
Comment 12 Jeffrey A. Law 2018-02-16 23:41:43 UTC
Fixed by Yury's testsuite fixes on the trunk.