[PATCH] D14476: [LVI] Introduce an intersect operation on lattice values

[Philip Reames via llvm-commits]

reames added inline comments.

================
Comment at: lib/Analysis/LazyValueInfo.cpp:1019
@@ +1018,3 @@
+    // TODO: Arbitrary choice, could be improved
+    return A;
+  }
----------------
sanjoy wrote:
> Why not return `A.isConstantRange() ? A : B`?
Unless I'm misreading something, your replacement would be incorrect.  Specifically, I'm returning here if either A or B is NOT a constant range.  

================
Comment at: lib/Analysis/LazyValueInfo.cpp:1041
@@ +1040,3 @@
+  LVILatticeVal LocalResult;
+  if (getEdgeValueLocal(Val, BBFrom, BBTo, LocalResult)) {
+    // No new work to push, just fallthrough
----------------
sanjoy wrote:
> This looks a little misleading.  If the return value of `getEdgeValueLocal` truly does not matter, I'd prefer the more idiomatic `(void)getEdgeValueLocal(...)`
I was planning on fixing this in a follow on NFC change by making the function return void.  Happy to tweak as suggested though.


http://reviews.llvm.org/D14476