[PATCH] D147533: [AArch64][CodeGen][SME] Allow vectors large than hardware support to use SVE's load zero/sign-extend for fixed vectors

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5223
 bool AArch64TargetLowering::isVectorLoadExtDesirable(SDValue ExtVal) const {
+  bool OverrideNEON = Subtarget->useSVEForFixedLengthVectors();
   return ExtVal.getValueType().isScalableVector() ||
----------------
david-arm wrote:
> Hi @dtemirbulatov, I personally feel this patch should be only trying to fix one thing at a time, but it seems to be trying to change both SVE fixed-length behaviour as well as streaming SVE behaviour. Perhaps it's better to focus on just the streaming SVE case for now, in which case I think you can more simply write this as:
> 
>   bool OverrideNEON = Subtarget->useSVEForFixedLengthVectors() || Subtarget->forceStreamingCompatibleSVE();
>   return ExtVal.getValueType().isScalableVector() ||
>          useSVEForFixedLengthVectorVT(
>              ExtVal.getValueType(), OverrideNEON);
> ```bool OverrideNEON = Subtarget->useSVEForFixedLengthVectors() || Subtarget->forceStreamingCompatibleSVE();```
@david-arm just pointing out that if forceStreamingCompatibleSVE() is true, then useSVEForFixedLengthVectors() is also true, so your suggestion is actually equivalent to the original code.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5224-5227
   return ExtVal.getValueType().isScalableVector() ||
-         useSVEForFixedLengthVectorVT(
-             ExtVal.getValueType(),
-             /*OverrideNEON=*/Subtarget->useSVEForFixedLengthVectors());
+         useSVEForFixedLengthVectorVT(ExtVal.getValueType(), OverrideNEON) ||
+         (OverrideNEON && ExtVal.getValueType().getFixedSizeInBits() >
+                              Subtarget->getMinSVEVectorSizeInBits());
----------------
When I change this code to:

  return ExtVal.getValueType().isScalableVector() || Subtarget->useSVEForFixedLengthVectors();

all the tests still pass. That suggests that the test for 'ExtVal.getSizeInBits() > getMinSVEVectorSizeInBits()' is not used.


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

https://reviews.llvm.org/D147533