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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen 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);
----------------
CarolineConcatto wrote:
> 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?
By default VECREDUCE_SEQ_FADD is marked as 'Expand', which means for fixed-length vectors that it will expand the operation to a sequence of scalar options to do the vector reduction. For scalable vectors, no such scalarisation exists (because it would require SelectionDAG to generate a loop to do this), so the compiler will fail to compile. That is the expected behaviour, because if the target can't use these instruction due to the selected streaming-mode, then the intrinsic/operation should not have been formed in the LLVM IR in the first place.


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156109