[llvm] [HashRecognize] Fix LHS ConstantRange check for BE (PR #148620)

[Piotr Fusik via llvm-commits]

================
@@ -198,35 +199,38 @@ KnownBits ValueEvolution::computeInstr(const Instruction *I) {
                         m_Instruction(FV)))) {
     Visited.insert(cast<Instruction>(I->getOperand(0)));
 
-    // We need to check LCR against [0, 2) in the little-endian case, because
-    // the RCR check is insufficient: it is simply [0, 1).
-    if (!ByteOrderSwapped) {
-      KnownBits KnownL = compute(L);
-      unsigned ICmpBW = KnownL.getBitWidth();
-      auto LCR = ConstantRange::fromKnownBits(KnownL, false);
-      auto CheckLCR = ConstantRange(APInt::getZero(ICmpBW), APInt(ICmpBW, 2));
-      if (LCR != CheckLCR) {
-        ErrStr = "Bad LHS of significant-bit-check";
-        return {BitWidth};
-      }
+    // Check that the predication is on (most|least) significant bit.
+    KnownBits KnownL = compute(L);
+    unsigned ICmpBW = KnownL.getBitWidth();
+    auto LCR = ConstantRange::fromKnownBits(KnownL, false);
+    // Check LCR against full-set, [0, -1), [0, -3), [0, -7), etc. depending on
+    // AtIter in the big-endian case, and against [0, 2) in the little-endian
+    // case.
+    auto CheckLCR = ConstantRange::getNonEmpty(
+        APInt::getZero(ICmpBW), ByteOrderSwapped
+                                    ? -APInt::getLowBitsSet(ICmpBW, AtIter)
+                                    : APInt(ICmpBW, 2));
+    if (LCR != CheckLCR) {
+      ErrStr = "Bad LHS of significant-bit-check";
+      return {BitWidth};
     }
 
-    // Check that the predication is on (most|least) significant bit.
     KnownBits KnownR = compute(R);
-    unsigned ICmpBW = KnownR.getBitWidth();
     auto RCR = ConstantRange::fromKnownBits(KnownR, false);
     auto AllowedR = ConstantRange::makeAllowedICmpRegion(Pred, RCR);
-    ConstantRange CheckRCR(APInt::getZero(ICmpBW),
-                           ByteOrderSwapped ? APInt::getSignedMinValue(ICmpBW)
-                                            : APInt(ICmpBW, 1));
+    // Check AllowedR against [0, smin) in the big-endian case, and against
+    // [0, 1) in the little-endian case.
+    ConstantRange CheckAllowedR(
+        APInt::getZero(ICmpBW),
+        ByteOrderSwapped ? APInt::getSignedMinValue(ICmpBW) : APInt(ICmpBW, 1));
----------------
pfusik wrote:

Here we check that the compare is `>= 0` (big-endian) or `== 0` (little-endian).
That is, `CheckAllowedR` is for the significant bit _clear_, which means we visit the shift branch instead of the shift-and-xor-poly branch.
This explains that `CheckLCR` checks:
- if we are shifting zero bits in every loop iteration (big-endian)
- compare 0 or 1 on the LHS (little-endian) - I understand the intent is to match `(x & 1)`, but we don't seem to check what is `x`

It took me a while to understand the above - please improve the comments and variable names.
For example, `AllowedR` isn't really descriptive. How about `AllowedForL` or `AllowedByR` ?

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