[PATCH] D135876: [InstCombine] Remove redundant splats in InstCombineVectorOps

[Matt Devereau via Phabricator via llvm-commits]

MattDevereau added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:2624-2625
+      dyn_cast<Instruction>(Builder.CreateBinOp(BinOp->getOpcode(), X, Y));
+  if (!NewBOI)
+    return nullptr;
+  NewBOI->copyIRFlags(BinOp);
----------------
spatel wrote:
> I prefer the form that I suggested (and was copied from nearby code):
>   if (auto *NewBOI = dyn_cast<Instruction>(NewBO))
>     NewBOI->copyIRFlags(&I);
> 
> Ie, it's better to continue here rather than give up. If we've somehow encountered a constant-folding opportunity, then we'll create the final expected (shuffled) constant right here instead of leaving it to some other transforms. 
> 
> (And again, I'm not sure how to write a regression test for this.)
Ah, sorry, I missed the fact that the flags would still propagate here when called from NewBOI. I'll update this with your suggestion.
I'm assuming that `NewBOI->copyIRFlags(&I);` here should be `NewBOI->copyIRFlags(NewBO);`? `&I` does not exist in this scope, and if you meant `&SVI` this would not copy the IR flags on the binops.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135876