[PATCH] D93817: [InstCombine] Update transformations to use poison for insertelement/shufflevector's placeholder value (2/2)

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:1221
       if (!isa<UndefValue>(I->getOperand(1))) {
-        I->setOperand(1, UndefValue::get(I->getOperand(1)->getType()));
         MadeChange = true;
----------------
This change is okay because it is splat (only the first vector of shufflevector was accessed)


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1344
       }
     }
     // Create new operands for a shuffle that includes the constant of the
----------------
`Values` is the RHS of a new shufflevector below.
The poison elements are unused elems, so okay to be poison.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1441
         if (LR.second == nullptr)
-          LR.second = UndefValue::get(LR.first->getType());
+          LR.second = PoisonValue::get(LR.first->getType());
         return new ShuffleVectorInst(LR.first, LR.second, Mask);
----------------
When LR.second was nullptr, it was simply a placeholder (so okay to be poison)


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:2355
     }
-    return new ShuffleVectorInst(LHS, UndefValue::get(RHS->getType()), Elts);
+    return new ShuffleVectorInst(LHS, PoisonValue::get(RHS->getType()), Elts);
   }
----------------
As seen in line 2353, Elts' indices are smaller than LHSWidth, so okay to be poison.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:551
+///    %2  = shufflevector <2 x float> %1, poison, <0, 1, poison, poison>
+///    %i2 = shufflevector <4 x float> %A, %2,    <0, 1, 4, 5>
 static void replaceExtractElements(InsertElementInst *InsElt,
----------------
Copied from https://github.com/llvm/llvm-project/commit/ae945e7927e3a38cc3a44e829bc158c1ce5602ad


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:602
   auto *WideVec =
-      new ShuffleVectorInst(ExtVecOp, UndefValue::get(ExtVecType), ExtendMask);
+      new ShuffleVectorInst(ExtVecOp, PoisonValue::get(ExtVecType), ExtendMask);
 
----------------
Okay to be poison (the added comment above replaceExtractElements has details)


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1132
 
-  return new ShuffleVectorInst(FirstIE, UndefVec, Mask);
+  return new ShuffleVectorInst(FirstIE, PoisonVec, Mask);
 }
----------------
Since this is creating a splat, this shufflevector change and the insertelement change above is okay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93817