Re: [PATCH v2] Target-independent store forwarding avoidance.

[Manolis Tsamis <manolis.tsamis@vrull.eu>]

On Sun, Jun 9, 2024 at 5:29 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 6/7/24 4:31 PM, Jeff Law wrote:
>
> >
> > I've actually added it to my tester just to see if there's any fallout.
> > It'll take a week to churn through the long running targets that
> > bootstrap in QEMU, but the crosses should have data Monday.
> The first round naturally didn't trigger anything because the option is
> off by default.  So I twiddled it to be on at -O1 and above.
>
> epiphany-elf ICEs in gen_rtx_SUBREG with the attached .i file compiled
> with -O2:
>
> > root@577c7458c93a://home/jlaw/jenkins/workspace/epiphany-elf/epiphany-elf-obj/newlib/epiphany-elf/newlib/libm/complex# epiphany-elf-gcc -O2 libm_a-cacos.i
> > during RTL pass: avoid_store_forwarding
> > ../../../..//newlib-cygwin/newlib/libm/complex/cacos.c: In function 'cacos':
> > ../../../..//newlib-cygwin/newlib/libm/complex/cacos.c:99:1: internal compiler error: in gen_rtx_SUBREG, at emit-rtl.cc:1032
> > 0x614538 gen_rtx_SUBREG(machine_mode, rtx_def*, poly_int<1u, unsigned long>)
> >         ../../..//gcc/gcc/emit-rtl.cc:1032
> > 0x614538 gen_rtx_SUBREG(machine_mode, rtx_def*, poly_int<1u, unsigned long>)
> >         ../../..//gcc/gcc/emit-rtl.cc:1030
> > 0xe82216 process_forwardings
> >         ../../..//gcc/gcc/avoid-store-forwarding.cc:273
> > 0xe82216 avoid_store_forwarding
> >         ../../..//gcc/gcc/avoid-store-forwarding.cc:489
> > 0xe82667 execute
> >         ../../..//gcc/gcc/avoid-store-forwarding.cc:558
>

Fixed in v3.
Since we're dealing with arbitrary modes we need to call
validate_subreg before doing gen_rtx_SUBREG. E.g. a SFMode to TIMode
subreg would fail there.
The code already handled not being able to use the subreg if recog
failed so only an additional check was needed. I have added a new
testcase function ssll_load_elim_2 that triggered this.

>
> ft32-elf ICE'd in bitmap_check_index at various optimization levels:
>
> > FAIL: execute/pr108498-2.c   -O1  (internal compiler error: in bitmap_check_index, at sbitmap.h:104)
> > FAIL: execute/pr108498-2.c   -O1  (test for excess errors)
> > FAIL: execute/pr108498-2.c   -O2  (internal compiler error: in bitmap_check_index, at sbitmap.h:104)
> > FAIL: execute/pr108498-2.c   -O2  (test for excess errors)
> > FAIL: execute/pr108498-2.c   -O3 -g  (internal compiler error: in bitmap_check_index, at sbitmap.h:104)
> > FAIL: execute/pr108498-2.c   -O3 -g  (test for excess errors)
>

Fixed in v3 as well.
The offending store had BLKMode and returned a store size of 0 which
could overflow it->offset + store_size - 1 among others. A new check
is added to reject any BLKMode set. I also added a check for sets that
have a memory source and destination as these wouldn't be handled
correctly.

>
> avr, c6x,
>
> lm32-elf failed to build libgcc with an ICE in leaf_function_p, I
> haven't isolated that yet.
>
>
> There were other failures as well.  But you've got a few to start with
> and we can retest pretty easily as the patch evolves.
>

The v3 also contains a fix for a memory corruption bug due to
stores.safe_push (stores[move_to_front]) having a self reference and
storing garbage when the push resulted in a resize. A testcase
function ssll_load_elim_3 that triggered this has been added.

Could you please run the v3 with your tester? I assume some of the
additional fixes introduced may clear some of the other issues.

Thanks,
Manolis

> jeff
>