[PATCH] D133433: [AArch64-SVE]: lower masked load/store of 128-bit fixed-width vectors

Thu Sep 8 16:21:04 PDT 2022

Matt added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:436
+bool AArch64Subtarget::useSVEForFixedLengthVectors() const {
+  if (ForceSVEFor128bitVectors)
+    return hasSVE();
----------------
paulwalker-arm wrote:
> This is going to enable a lot more code paths than it currently tested.  Can you explain the rational for the new flag? Does it relate to SME's streaming-compatible mode? or it is wanted for other reasons?
> This is going to enable a lot more code paths than it currently tested.

Out of curiosity, I've run a quick check for a potentially related issue, https://github.com/llvm/llvm-project/issues/56412

I'm no longer encountering the ICE when compiling "sve-fixed-length-masked-gather.ll" with this option enabled, as in `llc --force-sve-when-streaming-compatible -aarch64-sve-vector-bits-min=128 -mtriple=arm64-unknown-unknown -mcpu=neoverse-n2 sve-fixed-length-masked-gather.ll`.

At the same time, there's no impact on the generated code (identical assembly); in any case, that's strictly better than ICE.

@hassnaa-arm, I'm wondering, just to be on the safe side, could you possibly run a quick check on your end to make sure that you're not encountering any issues with "sve-fixed-length-masked-gather.ll", either? That, and perhaps even add a RUN line with `llc --force-sve-when-streaming-compatible -aarch64-sve-vector-bits-min=128` to that file (as it has caused an ICE with 128-bit SVE compilation before), if that would be no trouble?

@paulwalker-arm, If this patch gets accepted and the above test works fine perhaps that would offer a way to close https://github.com/llvm/llvm-project/issues/56412?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133433