[PATCH] D89576: [SVE][CodeGen] Lower scalable masked scatters

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

Just a couple of general observations as I'm not sure if I'll get chance to properly review before the patch is accepted. I'm not expecting any changes based on these comments as they're effectively refactoring and thus getting to a state where MGATHER/MSCATTER works for SVE is a better starting point for such refactoring.

1. I cannot help but think the conversion from MSCATTER to SVE intrinsic is backwards and will limit us in the future, or at least cause us to move code into lowering that probably doesn't belong.
2. I want to restrict when and where AArch64ISD::REINTERPRET_CAST is used.  You can see from the patterns you've added that some alias to ISD::BITCAST and ultimately allowing every combination will require a lot of patterns.  Instead I plan to restrict AArch64ISD::REINTERPRET_CAST to only allow casting between vectors of the same element type.
3. There's duplication between our enum representing SCALED and the MSNode's Scale operand.  Personally I don't know if there's any call for an arbitrary Scale and it causes unnecessary (for AArch64 at least) complication.

Again, I'm not expecting any changes from you but just wanted to let you know my thinking for the future.


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

https://reviews.llvm.org/D89576