[PATCH] D130433: [InstCombine] Add fold for redundant sign bits count comparison

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

I think this patch is almost ready - just a few more suggestions:

1. Update the patch description with links to the general Alive2 proofs that model the coded implementation (make sure the pre-conditions match the code).

2. Add at least one more negative test to make sure this transform only happens with "ashr":

  define i1 @wrong_shift_opcode_i8(i8 %x) {
    %y = lshr i8 %x, 5
    %z = xor i8 %y, %x
    %c = icmp ult i8 %z, 2
    ret i1 %c
  }



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:1687-1688
+  Type *XType = X->getType();
+  unsigned BitWidth = XType->getScalarSizeInBits();
+  if (Shift == 0 || PowerOf2.logBase2() > BitWidth - 2)
+    return nullptr;
----------------
The compare with BitWidth is equivalent to "PowerOf2.isNegative()" or "PowerOf2.isMinSignedValue()" - I'd prefer one of those for readability/efficiency.

Also, please add a code comment that describes the transform:
  // For power-of-2 C:
  // ((X s>> ShiftC) ^ X) u< C --> (X + C) u< (C << 1)

(and similar for the "u>" variant)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130433/new/

https://reviews.llvm.org/D130433