[PATCH] D143593: [InstCombine] Don't fold freeze poison when it's used in shufflevector

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

In D143593#4135928 <https://reviews.llvm.org/D143593#4135928>, @ManuelJBrito wrote:

> In D143593#4130108 <https://reviews.llvm.org/D143593#4130108>, @spatel wrote:
>
>> What if we squash any cast of freeze(poison) into the freeze itself as an addition to `getUndefReplacement()`:
>> https://alive2.llvm.org/ce/z/o7J2JS
>>
>> That would give us a far-fetched improvement on something like this:
>> https://alive2.llvm.org/ce/z/hC_Gws
>> ...so that could be an independent patch before the shuffle hack.
>
> I think this would work great! The main hurdle is that getUndefReplacement's users expect a Constant.
> In particular Constant::replaceUndefsWith, so I would have to change the return type of getUndefReplacement to Value and also change Constant::replaceUndefsWith to a take a Value as a Replacement.
> I'm not sure which option is more desirable, the current patch or your suggestion.

Yes, this is a smaller patch. It's contained enough that it probably doesn't matter either way, so LGTM.



================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3968
 
+static bool isUsedWithinShuffleVector(Value *V) {
+  for (auto *U : V->users()) {
----------------
Add a block comment for this function with something like:
  /// Check if any direct or bitcast user of this value is a shuffle instruction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143593