[PATCH] D14476: [LVI] Introduce an intersect operation on lattice values
Nick Lewycky via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 1 17:02:41 PST 2016
nlewycky added inline comments.
================
Comment at: lib/Analysis/LazyValueInfo.cpp:357
@@ -356,1 +356,3 @@
+ DEBUG(dbgs () << "PUSH: " << *BV.second << " in " << BV.first->getName()
+ << "\n");
----------------
Please fix the extra space after 'dbgs'
================
Comment at: lib/Analysis/LazyValueInfo.cpp:1055-1058
@@ +1054,6 @@
+ return true;
+ if (Val.isConstantRange() &&
+ Val.getConstantRange().isSingleElement())
+ // Integer constants are single element ranges
+ return true;
+ return false;
----------------
Optional: I'd reorder this test first on the grounds that it should be much more likely.
================
Comment at: lib/Analysis/LazyValueInfo.cpp:1090
@@ +1089,3 @@
+
+ // Can't get any more precise than constants
+ if (hasSingleValue(A))
----------------
A period at the end please.
================
Comment at: lib/Analysis/LazyValueInfo.cpp:1105-1107
@@ +1104,5 @@
+ A.getConstantRange().intersectWith(B.getConstantRange());
+ // Note: An empty range is implicitly converted to overdefined internally.
+ // TODO: We could instead use Undefined here since we've proven a conflict
+ // and thus know this path must be unreachable.
+ return LVILatticeVal::getRange(Range);
----------------
If using Undefined here doesn't work today, please file a PR and link to that PR from here. It has to be a bug, right?
================
Comment at: test/Transforms/CorrelatedValuePropagation/conflict.ll:2-3
@@ +1,4 @@
+; RUN: opt -correlated-propagation -S < %s | FileCheck %s
+; Checks that we don't crash on conflicting facts about a value
+; (i.e. unreachable code)
+
----------------
Can you check anything positive about the results, that we did or didn't fold away branches or comparisons maybe?
================
Comment at: test/Transforms/JumpThreading/induction.ll:2
@@ +1,3 @@
+; RUN: opt -S -jump-threading < %s | FileCheck %s
+
+
----------------
Extra newline.
================
Comment at: test/Transforms/JumpThreading/induction.ll:13
@@ +12,3 @@
+
+;; We can use an inductive argument to prove %iv is always positive
+ %cnd = icmp sge i32 %iv, 0
----------------
Could you add a CHECK line which shows that this happened? The presence of a branch to the backedge isn't entirely clear.
http://reviews.llvm.org/D14476
More information about the llvm-commits
mailing list