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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D50992#1212764, @spatel wrote:

> 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.


Yep, i noticed, thank you for doing these!



================
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);
----------------
spatel wrote:
> 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. :)
Right, constant-folding //again//. I should try to keep that in mind.


https://reviews.llvm.org/D50992