[PATCH] D79171: [InstCombine] canonicalize bitcast after insertelement into undef

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 6 08:02:57 PDT 2020


spatel marked 4 inline comments as done.
spatel added a subscriber: ctetreau.
spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:2487-2488
   if (VectorType *DestVTy = dyn_cast<VectorType>(DestTy)) {
-    if (DestVTy->getNumElements() == 1 && !SrcTy->isVectorTy()) {
+    // Beware: messing with this target-specific oddity will cause trouble.
+    if (DestVTy->getNumElements() == 1 && SrcTy->isX86_MMXTy()) {
       Value *Elem = Builder.CreateBitCast(Src, DestVTy->getElementType());
----------------
lebedev.ri wrote:
> Am i reading this correctly that this is NFC in context of rG5ebbabc1af360756f402203ba7704bb480f279a7?
> Can this be split off?
It's not quite NFC - this code diff results in the test diff in llvm/test/Transforms/InstCombine/bitcast-vec-canon.ll test "@c". That does seem like an independent improvement though, so I can break that into a preliminary commit.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1035
+    Type *ScalarTy = ScalarSrc->getType();
+    Type *VecTy = VectorType::get(ScalarTy, IE.getType()->getElementCount());
+    UndefValue *NewUndef = UndefValue::get(VecTy);
----------------
lebedev.ri wrote:
> I guess this won't work for scalable vectors?
> Can we somehow just replace the elt type in `VecOp->getType()` instead?
I think this is safe for scalable vectors, so I better add a test. See similar construct annotated below - around line 1279.
cc @ctetreau 


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1279
           I->getType()->getScalarType(),
           cast<VectorType>(NewOps[0]->getType())->getElementCount());
       assert(NewOps.size() == 1 && "cast with #ops != 1");
----------------
Safe for scalable vectors?


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

https://reviews.llvm.org/D79171





More information about the llvm-commits mailing list