[PATCH] D101900: [InstCombine] Fold extractelement + vector GEP with one use

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:434
+    } else if (auto *GEP = dyn_cast<GetElementPtrInst>(I)) {
+      if (IndexC && GEP->hasOneUse() && GEP->getNumOperands() == 2) {
+        uint64_t IdxVal = IndexC->getZExtValue();
----------------
This condition feels a bit too restrictive, because it would avoid:

  %gep = getelementptr i32, i32** %ptr, i32 0, <2 x i32> <i32 4, i32 4>
  extractelement <2 x i32*> %gep, i32 0

to not be simplified.




================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:450
+          Value *NewPtr = nullptr, *NewIdx = nullptr;
+          if (VecType->getElementType() == GEP->getPointerOperandType()) {
+            assert(isa<VectorType>(IdxType) &&
----------------
You can probably avoid spelling out the three possibilities and just have a loop that extracts the operand if it's type is a VectorType, as long as you make sure only one of the operands is of type VectorType.


================
Comment at: llvm/test/Transforms/InstCombine/gep-vector-indices.ll:10-12
+entry:
+  %tmp = insertelement <2 x i64> poison, i64 4, i32 0
+  %splatof4 = shufflevector <2 x i64> %tmp, <2 x i64> poison, <2 x i32> zeroinitializer
----------------
nit: For fixed-width, this pattern is unnecessary and you can write `<2 x i64> <i64 4, i64 4>` directly in the GEP.


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

https://reviews.llvm.org/D101900