[PATCH] D159191: [DAGCombiner][SVE] Add support for illegal extending masked loads

Paul Walker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 1 05:20:57 PDT 2023

paulwalker-arm added a comment.

Is there value in adding a single fixed length test?

Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5353
+  if (auto *Ld = dyn_cast<MaskedLoadSDNode>(ExtVal->getOperand(0))) {
+    if (!isLoadExtLegalOrCustom(ISD::ZEXTLOAD, ExtVT, Ld->getValueType(0))) {
+      unsigned NumExtMaskedLoads = 0;
Is this strictly necessary? Perhaps it is but I'm wondering if you're really just trying to limit this combine to cases where ExtVT is bigger than legal?

Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5356
+      for (auto *U : Ld->getMask()->uses())
+        NumExtMaskedLoads += isa<MaskedLoadSDNode>(U) ? 1 : 0;
Up to you but personally I think a straight forward:
if (isa<MaskedLoadSDNode>(U))
is cleaner.

Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5363
+  return ExtVT.isScalableVector() || Subtarget->useSVEForFixedLengthVectors();
I think this check probably want to be first so that we only start processing ExtVal when we know extending loads are a possibility.



More information about the llvm-commits mailing list