[PATCH] D29897: [BypassSlowDivision] Use ValueTracking to simplify run-time checks

[Nikolai Bozhenov via Phabricator via llvm-commits]

n.bozhenov added inline comments.


================
Comment at: lib/Transforms/Utils/BypassSlowDivision.cpp:194
 
+/// Check if an integer value fits into a smaller type.
+ValueRange FastDivInsertionTask::getValueRange(Value *Op,
----------------
jlebar wrote:
> Maybe, "check if an integer type fits into our bypass type"?
Not sure what you mean. Here we're testing not a type (how many bits it has?) but a value (may there be any non-zero upper bits?).


================
Comment at: lib/Transforms/Utils/BypassSlowDivision.cpp:195
+/// Check if an integer value fits into a smaller type.
+ValueRange FastDivInsertionTask::getValueRange(Value *Op,
+                                               const DataLayout &DL) {
----------------
jlebar wrote:
> Value *V ?  It's not necessarily an operator -- could be a constant or something.
Actually, Op was supposed to mean an operand here. But now I see it's ambiguous.


================
Comment at: lib/Transforms/Utils/BypassSlowDivision.cpp:201
+
+  APInt Zeros(LongLen, 0), Ones(LongLen, 0);
+
----------------
jlebar wrote:
> Should we assert that V has the same width as getSlowType()?  If its type is shorter I think it works out, but if it's longer, does it still work?
I have added an assert that type of the value is wider than BypassType.


https://reviews.llvm.org/D29897