[PATCH] D69387: [ConstantRange] Add toKnownBits() method

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/IR/ConstantRange.cpp:77
 
+KnownBits ConstantRange::toKnownBits(const ConstantRange &Range) {
+  KnownBits Known(Range.getBitWidth());
----------------
Any reason not to make this an instance method, i.e. `CR.toKnownBits()`. We only need the static method for `fromKnownBits()` because we're working on a non-ConstantRange type.


================
Comment at: llvm/lib/IR/ConstantRange.cpp:103
+      return Bit;
+    }
+    llvm_unreachable("Must have found bit mismatch.");
----------------
Rather than looping over bits, which seems rather expensive to me, I'd suggest expressing this in terms of `(V0 ^ V1).countLeadingZeros()`.


================
Comment at: llvm/lib/IR/ConstantRange.cpp:117
+  Known.One.clearLowBits(*Bit);
+  Known.Zero.clearLowBits(1 + *Bit);
+
----------------
Why is it necessary to clear one more bit in the Zero case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69387