[PATCH] D103153: [InstCombine] Add fold for extracting known elements from a stepvector

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:351
+    if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(SrcVec)) {
+      if (Intrinsic::ID IID = II->getIntrinsicID()) {
+        switch (IID) {
----------------
You don't need this "if".


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:354
+        case Intrinsic::experimental_stepvector:
+          if (IndexC->getZExtValue() <= NumElts) {
+            auto *Idx = ConstantInt::get(
----------------
foad wrote:
> Should be "<" not "<=", surely?
`IndexC->getValue().ult(NumElts)` seems safer, since getZExtValue can truncate wide values.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:356
+            auto *Idx = ConstantInt::get(
+                cast<VectorType>(EI.getVectorOperandType())->getScalarType(),
+                IndexC->getZExtValue());
----------------
Don't need the case to VectorType since getScalarType is defined on Type.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:357
+                cast<VectorType>(EI.getVectorOperandType())->getScalarType(),
+                IndexC->getZExtValue());
+            return replaceInstUsesWith(EI, Idx);
----------------
Just use getValue here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103153