[PATCH] D49229: [InstCombine] Fold redundant masking operations of shifted value

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2838
+
+  for (auto OtherUserV : MaskedValue->users()) {
+    BinaryOperator *OtherUser = dyn_cast<BinaryOperator>(OtherUserV);
----------------
lebedev.ri wrote:
> Is this even for instcombine?
> I wonder if this should be aggressiveinstcombine, or something else?
Right - you'll notice that in my potential suggestions in D48278, I did *not* include instcombine. I think this has the same problem as that patch, but when you do it in instcombine, it gets multiplied by another 8x factor because we run instcombine so many times in the normal opt pass pipelines.

In general, we don't walk user lists in instcombine because that's not efficient (same concern that @efriedma raised in the other patch I think). On top of that, this patch is using recursive value tracking which is also expensive.

I suggest looking at (new)-gvn or early-cse to see if we can find the redundant op efficiently.


Repository:
  rL LLVM

https://reviews.llvm.org/D49229