Alex Coplan <alex.cop...@arm.com> writes:
> Hi,
>
> The PR shows us ICEing due to an unrecognizable TFmode save emitted by
> aarch64_process_components. The problem is that for T{I,F,D}mode we
> conservatively require mems to be in range for x-register ldp/stp. That
> is because (at least for TImode) it can be allocated to both GPRs and
> FPRs, and in the GPR case that is an x-reg ldp/stp, and the FPR case is
> a q-register load/store.
>
> As Richard pointed out in the PR, aarch64_get_separate_components
> already checks that the offsets are suitable for a single load, so we
> just need to choose a mode in aarch64_reg_save_mode that gives the full
> q-register range. In this patch, we choose V16QImode as an alternative
> 16-byte "bag-of-bits" mode that doesn't have the artificial range
> restrictions imposed on T{I,F,D}mode.
>
> For T{F,D}mode in GCC 15 I think we could consider relaxing the
> restriction imposed in aarch64_classify_address, as AFAIK T{F,D}mode can
> only be allocated to FPRs (unlike TImode). But such a change seems too
> invasive to consider for GCC 14 at this stage (let alone backports).
GPRs can hold all three, due to the way aarch64_hard_regno_mode_ok
is defined. (They can also hold individual Advanced SIMD vectors.)
But the ABI says that TFmode is passed in FPRs, so I agree that it
seems better to optimise for the FPR range. Same for TDmode.
> Fortunately the new flexible load/store pair patterns in GCC 14 allow
> this mode change to work without further changes. The backports are
> more involved as we need to adjust the load/store pair handling to cater
> for V16QImode in a few places.
>
> Note that for the testcase we are relying on the torture options to add
> -funroll-loops at -O3 which is necessary to trigger the ICE on trunk
> (but not on the 13 branch).
>
> Bootstrapped/regtested on aarch64-linux-gnu, OK for trunk?
>
> Thanks,
> Alex
>
> gcc/ChangeLog:
>
> PR target/111677
> * config/aarch64/aarch64.cc (aarch64_reg_save_mode): Use
> V16QImode for the full 16-byte FPR saves in the vector PCS case.
>
> gcc/testsuite/ChangeLog:
>
> PR target/111677
> * gcc.target/aarch64/torture/pr111677.c: New test.
OK, thanks.
Richard
