* PR79066, non-PIC code generated for powerpc glibc with -fpic
@ 2017-01-13 11:40 Alan Modra
2017-01-14 9:29 ` Segher Boessenkool
0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2017-01-13 11:40 UTC (permalink / raw)
To: gcc-patches; +Cc: Segher Boessenkool
Bootstrapped and regression tested powerpc64-linux biarch. elf_high
has said "Elf specific ways of loading addresses for non-PIC code"
^^^^^^^
right from the inital V4 support in 1995.
OK for mainline?
PR target/79066
* config/rs6000/rs6000.md (elf_high, elf_low): Disable when pic.
testsuite/
* gcc.target/powerpc/pr79066.c: New.
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 80c10a7..98209fa 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -10597,14 +10597,14 @@ (define_insn_and_split "*tocref<mode>"
(define_insn "elf_high"
[(set (match_operand:SI 0 "gpc_reg_operand" "=b*r")
(high:SI (match_operand 1 "" "")))]
- "TARGET_ELF && ! TARGET_64BIT"
+ "TARGET_ELF && !TARGET_64BIT && !flag_pic"
"lis %0,%1@ha")
(define_insn "elf_low"
[(set (match_operand:SI 0 "gpc_reg_operand" "=r")
(lo_sum:SI (match_operand:SI 1 "gpc_reg_operand" "b")
(match_operand 2 "" "")))]
- "TARGET_ELF && ! TARGET_64BIT"
+ "TARGET_ELF && !TARGET_64BIT && !flag_pic"
"la %0,%2@l(%1)")
\f
;; Call and call_value insns
diff --git a/gcc/testsuite/gcc.target/powerpc/pr79066.c b/gcc/testsuite/gcc.target/powerpc/pr79066.c
new file mode 100644
index 0000000..86b2014
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr79066.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target { fpic && ilp32 } } } */
+/* { dg-options "-O2 -fpic" } */
+/* { dg-final { scan-assembler-not "lis.*@ha" } } */
+
+union U { double x; int i[2]; };
+
+double
+foo (double x)
+{
+ union U v;
+ v.i[0] = 0x7ff00000;
+ v.i[1] = 0;
+ return x / v.x;
+}
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PR79066, non-PIC code generated for powerpc glibc with -fpic
2017-01-13 11:40 PR79066, non-PIC code generated for powerpc glibc with -fpic Alan Modra
@ 2017-01-14 9:29 ` Segher Boessenkool
2017-01-14 13:38 ` Alan Modra
0 siblings, 1 reply; 10+ messages in thread
From: Segher Boessenkool @ 2017-01-14 9:29 UTC (permalink / raw)
To: Alan Modra; +Cc: gcc-patches
On Fri, Jan 13, 2017 at 10:10:12PM +1030, Alan Modra wrote:
> Bootstrapped and regression tested powerpc64-linux biarch. elf_high
> has said "Elf specific ways of loading addresses for non-PIC code"
> ^^^^^^^
> right from the inital V4 support in 1995.
>
> OK for mainline?
Have you checked if/what this changes for some bigger code?
Okay for trunk if there is nothing unexpected. Thanks!
Vladimir: Why does LRA attempt to manually construct high/lo_sum at all?
The next thing it tries is using lra_emit_move; this will also do it (on
all targets I checked), but only after appropriate (target-specific) checks.
It is way too late in stage 3 to attempt to change this now, of course ;-)
Segher
> PR target/79066
> * config/rs6000/rs6000.md (elf_high, elf_low): Disable when pic.
> testsuite/
> * gcc.target/powerpc/pr79066.c: New.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PR79066, non-PIC code generated for powerpc glibc with -fpic
2017-01-14 9:29 ` Segher Boessenkool
@ 2017-01-14 13:38 ` Alan Modra
2017-01-14 13:46 ` Jakub Jelinek
2017-01-14 15:19 ` PR79066, non-PIC code generated for powerpc glibc with -fpic Segher Boessenkool
0 siblings, 2 replies; 10+ messages in thread
From: Alan Modra @ 2017-01-14 13:38 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches
On Sat, Jan 14, 2017 at 03:28:51AM -0600, Segher Boessenkool wrote:
> On Fri, Jan 13, 2017 at 10:10:12PM +1030, Alan Modra wrote:
> > Bootstrapped and regression tested powerpc64-linux biarch. elf_high
> > has said "Elf specific ways of loading addresses for non-PIC code"
> > ^^^^^^^
> > right from the inital V4 support in 1995.
> >
> > OK for mainline?
>
> Have you checked if/what this changes for some bigger code?
Well, the bootstrap was all langs (minus ada due to not having ada
installed) and ppc32 multilibs were built. Plus the testsuite run
with RUNTESTFLAGS="--target_board=unix'{-m32,-m64}{-mlra,-mno-lra}'"
I would have bootstrapped -m32 except the machine I used lacked 32-bit
gmp, mpfr, mpc and I was lazy.
> Okay for trunk if there is nothing unexpected. Thanks!
I guess I should at least build glibc.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PR79066, non-PIC code generated for powerpc glibc with -fpic
2017-01-14 13:38 ` Alan Modra
@ 2017-01-14 13:46 ` Jakub Jelinek
2017-01-14 14:07 ` Avoid PR72749 by not using unspecs Alan Modra
2017-01-14 15:19 ` PR79066, non-PIC code generated for powerpc glibc with -fpic Segher Boessenkool
1 sibling, 1 reply; 10+ messages in thread
From: Jakub Jelinek @ 2017-01-14 13:46 UTC (permalink / raw)
To: Alan Modra; +Cc: Segher Boessenkool, gcc-patches
On Sun, Jan 15, 2017 at 12:08:39AM +1030, Alan Modra wrote:
> On Sat, Jan 14, 2017 at 03:28:51AM -0600, Segher Boessenkool wrote:
> > On Fri, Jan 13, 2017 at 10:10:12PM +1030, Alan Modra wrote:
> > > Bootstrapped and regression tested powerpc64-linux biarch. elf_high
> > > has said "Elf specific ways of loading addresses for non-PIC code"
> > > ^^^^^^^
> > > right from the inital V4 support in 1995.
> > >
> > > OK for mainline?
> >
> > Have you checked if/what this changes for some bigger code?
>
> Well, the bootstrap was all langs (minus ada due to not having ada
> installed) and ppc32 multilibs were built. Plus the testsuite run
> with RUNTESTFLAGS="--target_board=unix'{-m32,-m64}{-mlra,-mno-lra}'"
>
> I would have bootstrapped -m32 except the machine I used lacked 32-bit
> gmp, mpfr, mpc and I was lazy.
>
> > Okay for trunk if there is nothing unexpected. Thanks!
>
> I guess I should at least build glibc.
See Uros' comment about the INSN_CODE (insn) = insn_code_number;.
Also, I'm worried about it for another reason, after the
if (!targetm.legitimate_combined_insn (insn))
call the PATTERN is reverted to something different, so keeping INSN_CODE
equal to insn_code_number (which we sometimes even change to -1)
looks wrong, if it is the right thing to do it for the
legitimate_combined_insn call, it should be reverted afterwards when the
PATTERN changes again.
Or, perhaps instead of setting INSN_CODE, pass insn_code_number to the
target hook as another argument?
Jakub
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Avoid PR72749 by not using unspecs
2017-01-14 13:46 ` Jakub Jelinek
@ 2017-01-14 14:07 ` Alan Modra
2017-01-14 14:11 ` Jakub Jelinek
0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2017-01-14 14:07 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Segher Boessenkool, gcc-patches
On Sat, Jan 14, 2017 at 02:46:11PM +0100, Jakub Jelinek wrote:
> See Uros' comment about the INSN_CODE (insn) = insn_code_number;.
Later email retracted the one about a crash.
> Also, I'm worried about it for another reason, after the
> if (!targetm.legitimate_combined_insn (insn))
> call the PATTERN is reverted to something different, so keeping INSN_CODE
> equal to insn_code_number (which we sometimes even change to -1)
> looks wrong, if it is the right thing to do it for the
> legitimate_combined_insn call, it should be reverted afterwards when the
> PATTERN changes again.
It is reverted afterwards.
PATTERN (insn) = old_pat;
REG_NOTES (insn) = old_notes;
INSN_CODE (insn) = old_icode;
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Avoid PR72749 by not using unspecs
2017-01-14 14:07 ` Avoid PR72749 by not using unspecs Alan Modra
@ 2017-01-14 14:11 ` Jakub Jelinek
0 siblings, 0 replies; 10+ messages in thread
From: Jakub Jelinek @ 2017-01-14 14:11 UTC (permalink / raw)
To: Alan Modra; +Cc: Segher Boessenkool, gcc-patches
On Sun, Jan 15, 2017 at 12:37:19AM +1030, Alan Modra wrote:
> On Sat, Jan 14, 2017 at 02:46:11PM +0100, Jakub Jelinek wrote:
> > See Uros' comment about the INSN_CODE (insn) = insn_code_number;.
>
> Later email retracted the one about a crash.
>
> > Also, I'm worried about it for another reason, after the
> > if (!targetm.legitimate_combined_insn (insn))
> > call the PATTERN is reverted to something different, so keeping INSN_CODE
> > equal to insn_code_number (which we sometimes even change to -1)
> > looks wrong, if it is the right thing to do it for the
> > legitimate_combined_insn call, it should be reverted afterwards when the
> > PATTERN changes again.
>
> It is reverted afterwards.
>
> PATTERN (insn) = old_pat;
> REG_NOTES (insn) = old_notes;
> INSN_CODE (insn) = old_icode;
Ah, you're right, I've missed that. Also sorry for posting it in a wrong thread.
Jakub
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PR79066, non-PIC code generated for powerpc glibc with -fpic
2017-01-14 13:38 ` Alan Modra
2017-01-14 13:46 ` Jakub Jelinek
@ 2017-01-14 15:19 ` Segher Boessenkool
2017-01-16 5:20 ` Alan Modra
1 sibling, 1 reply; 10+ messages in thread
From: Segher Boessenkool @ 2017-01-14 15:19 UTC (permalink / raw)
To: Alan Modra; +Cc: gcc-patches
On Sun, Jan 15, 2017 at 12:08:39AM +1030, Alan Modra wrote:
> > Have you checked if/what this changes for some bigger code?
>
> Well, the bootstrap was all langs (minus ada due to not having ada
> installed) and ppc32 multilibs were built. Plus the testsuite run
> with RUNTESTFLAGS="--target_board=unix'{-m32,-m64}{-mlra,-mno-lra}'"
>
> I would have bootstrapped -m32 except the machine I used lacked 32-bit
> gmp, mpfr, mpc and I was lazy.
>
> > Okay for trunk if there is nothing unexpected. Thanks!
>
> I guess I should at least build glibc.
Yes exactly, something big that uses pic -- it is pretty obvious it won't
change anything for non-pic.
For gmp etc. you can use download_prequisites fwiw -- even lazier than
using the distro-provided binaries ;-)
Segher
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PR79066, non-PIC code generated for powerpc glibc with -fpic
2017-01-14 15:19 ` PR79066, non-PIC code generated for powerpc glibc with -fpic Segher Boessenkool
@ 2017-01-16 5:20 ` Alan Modra
2017-01-16 19:49 ` Segher Boessenkool
0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2017-01-16 5:20 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches
On Sat, Jan 14, 2017 at 09:19:52AM -0600, Segher Boessenkool wrote:
> On Sun, Jan 15, 2017 at 12:08:39AM +1030, Alan Modra wrote:
> > > Have you checked if/what this changes for some bigger code?
> >
> > Well, the bootstrap was all langs (minus ada due to not having ada
> > installed) and ppc32 multilibs were built. Plus the testsuite run
> > with RUNTESTFLAGS="--target_board=unix'{-m32,-m64}{-mlra,-mno-lra}'"
> >
> > I would have bootstrapped -m32 except the machine I used lacked 32-bit
> > gmp, mpfr, mpc and I was lazy.
> >
> > > Okay for trunk if there is nothing unexpected. Thanks!
> >
> > I guess I should at least build glibc.
>
> Yes exactly, something big that uses pic -- it is pretty obvious it won't
> change anything for non-pic.
glibc built fine without elf_high/elf_low with no regressions in its
testsuite. However, there is a problem in rs6000_emit_allocate_stack
-fstack-limit-symbol=SYMBOL code. This now might ICE if someone tries
to use the option with -fpic/PIC. I reckon the option combination to
be little used, so it shouldn't hurt to disable -fstack-limit-symbol
for PIC. (We were generating non-PIC for the trap, so we probably
would have gotten a complaint about text relocs in shared libraries.)
This revised patch has been bootstrapped and regression tested as
before, and tested with glibc too. OK?
PR target/79066
* config/rs6000/rs6000.md (elf_high, elf_low): Disable when pic.
* config/rs6000/rs6000.c (rs6000_emit_allocate_stack): Don't allow
symbolic stack limit when pic.
testsuite/
* gcc.target/powerpc/pr79066.c: New.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 11394b2..2dd6bbe 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -27668,7 +27668,8 @@ rs6000_emit_allocate_stack (HOST_WIDE_INT size, rtx copy_reg, int copy_off)
}
else if (GET_CODE (stack_limit_rtx) == SYMBOL_REF
&& TARGET_32BIT
- && DEFAULT_ABI == ABI_V4)
+ && DEFAULT_ABI == ABI_V4
+ && !flag_pic)
{
rtx toload = gen_rtx_CONST (VOIDmode,
gen_rtx_PLUS (Pmode,
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index b333474..f00334a 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -10581,14 +10581,14 @@ (define_insn_and_split "*tocref<mode>"
(define_insn "elf_high"
[(set (match_operand:SI 0 "gpc_reg_operand" "=b*r")
(high:SI (match_operand 1 "" "")))]
- "TARGET_ELF && ! TARGET_64BIT"
+ "TARGET_ELF && !TARGET_64BIT && !flag_pic"
"lis %0,%1@ha")
(define_insn "elf_low"
[(set (match_operand:SI 0 "gpc_reg_operand" "=r")
(lo_sum:SI (match_operand:SI 1 "gpc_reg_operand" "b")
(match_operand 2 "" "")))]
- "TARGET_ELF && ! TARGET_64BIT"
+ "TARGET_ELF && !TARGET_64BIT && !flag_pic"
"la %0,%2@l(%1)")
\f
;; Call and call_value insns
diff --git a/gcc/testsuite/gcc.target/powerpc/pr79066.c b/gcc/testsuite/gcc.target/powerpc/pr79066.c
new file mode 100644
index 0000000..86b2014
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr79066.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target { fpic && ilp32 } } } */
+/* { dg-options "-O2 -fpic" } */
+/* { dg-final { scan-assembler-not "lis.*@ha" } } */
+
+union U { double x; int i[2]; };
+
+double
+foo (double x)
+{
+ union U v;
+ v.i[0] = 0x7ff00000;
+ v.i[1] = 0;
+ return x / v.x;
+}
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PR79066, non-PIC code generated for powerpc glibc with -fpic
2017-01-16 5:20 ` Alan Modra
@ 2017-01-16 19:49 ` Segher Boessenkool
2017-01-17 2:52 ` Alan Modra
0 siblings, 1 reply; 10+ messages in thread
From: Segher Boessenkool @ 2017-01-16 19:49 UTC (permalink / raw)
To: Alan Modra; +Cc: gcc-patches
On Mon, Jan 16, 2017 at 03:50:01PM +1030, Alan Modra wrote:
> > > > Okay for trunk if there is nothing unexpected. Thanks!
> > >
> > > I guess I should at least build glibc.
> >
> > Yes exactly, something big that uses pic -- it is pretty obvious it won't
> > change anything for non-pic.
>
> glibc built fine without elf_high/elf_low with no regressions in its
> testsuite.
I meant comparing the generated code, sorry.
> However, there is a problem in rs6000_emit_allocate_stack
> -fstack-limit-symbol=SYMBOL code. This now might ICE if someone tries
> to use the option with -fpic/PIC. I reckon the option combination to
> be little used, so it shouldn't hurt to disable -fstack-limit-symbol
> for PIC. (We were generating non-PIC for the trap, so we probably
> would have gotten a complaint about text relocs in shared libraries.)
>
> This revised patch has been bootstrapped and regression tested as
> before, and tested with glibc too. OK?
>
> PR target/79066
> * config/rs6000/rs6000.md (elf_high, elf_low): Disable when pic.
> * config/rs6000/rs6000.c (rs6000_emit_allocate_stack): Don't allow
> symbolic stack limit when pic.
> testsuite/
> * gcc.target/powerpc/pr79066.c: New.
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 11394b2..2dd6bbe 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -27668,7 +27668,8 @@ rs6000_emit_allocate_stack (HOST_WIDE_INT size, rtx copy_reg, int copy_off)
> }
> else if (GET_CODE (stack_limit_rtx) == SYMBOL_REF
> && TARGET_32BIT
> - && DEFAULT_ABI == ABI_V4)
> + && DEFAULT_ABI == ABI_V4
> + && !flag_pic)
> {
> rtx toload = gen_rtx_CONST (VOIDmode,
> gen_rtx_PLUS (Pmode,
Please at least make it sorry() for ABI_V4 && flag_pic. Or what does it
result in with the patch as-is?
Okay with that change (if needed). Thanks!
Segher
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PR79066, non-PIC code generated for powerpc glibc with -fpic
2017-01-16 19:49 ` Segher Boessenkool
@ 2017-01-17 2:52 ` Alan Modra
0 siblings, 0 replies; 10+ messages in thread
From: Alan Modra @ 2017-01-17 2:52 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches
On Mon, Jan 16, 2017 at 01:49:36PM -0600, Segher Boessenkool wrote:
> On Mon, Jan 16, 2017 at 03:50:01PM +1030, Alan Modra wrote:
> > > > > Okay for trunk if there is nothing unexpected. Thanks!
> > > >
> > > > I guess I should at least build glibc.
> > >
> > > Yes exactly, something big that uses pic -- it is pretty obvious it won't
> > > change anything for non-pic.
> >
> > glibc built fine without elf_high/elf_low with no regressions in its
> > testsuite.
>
> I meant comparing the generated code, sorry.
libc.so had differences in debug sections due to libgcc source paths
being different, but other than that there were no differences.
> > However, there is a problem in rs6000_emit_allocate_stack
> > -fstack-limit-symbol=SYMBOL code. This now might ICE if someone tries
> > to use the option with -fpic/PIC. I reckon the option combination to
> > be little used, so it shouldn't hurt to disable -fstack-limit-symbol
> > for PIC. (We were generating non-PIC for the trap, so we probably
> > would have gotten a complaint about text relocs in shared libraries.)
> >
> > This revised patch has been bootstrapped and regression tested as
> > before, and tested with glibc too. OK?
> >
> > PR target/79066
> > * config/rs6000/rs6000.md (elf_high, elf_low): Disable when pic.
> > * config/rs6000/rs6000.c (rs6000_emit_allocate_stack): Don't allow
> > symbolic stack limit when pic.
> > testsuite/
> > * gcc.target/powerpc/pr79066.c: New.
> >
> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> > index 11394b2..2dd6bbe 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -27668,7 +27668,8 @@ rs6000_emit_allocate_stack (HOST_WIDE_INT size, rtx copy_reg, int copy_off)
> > }
> > else if (GET_CODE (stack_limit_rtx) == SYMBOL_REF
> > && TARGET_32BIT
> > - && DEFAULT_ABI == ABI_V4)
> > + && DEFAULT_ABI == ABI_V4
> > + && !flag_pic)
> > {
> > rtx toload = gen_rtx_CONST (VOIDmode,
> > gen_rtx_PLUS (Pmode,
>
> Please at least make it sorry() for ABI_V4 && flag_pic. Or what does it
> result in with the patch as-is?
I think we're OK as is. A warning is emitted "stack limit expression
is not supported".
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-01-17 2:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-13 11:40 PR79066, non-PIC code generated for powerpc glibc with -fpic Alan Modra
2017-01-14 9:29 ` Segher Boessenkool
2017-01-14 13:38 ` Alan Modra
2017-01-14 13:46 ` Jakub Jelinek
2017-01-14 14:07 ` Avoid PR72749 by not using unspecs Alan Modra
2017-01-14 14:11 ` Jakub Jelinek
2017-01-14 15:19 ` PR79066, non-PIC code generated for powerpc glibc with -fpic Segher Boessenkool
2017-01-16 5:20 ` Alan Modra
2017-01-16 19:49 ` Segher Boessenkool
2017-01-17 2:52 ` Alan Modra
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).