On 5/14/24 08:44, Jeff Law wrote:
>>> I was able to find the summary info:
>>>
>>>> Tests that now fail, but worked before (15 tests):
>>>> libgomp: libgomp.fortran/simd7.f90 -O0 execution test
>>>> libgomp: libgomp.fortran/task2.f90 -O0 execution test
>>>> libgomp: libgomp.fortran/vla2.f90 -O0 execution test
>>>> libgomp: libgomp.fortran/vla3.f90 -O3 -fomit-frame-pointer -
>>>> funroll-loops -fpeel-loops -ftracer -finline-functions execution test
>>>> libgomp: libgomp.fortran/vla3.f90 -O3 -g execution test
>>>> libgomp: libgomp.fortran/vla4.f90 -O1 execution test
>>>> libgomp: libgomp.fortran/vla4.f90 -O2 execution test
>>>> libgomp: libgomp.fortran/vla4.f90 -O3 -fomit-frame-pointer -
>>>> funroll-loops -fpeel-loops -ftracer -finline-functions execution test
>>>> libgomp: libgomp.fortran/vla4.f90 -O3 -g execution test
>>>> libgomp: libgomp.fortran/vla4.f90 -Os execution test
>>>> libgomp: libgomp.fortran/vla5.f90 -O1 execution test
>>>> libgomp: libgomp.fortran/vla5.f90 -O2 execution test
>>>> libgomp: libgomp.fortran/vla5.f90 -O3 -fomit-frame-pointer -
>>>> funroll-loops -fpeel-loops -ftracer -finline-functions execution test
>>>> libgomp: libgomp.fortran/vla5.f90 -O3 -g execution test
>>>> libgomp: libgomp.fortran/vla5.f90 -Os execution test
>>> So if you could check on those, it'd be appreciated.
>> I checked rv64gcv linux and those do not currently run in CI.
> So just ran with Vineet's patch in our CI system. His patch is still
> triggering those regressions. So we need to get that resolved before
> that second patch can go in.
So CI pointed to a lone Fortran execute failure which is very likely
also causing above.
FAIL: gfortran.dg/PR93963.f90 -O0 execution test
Turns out the alloca codepath in epilogue expansion is simply busted -
I'm surprised that we only see 1 failure in the entire testsuite run
(libgomp run aside).
> - if (!SMALL_OPERAND (adjust_offset.to_constant ()))
> + HOST_WIDE_INT adj_off_value = adjust_offset.to_constant ();
> + if (SMALL_OPERAND (adj_off_value))
> + {
> + adjust = GEN_INT (adj_off_value);
> + }
> + else if (SUM_OF_TWO_S12_ALGN (adj_off_value))
> + {
> + HOST_WIDE_INT base, off;
> + riscv_split_sum_of_two_s12 (adj_off_value, &base, &off);
> + insn = gen_add3_insn (stack_pointer_rtx,
hard_frame_pointer_rtx,
> + GEN_INT (base));
1. Missing gen_insn( ) here causes the insn to be overwritten by the
subsequent emit_insn below.
2. In sum of two s12 logic first insn is sp = xx + 2048, with the
additional insn expected to be of form
sp = sp + off corresponding to
stack_pointer_rtx+ stack_pointer_rtx+ off
which the following emit_insn () below is not.
...
> insn = emit_insn ( gen_add3_insn (stack_pointer_rtx,
hard_frame_pointer_rtx,
> adjust));
3. Yet another issue had to do with which insn should the dwarf be
attached -and the adj needed adjusting :-)
So In v3 I've split this patch into the main prologue/epilogue change
and one from the alloca one - which depending on reviewer guidance
(aesthetics, ugliness, trying to keep uniformity etc ? can be decided upon.
Thx,
-Vineet
