[llvm] [InstCombine] Prefer to keep power-of-2 constants when combining ashr exact and slt/ult of a constant (PR #86111)

[Alex Bradbury via llvm-commits]

================
@@ -2482,10 +2482,21 @@ Instruction *InstCombinerImpl::foldICmpShrConstant(ICmpInst &Cmp,
   // those conditions rather than checking them. This is difficult because of
   // undef/poison (PR34838).
   if (IsAShr && Shr->hasOneUse()) {
+    if (IsExact && (Pred == CmpInst::ICMP_SLT || Pred == CmpInst::ICMP_ULT) &&
+        !C.isMinSignedValue() && (C - 1).isPowerOf2()) {
+      // When C - 1 is a power of two and the transform can be legally
+      // performed, prefer this form so the produced constant is close to a
+      // power of two.
+      // icmp slt/ult (ashr exact X, ShAmtC), C
+      // --> icmp slt/ult (C - 1) << ShAmtC) -1
+      APInt ShiftedC = (C - 1).shl(ShAmtVal) + 1;
+      return new ICmpInst(Pred, X, ConstantInt::get(ShrTy, ShiftedC));
----------------
asb wrote:

Thank you for taking a look. I wasn't aware of the InstCombine contributors guide when first writing this patch (it was committed soon after) and it seems I fell into some traps it warned against.

I do however have generalised proofs now:
<https://alive2.llvm.org/ce/z/2BmPnq>
<https://alive2.llvm.org/ce/z/DtuhnR>

Note that the power of two precondition isn't added in those proofs as it's not relevant for correctness, just for whether we opt to do this transformation. The key precondition is ensure we don't overflow into the sign bit which I believe my implementation fails to do as it doesn't have an equivalent of ` if (ShiftedC.ashr(ShAmtVal) == C)`.

Could you please check you're happy those proofs demonstrate generalized correctness (assuming a properly guarded transform)? Thanks.

https://github.com/llvm/llvm-project/pull/86111