[PATCH] D150923: [KnownBits] Factor out and improve the lowbit computation for {u,s}div

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Support/KnownBits.cpp:545
+
+  // If we already have a constant, we can skip this.
+  unsigned BitWidth = LHS.getBitWidth();
----------------
What does the comment mean? What are we skipping?


================
Comment at: llvm/lib/Support/KnownBits.cpp:547
+  unsigned BitWidth = LHS.getBitWidth();
+  KnownBits KnownOut(Known);
+  unsigned LHSMinTZ = LHS.countMinTrailingZeros();
----------------
Seems weird to pass this in by const ref and then immediately copy it. Why not pass it in by value?


================
Comment at: llvm/lib/Support/KnownBits.cpp:551-562
+  // Odd / X -> Odd
+  //    X must be odd, otherwise the div is not exact.
+  // EvenX / EvenY [if TZ(X) == TZ(Y)] -> Odd
+  if (LHS.One[LHSMinTZ] &&
+      (LHSMinTZ == 0 ||
+       (RHS.countMinTrailingZeros() == LHSMinTZ && RHS.One[LHSMinTZ])))
+    KnownOut.One.setBit(0);
----------------
Can I suggest a slightly more unified way of handling both of these cases?
```
  int MinTZ = (int)LHS.countMinTrailingZeros() - (int)RHS.countMaxTrailingZeros();
  int MaxTZ = (int)LHS.countMaxTrailingZeros() - (int)RHS.countMinTrailingZeros();
  if (MinTZ >= 0) {
    // Result has at least MinTZ trailing zeros.
    KnownOut.Zero.setLowBits(MinTZ);
    if (MinTZ == MaxTZ) { // might also need to check this is < BitWidth ???
      // Result has exactly MinTZ trailing zeros.
      KnownOut.One.setBit(MinTZ);
    }
  }
```


================
Comment at: llvm/lib/Support/KnownBits.cpp:635
 
-  if (Exact) {
-    // Odd / Odd -> Odd
-    if (LHS.One[0] && RHS.One[0]) {
-      Known.Zero.clearBit(0);
-      Known.One.setBit(0);
-    }
-    // Even / Odd -> Even
-    else if (LHS.Zero[0] && RHS.One[0]) {
-      Known.One.clearBit(0);
-      Known.Zero.setBit(0);
-    }
-    // Odd / Even -> impossible
-    // Even / Even -> unknown
-  }
+  Known = divComputeLowBit(Known, LHS, RHS, Exact);
 
----------------
Nit: could use intersectWith instead of passing in Known.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150923