[PATCH] D39421: [InstCombine] Extracting common and-mask for shift operands of Or instruction

[Omer Paparo Bivas via Phabricator via llvm-commits]

opaparo added a comment.

In https://reviews.llvm.org/D39421#945064, @spatel wrote:

> Both this and https://reviews.llvm.org/D38037 are trying to start a pattern match with an 'or', but I'm curious if there's a 'trunc' in the larger source that creates these patterns? Either way, we're missing something bigger than patterns that start with 'or'.
>
> For example, I was looking at PR31667:
>  https://bugs.llvm.org/show_bug.cgi?id=31667
>
> Name: sub_mask_shift
>
>   %and1 = lshr i32 %x, 3
>   %shr1 = and i32 %and1, 8191
>   %and2 = lshr i32 %x, 1
>   %shr2 = and i32 %and2, 32767
>   %r = sub i32 %shr1, %shr2
>   
>
> ...which was filed as a backend bug, but we wouldn't handle that in IR either: 
>  https://rise4fun.com/Alive/id4
>
> So I think there's some more general sequence that we want to capture and optimize, but it may be difficult to justify as part of instcombine?
>
> Note that there is a proposal for a new pass where all of these might find a home:
>  https://reviews.llvm.org/D38313


(Now abandoned) https://reviews.llvm.org/D38037 was a the first draft of this review.
There is no 'trunc' in the larger source that creates these patterns. My patch may address these issues you mentioned, but it is not directly related to them.



================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2288
+      if (ShiftBy0 == ShiftBy1 && ShiftOpcode0 == ShiftOpcode1) {
+        // ((V<<C3)&C1) | ((V<<C3)&C2) --> (V<<C3)&(C1|C2)
+        unsigned SavedInstructions = Op0->hasOneUse() + Op1->hasOneUse() +
----------------
spatel wrote:
> Why are we looking for a pattern that early-cse can simplify? I think this is beyond the scope of instcombine.
Please check out the newly added tests multiuse4 and multiuse5.
I believe they illustrate that this is indeed an instcombine transformation.


Repository:
  rL LLVM

https://reviews.llvm.org/D39421