Message ID | DB6PR0801MB2053ED838D2FC5576E5658E383250@DB6PR0801MB2053.eurprd08.prod.outlook.com |
---|---|
State | New |
Headers | show |
Series | [AArch64] Fix ICE due to store_pair_lanes | expand |
On Mon, Nov 27, 2017 at 03:20:29PM +0000, Wilco Dijkstra wrote: > The recently added store_pair_lanes causes ICEs in output_operand. > This is due to aarch64_classify_address treating it like a 128-bit STR > rather than a STP. The valid immediate offsets don't fully overlap, > causing it to return false. Eg. offset 264 is a valid 8-byte STP offset > but not a valid 16-byte STR offset since it isn't a multiple of 16. > > The original instruction isn't passed in the printing code, so the context > is unclear. The solution is to add a new operand formatting specifier > which is used for LDP/STP instructions like this. This, like the Uml > constraint that applies to store_pair_lanes, uses PARALLEL when calling > aarch64_classify_address so that it knows it is an STP. > Also add the 'z' specifier for future use by load/store pair instructions. > > Passes regress, OK for commit? OK. But... > + if (aarch64_classify_address (&addr, x, mode, op, true)) This interface is not nice, resulting in... > +/* Print address 'x' of a LDP/STP with mode 'mode'. */ > +static void > +aarch64_print_ldpstp_address (FILE *f, machine_mode mode, rtx x) > +{ > + aarch64_print_address_internal (f, mode, x, PARALLEL); > +} > + > +/* Print address 'x' of a memory access with mode 'mode'. */ > +static void > +aarch64_print_operand_address (FILE *f, machine_mode mode, rtx x) > +{ > + aarch64_print_address_internal (f, mode, x, MEM); > +} These, which I *really* dislike. Ideas on how to clean up this interface would be appreciated. Thanks, James
On 28 November 2017 at 19:15, James Greenhalgh <james.greenhalgh@arm.com> wrote: > On Mon, Nov 27, 2017 at 03:20:29PM +0000, Wilco Dijkstra wrote: >> The recently added store_pair_lanes causes ICEs in output_operand. >> This is due to aarch64_classify_address treating it like a 128-bit STR >> rather than a STP. The valid immediate offsets don't fully overlap, >> causing it to return false. Eg. offset 264 is a valid 8-byte STP offset >> but not a valid 16-byte STR offset since it isn't a multiple of 16. >> >> The original instruction isn't passed in the printing code, so the context >> is unclear. The solution is to add a new operand formatting specifier >> which is used for LDP/STP instructions like this. This, like the Uml >> constraint that applies to store_pair_lanes, uses PARALLEL when calling >> aarch64_classify_address so that it knows it is an STP. >> Also add the 'z' specifier for future use by load/store pair instructions. >> >> Passes regress, OK for commit? > > OK. But... > >> + if (aarch64_classify_address (&addr, x, mode, op, true)) > > This interface is not nice, resulting in... > >> +/* Print address 'x' of a LDP/STP with mode 'mode'. */ >> +static void >> +aarch64_print_ldpstp_address (FILE *f, machine_mode mode, rtx x) >> +{ >> + aarch64_print_address_internal (f, mode, x, PARALLEL); >> +} >> + >> +/* Print address 'x' of a memory access with mode 'mode'. */ >> +static void >> +aarch64_print_operand_address (FILE *f, machine_mode mode, rtx x) >> +{ >> + aarch64_print_address_internal (f, mode, x, MEM); >> +} > > These, which I *really* dislike. > > Ideas on how to clean up this interface would be appreciated. > > Thanks, > James > Hi Wilco, This breaks the build of aarch64-none-elf toolchains: /tmp/8868058_9.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64_be-none-elf/gcc1/./gcc/xgcc -B/tmp/8868058_9.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64_be-none-elf/gcc1/./gcc/ -B/tmp/8868058_9.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/aarch64_be-none-elf/bin/ -B/tmp/8868058_9.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/aarch64_be-none-elf/lib/ -isystem /tmp/8868058_9.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/aarch64_be-none-elf/include -isystem /tmp/8868058_9.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/aarch64_be-none-elf/sys-include -g -O2 -mabi=ilp32 -O2 -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -W -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -isystem ./include -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector -Dinhibit_libc -I. -I. -I../../.././gcc -I/tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc -I/tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/. -I/tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/../gcc -I/tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/../include -o unwind-dw2-fde.o -MT unwind-dw2-fde.o -MD -MP -MF unwind-dw2-fde.dep -fexceptions -c /tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2-fde.c -fvisibility=hidden -DHIDE_EXPORTS during RTL pass: final /tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2-fde.c: In function 'search_object': /tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2-fde.c:1024:1: internal compiler error: in aarch64_print_address_internal, at config/aarch64/aarch64.c:5637 } ^ 0xf2259e aarch64_print_address_internal /tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/aarch64/aarch64.c:5637 0x7fb063 output_address(machine_mode, rtx_def*) /tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/final.c:3905 0x7fe563 output_asm_insn(char const*, rtx_def**) /tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/final.c:3766 0x7fed04 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) /tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/final.c:3064 0x7ffcfa final(rtx_insn*, _IO_FILE*, int) /tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/final.c:2052 0x80088b rest_of_handle_final /tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/final.c:4490 0x80088b execute /tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/final.c:4564 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions. make[4]: *** [unwind-dw2-fde.o] Error 1 make[4]: Leaving directory `/tmp/8868058_9.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64_be-none-elf/gcc1/aarch64_be-none-elf/ilp32/libgcc' make[3]: *** [multi-do] Error 1 Can you have a look? Thanks, Christophe
On Wed, 2017-11-29 at 21:13 +0100, Christophe Lyon wrote: > Hi Wilco, > > This breaks the build of aarch64-none-elf toolchains: > > /tmp/8868058_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/final.c:4564 > Please submit a full bug report, > with preprocessed source if appropriate. > Please include the complete backtrace with any bug report. > See <https://gcc.gnu.org/bugs/> for instructions. > make[4]: *** [unwind-dw2-fde.o] Error 1 > make[4]: Leaving directory > `/tmp/8868058_9.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64_be-none-elf/gcc1/aarch64_be-none-elf/ilp32/libgcc' > make[3]: *** [multi-do] Error 1 I am seeing this failure too, on my aarch64-linux-gnu build where I have ILP32 enabled. Steve Ellcey sellcey@cavium.com
FYI: Here is a cut down test case showing the failure: int foo (void) { } extern void plunk (); int splat (void) { static int once = 0; plunk (&once, foo); } % obj/gcc/gcc/cc1 -mabi=ilp32 -O2 -quiet x.i during RTL pass: final x.i: In function ‘splat’: x.i:7:1: internal compiler error: in aarch64_print_address_internal, at config/aarch64/aarch64.c:5638 } ^ 0x14286c7 aarch64_print_address_internal /home/sellcey/gcc-spec- ilp32/src/gcc/gcc/config/aarch64/aarch64.c:5638 0x1428d4b aarch64_print_operand_address /home/sellcey/gcc-spec- ilp32/src/gcc/gcc/config/aarch64/aarch64.c:5735 0xb5fa93 output_address(machine_mode, rtx_def*) /home/sellcey/gcc-spec-ilp32/src/gcc/gcc/final.c:3905 0xb5f40b output_asm_insn(char const*, rtx_def**) /home/sellcey/gcc-spec-ilp32/src/gcc/gcc/final.c:3766 0xb5e0df final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) /home/sellcey/gcc-spec-ilp32/src/gcc/gcc/final.c:3064 0xb5bfbb final(rtx_insn*, _IO_FILE*, int) /home/sellcey/gcc-spec-ilp32/src/gcc/gcc/final.c:2052 0xb60d27 rest_of_handle_final /home/sellcey/gcc-spec-ilp32/src/gcc/gcc/final.c:4490 0xb6110f execute /home/sellcey/gcc-spec-ilp32/src/gcc/gcc/final.c:4564 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions.
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md index cddd935d96589c52519334bd1b8c24e80ea475f3..b7dfbfaa1b00df113329e4b35397e2b6ce234786 100644 --- a/gcc/config/aarch64/aarch64-simd.md +++ b/gcc/config/aarch64/aarch64-simd.md @@ -3047,8 +3047,8 @@ (define_insn "store_pair_lanes<mode>" (match_operand:VDC 2 "register_operand" "w, r")))] "TARGET_SIMD" "@ - stp\\t%d1, %d2, %0 - stp\\t%x1, %x2, %0" + stp\\t%d1, %d2, %y0 + stp\\t%x1, %x2, %y0" [(set_attr "type" "neon_stp, store_16")] ) diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 60a8555b16296f07c68059bc54db803aeef0f369..e601f573bde4fae9a6e32af79ee987190895e088 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -150,6 +150,7 @@ static bool aarch64_builtin_support_vector_misalignment (machine_mode mode, bool is_packed); static machine_mode aarch64_simd_container_mode (scalar_mode mode, unsigned width); +static void aarch64_print_ldpstp_address (FILE *f, machine_mode mode, rtx x); /* Major revision number of the ARM Architecture implemented by the target. */ unsigned aarch64_architecture_version; @@ -5448,7 +5449,11 @@ static const int aarch64_nzcv_codes[] = 'L': Output constant address specified by X with a relocation offset if appropriate. 'G': Prints address of X, specifying a PC relative - relocation mode if appropriate. */ + relocation mode if appropriate. + 'y': Output address of LDP or STP - this is used for + some LDP/STPs which don't use a PARALLEL in their + pattern (so the mode needs to be adjusted). + 'z': Output address of a typical LDP or STP. */ static void aarch64_print_operand (FILE *f, rtx x, int code) @@ -5650,8 +5655,6 @@ aarch64_print_operand (FILE *f, rtx x, int code) case MEM: output_address (GET_MODE (x), XEXP (x, 0)); - /* Check all memory references are Pmode - even with ILP32. */ - gcc_assert (GET_MODE (XEXP (x, 0)) == Pmode); break; case CONST: @@ -5815,18 +5818,48 @@ aarch64_print_operand (FILE *f, rtx x, int code) } break; + case 'y': + case 'z': + { + machine_mode mode = GET_MODE (x); + + if (GET_CODE (x) != MEM) + { + output_operand_lossage ("invalid operand for '%%%c'", code); + return; + } + + if (code == 'y') + { + /* LDP/STP which uses a single double-width memory operand. + Adjust the mode to appear like a typical LDP/STP. + Currently this is supported for 16-byte accesses only. */ + gcc_assert (GET_MODE_SIZE (mode) == 16); + mode = DFmode; + } + + aarch64_print_ldpstp_address (f, mode, XEXP (x, 0)); + } + break; + default: output_operand_lossage ("invalid operand prefix '%%%c'", code); return; } } +/* Print address 'x' of a memory access with mode 'mode'. + 'op' is the context required by aarch64_classify_address. It can either be + MEM for a normal memory access or PARALLEL for LDP/STP. */ static void -aarch64_print_operand_address (FILE *f, machine_mode mode, rtx x) +aarch64_print_address_internal (FILE *f, machine_mode mode, rtx x, RTX_CODE op) { struct aarch64_address_info addr; - if (aarch64_classify_address (&addr, x, mode, MEM, true)) + /* Check all addresses are Pmode - including ILP32. */ + gcc_assert (GET_MODE (x) == Pmode); + + if (aarch64_classify_address (&addr, x, mode, op, true)) switch (addr.type) { case ADDRESS_REG_IMM: @@ -5909,6 +5942,20 @@ aarch64_print_operand_address (FILE *f, machine_mode mode, rtx x) output_addr_const (f, x); } +/* Print address 'x' of a LDP/STP with mode 'mode'. */ +static void +aarch64_print_ldpstp_address (FILE *f, machine_mode mode, rtx x) +{ + aarch64_print_address_internal (f, mode, x, PARALLEL); +} + +/* Print address 'x' of a memory access with mode 'mode'. */ +static void +aarch64_print_operand_address (FILE *f, machine_mode mode, rtx x) +{ + aarch64_print_address_internal (f, mode, x, MEM); +} + bool aarch64_label_mentioned_p (rtx x) {