[PATCH] D44266: [InstCombine] remove use restriction for min/max with not operands (PR35875)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D44266#1041299, @dmgreen wrote:

> Before I went away I ran a load of benchmarks. They eventually finished and showed no regressions except for this rgb to cmy case (which went up 48% on v6m targets!)


Thanks for verifying that this change does what we intended on the motivating case (and does no noticeable harm otherwise).

> If this is unpalatable because it can increase instruction count, we may be able to do something in IsFreeOrProfitableToInvert in InstCombineSelect. Is checking the users in InstCombine (more than just counting them) not a done thing?

It is done sometimes, but I view that code construct with suspicion. It's usually just covering for a misplaced pattern match. Ie, in this case it wouldn't be much different than starting a larger match from a sub.

I still don't know how we should handle this, but staring at a bit more, I see that this is just an application of DeMorgan's Law. So if we proceed down this path, we also want to loosen the use restrictions to allow:

  Name: not_not_and_sub_with_not
  %nx = xor i8 %x, -1
  %ny = xor i8 %y, -1
  %nz = xor i8 %z, -1
  %a = and i8 %nx, %ny
  %s = sub i8 %a, %nz
    =>
  %o = or i8 %x, %y
  %s = sub i8 %z, %o

https://rise4fun.com/Alive/cvv
...or if we decide to use larger matches starting from the sub, we'd want to match basic logic ops as well as min/max.


https://reviews.llvm.org/D44266