[PATCH] D135324: [AArch64-SVE]: force using SVE in streaming mode to lower arithmetic and logical fixed-width vector ops.

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:15744-15750
+  if (VT.isFixedLengthVector() && Subtarget->forceStreamingCompatibleSVE()) {
+    // convert fixed-length vector to scalable one:
+    EVT ScalableContainerVT = getContainerForFixedLengthVector(DAG, VT);
+    convertToScalableVector(DAG, ScalableContainerVT, LHS);
+    convertToScalableVector(DAG, ScalableContainerVT, RHS);
+    return performSVEAndCombine(N, DCI);
+  }
----------------
This looks odd.  We shouldn't really be doing lower within DAGCombine.  What happens if you just exit the combine for the invalid case?

That said, I can see functions like `tryAdvSIMDModImm32()` are used in other part of codegen so I'm wondering if the prevention logic is best place within such functions so all use cases are covered.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:22383
+  assert(useSVEForFixedLengthVectorVT(
+             VT, Subtarget->forceStreamingCompatibleSVE()) &&
          "Only expected to lower fixed length vector operation!");
----------------
When we hit a similar issue with `LowerToPredicatedOp()` we decide to drop the calls to `useSVEForFixedLengthVectorVT()` in favour or just using `VT.isFixedLengthVector() && isTypeLegal(VT)`.  Would the same work in your case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135324