[PATCH] D34184: [InstCombine] Teach foldSelectICmpAndOr to recognize (select (icmp slt (trunc (X)), 0), Y, (or Y, C2))

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D34184#781467, @craig.topper wrote:

> I think you're right. The 'icmp' and the 'or' pieces aren't checked for single use. In the existing code we do reuse the 'and'. In my new code i did make sure the trunc only had one use so that putting back an 'and' doesn't increase anything. So in the existing code we potentially create an 'or' and possibly a 'shift' and only delete a 'select'. And the really bad case we create an 'xor', an 'or', and a 'shift' while only removing a 'select'. This is kinda weird to handle because we don't always create a shift and don't always create an xor.


Yeah, it's a mess. I think the safe thing would be:

1. Add tests with extra uses to show how this can go bad.
2. Check one-use on everything to avoid that.
3. Make the enhancement proposed in this patch.
4. Separate out the various cases and remove one-use if possible (I think this would be a rare enhancement, so probably not too important...but I don't have any actual data).


https://reviews.llvm.org/D34184