[PATCH] D25075: [LazyValueInfo] Understand monotonically increasing loops and identity values

[James Molloy via llvm-commits]

jmolloy retitled this revision from "[JumpThreading] Allow threading over loop headers when creating single block loops" to "[LazyValueInfo] Understand monotonically increasing loops and identity values".
jmolloy updated this revision to Diff 73478.
jmolloy added a comment.

Hi Philip,

Thanks for the review. After spending a day poring over LVI, I think I've got something more palatable.

I investigated trying to understand monotonic increments in the base case (getEdgeValue and friends), but I drew a blank there. When analyzing PHI incoming values, we obviously haven't fully analyzed the PHI itself. Because those incoming values rely on the PHI, we really have no choice but to return a conservative result (overdefined). I experimented with providing a "partially evaluated PHI" stack but couldn't get a solution that returned conservatively correct results - really, it seems we need to look at a PHI holistically.

This new patch sits in solveBlockValuePHINode as before but has the big change you mentioned in your review; it intersect()s where applicable so we don't clobber any interesting information we may have picked up in getEdgeValue().

The one thing I don't like about this new diff is the "TheCache.eraseValue(PhiVal);" call. We need this because, while analyzing the PHI we will have analyzed the increment (in getEdgeValue) and decided it gives us no information (ConstantRange<full-set>). Later, we understand that increment in the context of the PHI it is incrementing and now have a more precise definition for it, but the full-set value is still cached. I erase it here to ensure it gets recomputed, but given that's never done elsewhere I'm not sure that's right.

Cheers,

James


Repository:
  rL LLVM

https://reviews.llvm.org/D25075

Files:
  lib/Analysis/LazyValueInfo.cpp
  test/Transforms/CorrelatedValuePropagation/monotonic.ll


Index: test/Transforms/CorrelatedValuePropagation/monotonic.ll
===================================================================
--- /dev/null
+++ test/Transforms/CorrelatedValuePropagation/monotonic.ll
@@ -0,0 +1,27 @@
+; RUN: opt -S < %s -correlated-propagation | FileCheck %s
+
+; CHECK-LABEL: @f
+; CHECK:  = phi i32 {{.*}} [ 3, %select.unfold ]
+define void @f() {
+entry:
+  br label %while.cond
+
+select.unfold:                                    ; preds = %while.body
+  br label %while.cond
+
+while.cond:                                       ; preds = %while.body, %select.unfold, %entry
+  %i.0 = phi i32 [ 2, %entry ], [ %i.0, %while.body ], [ %add, %select.unfold ]
+  %cmp = icmp slt i32 %i.0, 3
+  br i1 %cmp, label %while.body, label %return
+
+while.body:                                       ; preds = %while.cond
+  %add = add nsw i32 %i.0, 1
+  %call = call i32 @g()
+  %cmp3 = icmp eq i32 %call, 0
+  br i1 %cmp3, label %select.unfold, label %while.cond
+
+return:
+  ret void
+}
+
+declare i32 @g()
Index: lib/Analysis/LazyValueInfo.cpp
===================================================================
--- lib/Analysis/LazyValueInfo.cpp
+++ lib/Analysis/LazyValueInfo.cpp
@@ -930,6 +930,37 @@
     if (EdgesMissing)
       continue;
 
+    // If this is an identity arc, we cannot be less specific now than we were
+    // before.
+    if (PhiVal == PN)
+      EdgeResult = intersect(EdgeResult, Result);
+
+    // If the PHI has known bounds already and this is an increment, we can
+    // produce a much more accurate lattice value than overdefined.
+    if ((EdgeResult.isOverdefined() ||
+         (EdgeResult.isConstantRange() &&
+          EdgeResult.getConstantRange().isFullSet())) &&
+        (Result.isConstant() || Result.isConstantRange())) {
+      // Can we determine this is a monotonically increasing increment?
+      ConstantInt *Inc;
+      bool NSW = match(PhiVal, m_NSWAdd(m_Specific(PN), m_ConstantInt(Inc)));
+      bool NUW =
+        !NSW && match(PhiVal, m_NUWAdd(m_Specific(PN), m_ConstantInt(Inc)));
+      if ((NSW || NUW) && !Inc->isNegative()) {
+        APInt Start = Result.isConstant()
+          ? cast<ConstantInt>(Result.getConstant())->getValue()
+          : Result.getConstantRange().getLower();
+        APInt End = NSW ? APInt::getSignedMaxValue(Start.getBitWidth())
+          : APInt::getMaxValue(Start.getBitWidth());
+        EdgeResult = LVILatticeVal::getRange(ConstantRange(Start, End));
+
+        // We've discovered that this is a monotonically increasing increment.
+        // PhiVal may have been cached; in this case we must erase it so that
+        // it is recomputed based on this new information.
+        TheCache.eraseValue(PhiVal);
+      }
+    }
+
     Result.mergeIn(EdgeResult, DL);
 
     // If we hit overdefined, exit early.  The BlockVals entry is already set


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D25075.73478.patch
Type: text/x-patch
Size: 2863 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161004/001efeed/attachment.bin>