[PATCH] D28406: [InstCombine] icmp sgt (shl nsw X, C1), C0 --> icmp sgt X, C0 >> C1

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D28406#649696, @bryant wrote:

> I'm not seeing any test cases for nsw/nuw eq/ne. Should/Can they be added?


This is why I started with a minimal patch. :)
This patch does not intend to change any functionality for the nuw path, so either those tests already exist or should be added separately. The removal of the trailing check for shl nuw + icmp eq/ne could have been part of r292260, so I can remove that separately if you think that's better. Ie, we're now handling all of the nuw transforms in the earlier block.

Tests that change with this patch and are commented with "Known bits analysis turns this into an equality predicate" are effectively tests of nsw+eq/ne, but I can add explicit checks.



================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:1959
+      assert(C->lshr(*ShiftAmt).shl(*ShiftAmt) == *C &&
+             "Compare known true or false was not folded");
+      APInt ShiftedC = C->lshr(*ShiftAmt);
----------------
bryant wrote:
> Can this branch be merged with ugt?
> 
> ```
> if (Cmp.isEquality() || Pred == ICMP_UGT) {
>   assert(!Cmp.isEquality || C->lshr(*ShiftAmt).shl(*ShiftAmt) == *C &&
>              "Compare known true or false was not folded");
>   // do the transform
> }
> ```
It could, but I think this is easier to read, and I'd really prefer to leave refactoring for a follow-up if that seems worthwhile.


https://reviews.llvm.org/D28406