[PATCH] D115724: [InstCombine] Fold for masked scatters to a uniform address

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:378
+  // uniform store and can be folded.
+  if (ConstMask->isAllOnesValue())
+    if (auto *Splat = getSplatValue(II.getArgOperand(1))) {
----------------
I think you can distinguish two cases here:
1. The pointer is a splat(), the value is not a splat, the mask is an all-active mask.
2. The pointer is a splat(), the value is a splat(), the mask is not all-inactive (but not necessarily all active).

For 2, you can extract any lane instead of the last lane.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:379
+  if (ConstMask->isAllOnesValue())
+    if (auto *Splat = getSplatValue(II.getArgOperand(1))) {
+      Align Alignment = cast<ConstantInt>(II.getArgOperand(2))->getAlignValue();
----------------
What is operand 1? Can you change `Splat` to something more descriptive? e.g. `SplatPtr` ?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:381
+      Align Alignment = cast<ConstantInt>(II.getArgOperand(2))->getAlignValue();
+      VectorType *WideLoad =
+          dyn_cast<VectorType>(II.getArgOperand(1)->getType());
----------------
nit: WideLoadTy ?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:382
+      VectorType *WideLoad =
+          dyn_cast<VectorType>(II.getArgOperand(1)->getType());
+      ElementCount VF = WideLoad->getElementCount();
----------------
this must be `cast<VectorType>`


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:386
+          ConstantInt::get(Builder.getInt32Ty(), VF.getKnownMinValue());
+      Value *RunTimeVF = Builder.CreateVScale(EC);
+      // LastLane = RunTimeVF - 1
----------------
This is only valid if the the type is a `ScalableVectorType`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115724