Manolis Tsamis <manolis.tsa...@vrull.eu> writes:
>> > I could have some help with that, because after the new changes a
>> > subreg related ICE also happens within store_bit_field when a DI ->
>> > V4SI case is hit. Afaik store_bit_field should just return NULL if it
>> > can't handle something so I don't really know how to address this ICE
>> > in the new version.
>>
>> I don't think store_bit_field is expected to fail. But yeah, if you have
>> a testcase, I can take a look.
>>
>
> Yes, that was my initial reaction too. I don't yet have a reduced
> testcase, this now only happens with SPEC2017 gcc_r when compiled with
> `-g -O3 -march=armv8.2-a -flto=32 -favoid-store-forwarding` (doesn't
> reproduce without flto). I have also attached the work-in-progress
> patch that implements your changes up to that point and which can be
> used to reproduce this, at least till we have a smaller testcase.
>
> The case that leads to that ICE is this:
>
> Store forwarding detected:
> From: (insn 1700 1714 1701 132 (set (mem/j/c:SI (plus:DI (reg/f:DI 64 sfp)
> (const_int -160 [0xffffffffffffff60])) [0 +0 S4 A128])
> (reg:SI 617)) "real.c":158:9 69 {*movsi_aarch64}
> (expr_list:REG_DEAD (reg:SI 617)
> (nil)))
> From: (insn 1695 1698 1715 132 (set (mem/c:TI (plus:DI (reg/f:DI 64 sfp)
> (const_int -160 [0xffffffffffffff60])) [0 MEM [(void
> *)&r]+0 S16 A128])
> (const_int 0 [0])) "real.c":157:3 75 {*movti_aarch64}
> (nil))
> To: (insn 1709 1703 1696 132 (set (reg:V4SI 622 [ r ])
> (mem/c:V4SI (plus:DI (reg/f:DI 64 sfp)
> (const_int -160 [0xffffffffffffff60])) [51 r+0 S16
> A128])) "builtins.c":9619:13 1270 {*aarch64_simd_movv4si}
> (nil))
>
> Which is somewhat suspicious; we'll have to first make sure it's not a
> avoid-store-forwarding bug.
Does 1695 come before 1700? If so, it looks like it should be valid.
> In any case the ICE is
>
> builtins.c:6684:1: internal compiler error: in simplify_subreg, at
> simplify-rtx.cc:7680
> 6684 | }
> | ^
> 0xc41ecb simplify_context::simplify_subreg(machine_mode, rtx_def*,
> machine_mode, poly_int<2u, unsigned long>)
> /home/mtsamis/src/gcc/gcc4/gcc/simplify-rtx.cc:7680
> 0xc42d0f simplify_context::simplify_gen_subreg(machine_mode, rtx_def*,
> machine_mode, poly_int<2u, unsigned long>)
> /home/mtsamis/src/gcc/gcc4/gcc/simplify-rtx.cc:8007
> 0x8afb4b simplify_gen_subreg(machine_mode, rtx_def*, machine_mode,
> poly_int<2u, unsigned long>)
> /home/mtsamis/src/gcc/gcc4/gcc/rtl.h:3562
> 0x8afb4b force_subreg(machine_mode, rtx_def*, machine_mode,
> poly_int<2u, unsigned long>)
> /home/mtsamis/src/gcc/gcc4/gcc/explow.cc:755
> 0x8b9ef3 store_bit_field_1
> /home/mtsamis/src/gcc/gcc4/gcc/expmed.cc:810
> 0x8ba327 store_bit_field(rtx_def*, poly_int<2u, unsigned long>,
> poly_int<2u, unsigned long>, poly_int<2u, unsigned long>, poly_int<2u,
> unsigned long>, machine_mode, rtx_def*, bool, bool)
> /home/mtsamis/src/gcc/gcc4/gcc/expmed.cc:1214
> 0xc53c7f generate_bit_insert_sequence
> /home/mtsamis/src/gcc/gcc4/gcc/avoid-store-forwarding.cc:122
> 0xc53c7f process_store_forwarding
> /home/mtsamis/src/gcc/gcc4/gcc/avoid-store-forwarding.cc:231
> 0xc53c7f avoid_store_forwarding
> /home/mtsamis/src/gcc/gcc4/gcc/avoid-store-forwarding.cc:504
> 0xc546eb execute
> /home/mtsamis/src/gcc/gcc4/gcc/avoid-store-forwarding.cc:571
OK, thanks, I'll have a go at reproducing, but it sounds like it might
be tricky. :)
>> > So, although I don't like it and it complicates things, I had to add
>> > recog_memoized pretty much everywhere and also make sure to only emit
>> > if all of these are successful...
>>
>> Do you have testcases? emit_move_insn's job is to make the move
>> legitimate, so it seems like the checks might be papering over an issue
>> elsewhere.
>>
> I had a quick look and couldn't find an existing testcase. I'll have
> to recheck more thoroughly in order to create a testcase (if possible
> at all).
> I remember that in one case the rtx provided was zero_extend and there
> was no instruction for that so recog would fail.
Ah, ok. That might have been the bug then. emit_move_insn should only
be used for moving one object to another, where an "object" can be a
constant, memory or register (including subregs of registers).
The advantage of using it is that it works out how to make the move
valid, given target limitations.
If the instruction can be more general than that then we should just use
gen_rtx_SET, and recog the result.
Thanks,
Richard
