[PATCH] D148120: [InstCombine] Remove scalable get_active_lane_mask calls which are always false

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3074
+      break;
+    if (BO->getOpcode() != Instruction::Add || !BO->hasNoSignedWrap() ||
+        !BO->hasNoUnsignedWrap())
----------------
I think in reality the add of an induction variable generated by the loop vectoriser won't have the nsw and nuw flags on it so this optimisation won't trigger in practice. I don't think we need to require these flags though for your optimisation to work. You should just be able to check the start value for the PHI is 0, because you know it cannot wrap on the first iteration. This is typically the canonical form for vectoriser output as well I think.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3079
+    ConstantInt *PhiOp0 = dyn_cast<ConstantInt>(L);
+    if (!PhiOp0)
+      break;
----------------
We should also bail out if the it's not zero here I think.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3091
+    uint64_t MinVScaleElts = VScaleMin * ShlValue->getZExtValue();
+    if (PhiOp0->getZExtValue() + MinVScaleElts < Op1->getZExtValue() ||
+        MinVScaleElts < Op1->getZExtValue())
----------------
By bailing out earlier for non-zero values of PhiOp0 you can simplify this to:

  if (MinVScaleElts < Op1->getZExtValue())
    break;


================
Comment at: llvm/test/Transforms/InstCombine/get-active-lane-mask.ll:42
+  tail call void @llvm.masked.store.nxv16i8.p0(<vscale x 16 x i8> %1, ptr %2, i32 1, <vscale x 16 x i1> %active.lane.mask)
+  %index.next = add nuw nsw i64 %index, %vf
+  %active.lane.mask.next = tail call <vscale x 16 x i1> @llvm.get.active.lane.mask.nxv16i1.i64(i64 %index.next, i64 4)
----------------
It would be good to test this without the nuw and nsw flags, since that's what the vectoriser generates.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148120