[PATCH] D156109: [AArch64][SME] Create new interface for isFullSVEAvailable.

[Caroline via Phabricator via llvm-commits]

CarolineConcatto added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1454
+      if (Subtarget->isFullSVEAvailable())
+        setOperationAction(ISD::VECREDUCE_SEQ_FADD, VT, Custom);
       setOperationAction(ISD::VECTOR_SPLICE, VT, Custom);
----------------
Here you are also changing the logic.
Before for SVE FADD was custom, now it can be legal.
So it means we can lower to an assembly, but I don't see that anywhere.
I believe it will crash. Is that the expected behaviour?
I have the same question for line 1508.
I believe it will crash. Is that the expected behaviour?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:19532
     break;
   case Intrinsic::aarch64_sve_fadda:
+    if (Subtarget->isFullSVEAvailable())
----------------
This does not look right.
What will happen when isFullSVEAvailable() is no available?
Should it crash?



================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.h:219
+  /// Returns true if the target has SVE and can use the full range of SVE
+  /// instructions, for example becuase it knows the function is known not to be
+  /// in streaming-SVE mode, or when the target has FEAT_FA64 enabled (which
----------------
s/becuase/because/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156109