[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 Iain Sandoe changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |FIXED --- Comment #37 from Iain Sandoe --- fixed on all open branches.
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 Jakub Jelinek changed: What|Removed |Added Target Milestone|9.0 |9.2 --- Comment #36 from Jakub Jelinek --- GCC 9.1 has been released.
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #35 from Alan Modra --- Author: amodra Date: Sat Feb 9 12:44:02 2019 New Revision: 268722 URL: https://gcc.gnu.org/viewcvs?rev=268722=gcc=rev Log: [RS6000] Correct save_reg_p PR target/88343 * config/rs6000/rs6000.c (rs6000_reg_live_or_pic_offset_p): Match logic in rs6000_emit_prologue emitting pic_offset_table setup. Modified: branches/gcc-7-branch/gcc/ChangeLog branches/gcc-7-branch/gcc/config/rs6000/rs6000.c
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #34 from Iain Sandoe --- (In reply to Alan Modra from comment #33) > It looks to me like that hunk is just removing some dead code, so it doesn't > matter whether it stays or goes. yes, just a tidy-up, should not affect function (but maybe best to apply when it's fresh in the mind). I can try to talk care of it tomorrow.
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #33 from Alan Modra --- It looks to me like that hunk is just removing some dead code, so it doesn't matter whether it stays or goes.
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #32 from Iain Sandoe --- Thanks for the patches Alan. Note that 7 and 8 back ports might need this hunk which was applied to trunk but removed from 7 and 8 when we backed out the change because of breakage there. @@ -23970,13 +23974,6 @@ first_reg_to_save (void) if (save_reg_p (first_reg)) break; -#if TARGET_MACHO - if (flag_pic - && crtl->uses_pic_offset_table - && first_reg > RS6000_PIC_OFFSET_TABLE_REGNUM) -return RS6000_PIC_OFFSET_TABLE_REGNUM; -#endif - return first_reg; } my ppc darwin box is busy with test cycle at present, but I will check 8.2.1 tomorrow with the patch as applied - and also look to see if the hunk above is needed?
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #31 from Alan Modra --- Author: amodra Date: Sat Feb 9 08:39:58 2019 New Revision: 268715 URL: https://gcc.gnu.org/viewcvs?rev=268715=gcc=rev Log: [RS6000] Correct save_reg_p PR target/88343 * config/rs6000/rs6000.c (save_reg_p): Match logic in rs6000_emit_prologue emitting pic_offset_table setup. Modified: branches/gcc-8-branch/gcc/ChangeLog branches/gcc-8-branch/gcc/config/rs6000/rs6000.c
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343
--- Comment #30 from Alan Modra ---
Author: amodra
Date: Fri Feb 8 22:20:58 2019
New Revision: 268708
URL: https://gcc.gnu.org/viewcvs?rev=268708=gcc=rev
Log:
[RS6000] Correct save_reg_p
Fixes lack of r30 save/restore on
// -m32 -fpic -ftls-model=initial-exec
__thread char* p;
char** f1 (void) { return }
and
// -m32 -fpic -msecure-plt
extern int foo (int);
int f1 (int x) { return foo (x); }
These are both caused by save_reg_p returning false when the pic
offset table reg (r30 for ABI_V4) was used, due to the logic not
exactly matching that in rs6000_emit_prologue to set up r30.
I also noticed that save_reg_p isn't following the comment regarding
calls_eh_return (since svn 267049, git 0edf78b1b2a0), and the comment
needs tweaking too. For why the revised comment is correct, grep for
saves_all_registers in lra.c, and yes, we do want to save the pic
offset table reg for eh_return.
PR target/88343
* config/rs6000/rs6000.c (save_reg_p): Correct calls_eh_return
case. Match logic in rs6000_emit_prologue emitting pic_offset_table
setup.
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/rs6000/rs6000.c
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343
--- Comment #29 from Segher Boessenkool ---
This:
===
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 401e719..f0adef7 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -37988,7 +37988,10 @@ rs6000_call_sysv (rtx value, rtx func_desc, rtx
tlsarg,
&& flag_pic
&& GET_CODE (func_addr) == SYMBOL_REF
&& !SYMBOL_REF_LOCAL_P (func_addr))
-call[n++] = gen_rtx_USE (VOIDmode, pic_offset_table_rtx);
+{
+ crtl->uses_pic_offset_table = 1;
+ call[n++] = gen_rtx_USE (VOIDmode, pic_offset_table_rtx);
+}
call[n++] = gen_hard_reg_clobber (Pmode, LR_REGNO);
===
seems to do the trick.
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #28 from Segher Boessenkool --- (In reply to Alan Modra from comment #27) > And possibly -msecure-plt Yeah that does the trick, thanks :-)
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #27 from Alan Modra --- And possibly -msecure-plt
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #26 from Alan Modra --- > I don't see that; I get You need -fpic
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #25 from Segher Boessenkool --- I don't see that; I get f1: .LFB0: .cfi_startproc b foo@plt .cfi_endproc .LFE0: What extra options do I need? Or, what other target (you didn't say, I used powerpc-linux).
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #24 from Iain Sandoe --- so, it seems that there are more cases where the RS6000_PIC_OFFSET_TABLE_REGNUM is used without setting the uses_pic_offset_table. We can easily back the change out to "fix" master - but that seems to be papering over the underlying issue?
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343
Alan Modra changed:
What|Removed |Added
CC||amodra at gmail dot com
--- Comment #23 from Alan Modra ---
On master, something as simple as
extern int foo (int);
int f1 (int x) { return foo (x); }
compiled with -S -m32 -mbig -fpic -O2 results in a use of r30 without saving.
f1:
.LFB0:
.cfi_startproc
mflr 0
.cfi_register 65, 0
bcl 20,31,.L2
.L2:
stwu 1,-16(1)
.cfi_def_cfa_offset 16
mflr 30
addis 30,30,_GLOBAL_OFFSET_TABLE_-.L2@ha
addi 30,30,_GLOBAL_OFFSET_TABLE_-.L2@l
stw 0,20(1)
.cfi_offset 65, 4
bl foo@plt
lwz 0,20(1)
addi 1,1,16
.cfi_def_cfa_offset 0
mtlr 0
.cfi_restore 65
blr
.cfi_endproc
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #22 from joseph at codesourcery dot com --- A large proportion of the glibc libm tests still segfault for powerpc-linux-gnu soft-float with that patch applied to GCC trunk. (Although I don't see nextafter tests among those falling over, I don't think that means much, given how sensitive this issue is to the exact details of code generation.)
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343
--- Comment #21 from Segher Boessenkool ---
Before the holidays I did this patch:
===
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index fa5f032..2ffe7d9 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -8721,7 +8721,10 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model
model)
else
{
if (flag_pic == 1)
- got = gen_rtx_REG (Pmode, RS6000_PIC_OFFSET_TABLE_REGNUM);
+ {
+ got = gen_rtx_REG (Pmode, RS6000_PIC_OFFSET_TABLE_REGNUM);
+ crtl->uses_pic_offset_table = 1;
+ }
else
{
rtx gsym = rs6000_got_sym ();
===
... but I have no idea if it solved things or not (before Iain alerted me to
it I had no idea what it was _for_!)
Joseph, could you try it out?
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #20 from Iain Sandoe --- (In reply to Joseph S. Myers from comment #19) > Created attachment 45330 [details] > Bad assembly (from trunk r267560 with the patch still present) Thanks, Joseph, that's very helpful. .. so the code plainly uses pic base reg /_GLOBAL_OFFSET_TABLE_ and thus fails when it's not saved... .. the question to be addressed then is why crtl->uses_pic_offset_table not set in this case? (a question which perhaps has wider implications than this missed optimisation).
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #19 from Joseph S. Myers --- Created attachment 45330 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45330=edit Bad assembly (from trunk r267560 with the patch still present)
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #18 from Joseph S. Myers --- Created attachment 45329 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45329=edit Good assembly (with the GCC patch reverted)
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #17 from Joseph S. Myers --- Created attachment 45328 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45328=edit Preprocessed source Replacing s_nextafterf.os built with trunk with that patch reverted, by the same file as built with trunk without the reversion, is sufficient to produce a glibc binary showing the problem. Preprocessed sources attached. Given the --with-float=soft compiler, compile with: -std=gnu11 -fgnu89-inline -g -O2 -fmerge-all-constants -frounding-math -fno-stack-protector -fno-math-errno -mlong-double-128 -fpic s_nextafterf.i.
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343
--- Comment #16 from joseph at codesourcery dot com ---
On Thu, 3 Jan 2019, iains at gcc dot gnu.org wrote:
> Unfortunately, there doesn't appear to be anything in the GCC test suite that
> catches this (all languages reg-strap was clean, on gcc110). Does it show for
> a 32b multilib on a 64b host, or only with a 32b host?
All my testing has been for x86_64-linux-gnu host, powerpc-linux-gnu
target, --disable-multilib --with-float=soft.
The issue appears when a test built with a compiler without the problem
patch is run with glibc built with a compiler with the patch, so it looks
like something miscompiled in glibc - but it also seems sensitive to e.g.
the exact compilation options used for compiling the test with the
non-buggy compiler (as would be expected if e.g. the buggy compiler is
generating code that's using uninitialized or overwritten data in some way
and so details that should be irrelevant end up mattering). I'll look for
a specific object file in glibc that shows the issue if built with the
buggy compiler and inserted in glibc otherwise built with the non-buggy
compiler. The current cut-down version of the glibc test I have (built
with -lm -O2 -fno-builtin with GCC 8 branch just before the change) is:
float nextafterf (float, float);
int printf (const char *, ...);
static void
check_ulp (void)
{
float ulps, value;
value = nextafterf (10, 20);
/* Should print 0x1.42p+3. */
printf ("%a\n", value);
for (int i = 1; i < 100; i++)
value = nextafterf (value, 20);
ulps = __builtin_fabsf (value - 10);
/* Should print 0x1.9p-14; buggy case hangs before this. */
printf ("%a\n", ulps);
}
int
main (void)
{
check_ulp ();
}
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #15 from Iain Sandoe --- Author: iains Date: Thu Jan 3 08:45:35 2019 New Revision: 267543 URL: https://gcc.gnu.org/viewcvs?rev=267543=gcc=rev Log: revert fix for pr88343 This causes problems for soft-f on GLIBC, see pr comment 11. 2019-01-03 Iain Sandoe revert: 2018-12-23 Iain Sandoe backport from mainline. 2018-12-12 Segher Boessenkool Iain Sandoe PR target/88343 * config/rs6000/rs6000.c (save_reg_p): Do not save the picbase reg unless it has been used. (first_reg_to_save): Remove dead code. Modified: branches/gcc-8-branch/gcc/ChangeLog branches/gcc-8-branch/gcc/config/rs6000/rs6000.c
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #14 from Iain Sandoe --- Author: iains Date: Thu Jan 3 08:34:41 2019 New Revision: 267542 URL: https://gcc.gnu.org/viewcvs?rev=267542=gcc=rev Log: revert fix for pr88343 causes problems with soft-fp in GLIBC, see pr comment 11 2019-01-03 Iain Sandoe revert: 2018-12-30 Iain Sandoe backport from mainline. 2018-12-12 Segher Boessenkool Iain Sandoe PR target/88343 * config/rs6000/rs6000.c (save_reg_p): Do not save the picbase reg unless it has been used. (first_reg_to_save): Remove dead code. Modified: branches/gcc-7-branch/gcc/ChangeLog branches/gcc-7-branch/gcc/config/rs6000/rs6000.c
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #13 from Iain Sandoe --- (In reply to Joseph S. Myers from comment #11) > This change results in miscompilation of glibc for 32-bit soft-float powerpc > (symptoms: many libm tests as run by "make regen-ulps" either segfault, or > produce spurious errors that don't occur with compilers without this change, > or in the case of tests for float fail the "Value outside of 100 +/- 1ulp." > internal sanity check). > > Specifically, on GCC 7 branch r267476 works OK but the tip of the branch, > which has no subsequent changes other than this patch and changes to > DATESTAMP, miscompiles glibc. On GCC 8 branch r267386 (the revision before > this change) works and tip of the branch miscompiles glibc (but I haven't > bisected there to confirm it is indeed this change that's responsible). > Trunk also miscompiles glibc (I haven't tried previous revisions there). Unfortunately, there doesn't appear to be anything in the GCC test suite that catches this (all languages reg-strap was clean, on gcc110). Does it show for a 32b multilib on a 64b host, or only with a 32b host? (if you have a specific reproducer that would be most welcome).
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 Segher Boessenkool changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|FIXED |--- --- Comment #12 from Segher Boessenkool --- Please revert this on 7 and 8 for now, and we'll fix this on trunk? Or revert it on trunk as well, if it is in the way. Thanks!
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #11 from Joseph S. Myers --- This change results in miscompilation of glibc for 32-bit soft-float powerpc (symptoms: many libm tests as run by "make regen-ulps" either segfault, or produce spurious errors that don't occur with compilers without this change, or in the case of tests for float fail the "Value outside of 100 +/- 1ulp." internal sanity check). Specifically, on GCC 7 branch r267476 works OK but the tip of the branch, which has no subsequent changes other than this patch and changes to DATESTAMP, miscompiles glibc. On GCC 8 branch r267386 (the revision before this change) works and tip of the branch miscompiles glibc (but I haven't bisected there to confirm it is indeed this change that's responsible). Trunk also miscompiles glibc (I haven't tried previous revisions there).
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 Iain Sandoe changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #10 from Iain Sandoe --- Fixed on open branches, it's possibly required for 6.x if someone is maintaining a branch there, they might wish to apply it.
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #9 from Iain Sandoe --- Author: iains Date: Sun Dec 30 13:20:19 2018 New Revision: 267477 URL: https://gcc.gnu.org/viewcvs?rev=267477=gcc=rev Log: fix PR target/88343 for 32b powerpc. 2018-12-30 Iain Sandoe backport from mainline. 2018-12-12 Segher Boessenkool Iain Sandoe PR target/88343 * config/rs6000/rs6000.c (save_reg_p): Do not save the picbase reg unless it has been used. (first_reg_to_save): Remove dead code. Modified: branches/gcc-7-branch/gcc/ChangeLog branches/gcc-7-branch/gcc/config/rs6000/rs6000.c
[Bug target/88343] [7/8 Regression] R31 is unconditionally saved/restored on powerpc-darwin even when it's not necessary.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88343 --- Comment #8 from Iain Sandoe --- Author: iains Date: Sun Dec 23 21:17:46 2018 New Revision: 267387 URL: https://gcc.gnu.org/viewcvs?rev=267387=gcc=rev Log: fix PR target/88343 by backporting r267049 The PR is about unnecessary saves of the pic base register, it shows on m32 Linux and m32/m64 Darwin. 2018-12-23 Iain Sandoe backport from mainline. 2018-12-12 Segher Boessenkool Iain Sandoe PR target/88343 * config/rs6000/rs6000.c (save_reg_p): Do not save the picbase reg unless it has been used. (first_reg_to_save): Remove dead code. Modified: branches/gcc-8-branch/gcc/ChangeLog branches/gcc-8-branch/gcc/config/rs6000/rs6000.c
