| Message ID | 583D45D0.5000201@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 29, 2016 at 1:09 AM, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > Hi all, > > This ICE only occurs on big-endian ILP32 -fpie code. The expansion code > generates the invalid load: > (insn 6 5 7 (set (reg/f:SI 76) > (unspec:SI [ > (mem/u/c:SI (lo_sum:SI (nil) > (symbol_ref:SI ("dbs") [flags 0x40] <var_decl > 0x7f6e387c0ab0 dbs>)) [0 S4 A8]) > ] UNSPEC_GOTSMALLPIC28K)) > (expr_list:REG_EQUAL (symbol_ref:SI ("dbs") [flags 0x40] <var_decl > 0x7f6e387c0ab0 dbs>) > (nil))) > > to load the symbol. Note the (nil) argument to lo_sum. > The buggy hunk meant to take the lowpart of the pic_offset_table_rtx > register but it did so by explicitly > constructing a subreg, for which the offset is wrong for big-endian. The > right way is to use gen_lowpart which > knows what exactly to do, with this patch we emit: > (insn 6 5 7 (set (reg/f:SI 76) > (unspec:SI [ > (mem/u/c:SI (lo_sum:SI (subreg:SI (reg:DI 73) 4) > (symbol_ref:SI ("dbs") [flags 0x40] <var_decl > 0x7ffb097e6ab0 dbs>)) [0 S4 A8]) > ] UNSPEC_GOTSMALLPIC28K)) > (expr_list:REG_EQUAL (symbol_ref:SI ("dbs") [flags 0x40] <var_decl > 0x7ffb097e6ab0 dbs>) > (nil))) > > and everything works fine. > > Bootstrapped and tested on aarch64-none-linux-gnu. > Also tested on aarch64_be-none-elf. > Ok for trunk? Naveen posted the exact same patch: https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02305.html Though with a slightly different testcase :). Thanks, Andrew > > Thanks, > Kyrill > > 2016-11-29 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/71112 > * config/aarch64/aarch64.c (aarch64_load_symref_appropriately, > case SYMBOL_SMALL_GOT_28K): Use gen_lowpart rather than constructing > subreg directly. > > 2016-11-29 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/71112 > * gcc.c-torture/compile/pr71112.c: New test.
On 30/11/16 05:27, Andrew Pinski wrote: > On Tue, Nov 29, 2016 at 1:09 AM, Kyrill Tkachov > <kyrylo.tkachov@foss.arm.com> wrote: >> Hi all, >> >> This ICE only occurs on big-endian ILP32 -fpie code. The expansion code >> generates the invalid load: >> (insn 6 5 7 (set (reg/f:SI 76) >> (unspec:SI [ >> (mem/u/c:SI (lo_sum:SI (nil) >> (symbol_ref:SI ("dbs") [flags 0x40] <var_decl >> 0x7f6e387c0ab0 dbs>)) [0 S4 A8]) >> ] UNSPEC_GOTSMALLPIC28K)) >> (expr_list:REG_EQUAL (symbol_ref:SI ("dbs") [flags 0x40] <var_decl >> 0x7f6e387c0ab0 dbs>) >> (nil))) >> >> to load the symbol. Note the (nil) argument to lo_sum. >> The buggy hunk meant to take the lowpart of the pic_offset_table_rtx >> register but it did so by explicitly >> constructing a subreg, for which the offset is wrong for big-endian. The >> right way is to use gen_lowpart which >> knows what exactly to do, with this patch we emit: >> (insn 6 5 7 (set (reg/f:SI 76) >> (unspec:SI [ >> (mem/u/c:SI (lo_sum:SI (subreg:SI (reg:DI 73) 4) >> (symbol_ref:SI ("dbs") [flags 0x40] <var_decl >> 0x7ffb097e6ab0 dbs>)) [0 S4 A8]) >> ] UNSPEC_GOTSMALLPIC28K)) >> (expr_list:REG_EQUAL (symbol_ref:SI ("dbs") [flags 0x40] <var_decl >> 0x7ffb097e6ab0 dbs>) >> (nil))) >> >> and everything works fine. >> >> Bootstrapped and tested on aarch64-none-linux-gnu. >> Also tested on aarch64_be-none-elf. >> Ok for trunk? > Naveen posted the exact same patch: > https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02305.html > Though with a slightly different testcase :). Ah, indeed. Sorry for the noise :) Kyrill > Thanks, > Andrew > >> Thanks, >> Kyrill >> >> 2016-11-29 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >> >> PR target/71112 >> * config/aarch64/aarch64.c (aarch64_load_symref_appropriately, >> case SYMBOL_SMALL_GOT_28K): Use gen_lowpart rather than constructing >> subreg directly. >> >> 2016-11-29 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >> >> PR target/71112 >> * gcc.c-torture/compile/pr71112.c: New test.
commit 99052634535958a776a2e7b7c9c6169cb656f5a8
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date: Mon Nov 28 12:26:57 2016 +0000
[AArch64] PR target/71112: Properly create lowpart of pic_offset_table_rtx with -fpie
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 686a8e92..aa833c8 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1299,7 +1299,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm,
emit_move_insn (gp_rtx, gen_rtx_HIGH (Pmode, s));
if (mode != GET_MODE (gp_rtx))
- gp_rtx = simplify_gen_subreg (mode, gp_rtx, GET_MODE (gp_rtx), 0);
+ gp_rtx = gen_lowpart (mode, gp_rtx);
}
if (mode == ptr_mode)
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr71112.c b/gcc/testsuite/gcc.c-torture/compile/pr71112.c
new file mode 100644
index 0000000..69e2df6
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr71112.c
@@ -0,0 +1,10 @@
+/* PR target/71112. */
+/* { dg-additional-options "-fpie" { target pie } } */
+
+extern int dbs[100];
+void f (int *);
+int nscd_init (void)
+{
+ f (dbs);
+ return 0;
+}
Hi all, This ICE only occurs on big-endian ILP32 -fpie code. The expansion code generates the invalid load: (insn 6 5 7 (set (reg/f:SI 76) (unspec:SI [ (mem/u/c:SI (lo_sum:SI (nil) (symbol_ref:SI ("dbs") [flags 0x40] <var_decl 0x7f6e387c0ab0 dbs>)) [0 S4 A8]) ] UNSPEC_GOTSMALLPIC28K)) (expr_list:REG_EQUAL (symbol_ref:SI ("dbs") [flags 0x40] <var_decl 0x7f6e387c0ab0 dbs>) (nil))) to load the symbol. Note the (nil) argument to lo_sum. The buggy hunk meant to take the lowpart of the pic_offset_table_rtx register but it did so by explicitly constructing a subreg, for which the offset is wrong for big-endian. The right way is to use gen_lowpart which knows what exactly to do, with this patch we emit: (insn 6 5 7 (set (reg/f:SI 76) (unspec:SI [ (mem/u/c:SI (lo_sum:SI (subreg:SI (reg:DI 73) 4) (symbol_ref:SI ("dbs") [flags 0x40] <var_decl 0x7ffb097e6ab0 dbs>)) [0 S4 A8]) ] UNSPEC_GOTSMALLPIC28K)) (expr_list:REG_EQUAL (symbol_ref:SI ("dbs") [flags 0x40] <var_decl 0x7ffb097e6ab0 dbs>) (nil))) and everything works fine. Bootstrapped and tested on aarch64-none-linux-gnu. Also tested on aarch64_be-none-elf. Ok for trunk? Thanks, Kyrill 2016-11-29 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR target/71112 * config/aarch64/aarch64.c (aarch64_load_symref_appropriately, case SYMBOL_SMALL_GOT_28K): Use gen_lowpart rather than constructing subreg directly. 2016-11-29 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR target/71112 * gcc.c-torture/compile/pr71112.c: New test.