[PATCH] D50992: [InstCombine] try to fold insertelt + vector op into scalar op + insertelt

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

Note: this patch is on hold pending backend enhancements that try to avoid known codegen regressions.
The first 2 of those are https://reviews.llvm.org/D51186 and https://reviews.llvm.org/D51125.



================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:1362
+  ConstantInt *Idx;
+  bool ConstOp1 = isa<Constant>(Inst.getOperand(1));
+  if (match(&Inst, m_c_BinOp(
----------------
lebedev.ri wrote:
> I'd prefer `ConstIsOp1`
Yes, will fix.


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:1372
+    Value *ScalarOp = Builder.CreateBinOp(Opcode, NewLHS, NewRHS);
+    if (auto *ScalarBO = dyn_cast<BinaryOperator>(ScalarOp))
+      ScalarBO->copyIRFlags(&Inst);
----------------
lebedev.ri wrote:
> Hm, why `dyn_cast<>()`, we did just use `CreateBinOp()`?
CreateBinOp() will return a Constant if it manages to constant fold, so we can't be sure that 'ScalarOp' is actually a BinaryOperator. I suppose that would mean X was a constant...which seems unlikely. But it's the kind of thing a fuzzer finds N months after commit. :)


https://reviews.llvm.org/D50992