[PATCH] D45862: [InstCombine] Relax restriction in foldSelectInstWithICmp for sake of smaller code size

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

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

> Just for note: I am not aware of any asm advantages or disadvantages of what we have now in practice. I am only speculating about it in assumption that this bit-test magic was done with some reason. I don't have any real evidence that it is somehow good or bad in asm, but what I know is that does pessimize three-way comparison recognition.


I forgot about this, but I think I asked about the same problem as PR37147 here:
http://lists.llvm.org/pipermail/llvm-dev/2017-July/114885.html

...and the conclusion was that we want selects (or at least nobody strongly opposed that). And so I went to the DAG and made the necessary changes to allow producing good asm for all targets for those patterns (ie, there's a hook to convert those back to bit math). So there should be little risk in removing/inverting the instcombines at this point.

I'm looking closer now at how to replace some of the instcombines now without regressing anything. I expect this is going to take multiple steps to unravel. For example, we even have a 'm_Signum' matcher that's looking for shifts.


https://reviews.llvm.org/D45862