[PATCH] D23358: [LVI] Take guards into account

[Sanjoy Das via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

lgtm with some minor comments


================
Comment at: lib/Analysis/LazyValueInfo.cpp:868
@@ -865,2 +867,3 @@
 // If we can determine a constraint on the value given conditions assumed by
 // the program, intersect those constraints with BBLV
+void LazyValueInfoCache::intersectAssumeOrGuardBlockValueConstantRange(
----------------
Doesn't have to be this change, but please document `BBI`.

================
Comment at: lib/Analysis/LazyValueInfo.cpp:884
@@ -882,1 +883,3 @@
   }
+
+  for (BasicBlock::iterator I = BBI->getIterator(),
----------------
Can this be make conditional on if there are uses of the guard intrinsic?  E.g. see `HasGuards` in ScalarEvolution -- that way clients of LLVM that don't use guards at all won't have to do this linear scan.

================
Comment at: lib/Analysis/LazyValueInfo.cpp:886
@@ +885,3 @@
+  for (BasicBlock::iterator I = BBI->getIterator(),
+                            E = BBI->getParent()->begin();
+       I != E; I--) {
----------------
Optional -- we could rewrite this as a range for:

```
for (auto &I : make_range(std::reverse_iterator(BBI->getIterator()), BBI->getParent()->rend()))
```

(Not sure if that is clearly better)

================
Comment at: lib/Analysis/LazyValueInfo.cpp:890
@@ +889,3 @@
+    if (!match(&*I, m_Intrinsic<Intrinsic::experimental_guard>(
+               m_Value(Condition))))
+      continue;
----------------
Is this clang formatted?


https://reviews.llvm.org/D23358