[PATCH] D124457: [AArch64][SVE] Only fold frame indexes referencing SVE objects into SVE loads/stores

Wed Apr 27 06:38:59 PDT 2022

bsmith added a comment.

In D124457#3476866 <https://reviews.llvm.org/D124457#3476866>, @bsmith wrote:

> In D124457#3474976 <https://reviews.llvm.org/D124457#3474976>, @efriedma wrote:
>
>> Is FrameIndex 0 actually reliably located in a position that doesn't have an offset?  That seems unlikely to me, especially if variable-size allocation becomes involved.
>
> Hm, you might be right. It seems a shame to regress cases where the index would be zero, but I'm not sure there is way to tell at this point if the offset will resolve to zero?
>
>> Alternatively, we could fix the code that decides whether we need an emergency spill slot. But I guess there's not really much point to attempting to fold the frame indexes in the first place.
>
> I considered that, the issue though is finding the correct condition to do this in without being far too conservative, I think you'd have to do it by traversing the function finding all SVE load/stores and checking what they access, which doesn't seem very nice for an issue we are creating for ourselves.

Thinking about this some more, in the interest of not regressing and given this isn't a correctness issue, perhaps we can leave it like this and 'assume' frame index zero has a zero offset, if it is not it will just get pulled back out like it does now (hence no correctness issue), it just may still be possible (although probably incredibly hard) to produce a crash due to lack of emergency spill slot. If it does turn out to be an issue still we can always come back and remove this assumption if necessary.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124457/new/

https://reviews.llvm.org/D124457