[PATCH] D45862: [InstCombine] Remove decanonicalizing transforms of selects

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D45862#1077652, @mkazantsev wrote:

> Let's try an alternative approach. In this patch we remove harmful transforms that replace selects with random pieces of bit magic.
>
> Here we only stop harmful transforms. The patch that will canonicalize all this bit magic to selects will go separately.


I agree with this direction (and I have a patch that would do something similar), but this is going too far. We should remove the transforms that are harmful, but without creating known regressions. This requires writing new, more specific folds as we remove the over-reaching logic.

As you can see from test15g and test15h, there are good folds buried in here. Those correspond to something like this:

  Name: set_bit_if_masked_val_is_clear
  Pre: isPowerOf2(C1) && ((C2 ^ C3) == C1) && (C2 u> C3)
   %t1 = and i8 %x, C1
   %t2 = icmp eq i8 %t1, 0
   %r = select i1 %t2, i8 C2, i8 C3
  =>
   %r = xor i8 %t1, C2
  
  Name: set_bit_if_masked_val_is_set
  Pre: isPowerOf2(C1) && ((C2 ^ C3) == C1) && (C2 u> C3)
   %t1 = and i8 %x, C1
   %t2 = icmp ne i8 %t1, 0
   %r = select i1 %t2, i8 C2, i8 C3
  =>
   %r = or i8 %t1, C3
  
  Name: clear_bit_if_masked_val_is_clear
  Pre: isPowerOf2(C1) && ((C2 ^ C3) == C1) && (C3 u> C2)
   %t1 = and i8 %x, C1
   %t2 = icmp eq i8 %t1, 0
   %r = select i1 %t2, i8 C2, i8 C3
  =>
   %r = or i8 %t1, C2
   
  Name: clear_bit_if_masked_val_is_set
  Pre: isPowerOf2(C1) && ((C2 ^ C3) == C1) && (C3 u> C2)
   %t1 = and i8 %x, C1
   %t2 = icmp ne i8 %t1, 0
   %r = select i1 %t2, i8 C2, i8 C3
  =>
   %r = xor i8 %t1, C3

https://rise4fun.com/Alive/XG7

To verify that we're doing this properly, we need to add more tests for patterns like this. The current set isn't providing very good coverage.


https://reviews.llvm.org/D45862