[PATCH] D137632: [LoopPredication] Widen checks if condition operands constant ranges are known

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:789
+                                                   Instruction *Guard) {
+  if (!WidenChecksUsingConstantRange || LI->getLoopFor(Guard->getParent()) != L)
+    return None;
----------------
Need to make it clear that `Guard` always belongs to loop, but here we are filtering away guards from nested loops. Assert on `L.contains(Guard->getParent())`?


================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:809
+
+  auto ComputeRange = [&](Value *V, bool IsSigned, Instruction *CtxI) {
+    auto Range = ConstantRange::fromKnownBits(
----------------
Isn't `[this]` enough?

Can it be `const Value` and `const Instruction`?


================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:820
+  case CmpInst::ICMP_NE:
+    LLVM_DEBUG(dbgs() << "Unsupported predicate\n");
+    return None;
----------------
Not very informative, can we instead print one message, in the end, and make it verbose? Like, "Speculating LHS = ... less than RHS = ... because max(LHS) < max(RHS)" or something?


================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:852
+  if (IsSigned)
+    LHSMaxIsGreaterThanRHSMax = LHSMaxValue.sgt(RHSMaxValue);
+  if (LHSMaxIsGreaterThanRHSMax) {
----------------
```
  if (IsSigned)
    LHSMaxIsGreaterThanRHSMax = LHSMaxValue.sgt(RHSMaxValue);
  else
    HSMaxIsGreaterThanRHSMax = LHSMaxValue.ugt(RHSMaxValue);
```
?


================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:909
+        Checks.push_back(*RC);
+        NumWidened++;
+        continue;
----------------
Should we have a separate statistics for that?


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

https://reviews.llvm.org/D137632