| Message ID | 20170113114012.GT32333@bubble.grove.modra.org |
|---|---|
| State | New |
| Headers | show |
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.
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.
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
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;
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
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
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)") ;; 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; +}
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.