[ConstraintElim] Support arbitrary incoming values for inductions

[fhahn]

Support arbitray incoming values for AddRecs by getting the loop predecessor and checking if its SCEV matches the AddRec start.

This is done after the existing check, which can help to catch cases where the expression gets simplified by SCEV to either an IR constant or existing value which can be used instead.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Changes

Support arbitray incoming values for AddRecs by getting the loop predecessor and checking if its SCEV matches the AddRec start.

This is done after the existing check, which can help to catch cases where the expression gets simplified by SCEV to either an IR constant or existing value which can be used instead.

---

Full diff: https://github.com/llvm/llvm-project/pull/68032.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Scalar/ConstraintElimination.cpp (+6-4)
• (modified) llvm/test/Transforms/ConstraintElimination/monotonic-int-phis-nested-loops.ll (+1-2)

diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index bca37da5ff10f2b..312e9f19b116940 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -859,7 +859,8 @@ void State::addInfoForInductions(BasicBlock &BB) {
     return;
 
   auto *AR = dyn_cast_or_null<SCEVAddRecExpr>(SE.getSCEV(PN));
-  if (!AR)
+  BasicBlock *LoopPred = L->getLoopPredecessor();
+  if (!AR || !LoopPred)
     return;
 
   const SCEV *StartSCEV = AR->getStart();
@@ -868,9 +869,10 @@ void State::addInfoForInductions(BasicBlock &BB) {
     StartValue = C->getValue();
   else if (auto *U = dyn_cast<SCEVUnknown>(StartSCEV))
     StartValue = U->getValue();
-
-  if (!StartValue)
-    return;
+  else {
+    StartValue = PN->getIncomingValueForBlock(LoopPred);
+    assert(SE.getSCEV(StartValue) == AR->getStart() && "inconsistent start value);
+  }
 
   DomTreeNode *DTN = DT.getNode(InLoopSucc);
   auto Inc = SE.getMonotonicPredicateType(AR, CmpInst::ICMP_UGT);
diff --git a/llvm/test/Transforms/ConstraintElimination/monotonic-int-phis-nested-loops.ll b/llvm/test/Transforms/ConstraintElimination/monotonic-int-phis-nested-loops.ll
index 4835d48c57b6327..e5d101f7fdea106 100644
--- a/llvm/test/Transforms/ConstraintElimination/monotonic-int-phis-nested-loops.ll
+++ b/llvm/test/Transforms/ConstraintElimination/monotonic-int-phis-nested-loops.ll
@@ -17,8 +17,7 @@ define void @start_value_of_inner_add_rec_is_add_rec_condition_can_be_simplified
 ; CHECK-NEXT:    [[CMP2_NOT:%.*]] = icmp eq i32 [[K_0]], [[LEN]]
 ; CHECK-NEXT:    br i1 [[CMP2_NOT]], label [[OUTER_LATCH]], label [[INNER_LATCH]]
 ; CHECK:       inner.latch:
-; CHECK-NEXT:    [[CMP_NOT_I:%.*]] = icmp ult i32 [[K_0]], [[LEN]]
-; CHECK-NEXT:    call void @use(i1 [[CMP_NOT_I]])
+; CHECK-NEXT:    call void @use(i1 true)
 ; CHECK-NEXT:    [[K_INC]] = add i32 [[K_0]], 1
 ; CHECK-NEXT:    br label [[INNER_HEADER]]
 ; CHECK:       outer.latch:

[nikic]

This looks fine to me, but I do wonder whether

This is done after the existing check, which can help to catch cases where the expression gets simplified by SCEV to either an IR constant or existing value which can be used instead.

really makes sense, especially the SCEVUnknown bit. Are there any tests that show a behavior difference if the check is dropped?

[nikic]

Suggested change

	assert(SE.getSCEV(StartValue) == AR->getStart() && "inconsistent start value);
	assert(SE.getSCEV(StartValue) == StartSCEV && "inconsistent start value);

[fhahn]

This looks fine to me, but I do wonder whether

This is done after the existing check, which can help to catch cases where the expression gets simplified by SCEV to either an IR constant or existing value which can be used instead.

really makes sense, especially the SCEVUnknown bit. Are there any tests that show a behavior difference if the check is dropped?

Ah yes, I forgot to drop the SCEVUnknown case. For the constant case, there's the admittedly slightly artificialg https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/ConstraintElimination/monotonic-int-phis-wrapping.ll#L106

Updated the code to drop the SCEVUnknown case, sorry for the force push (I am just so used to rebasing....)

[nikic]

LGTM

[RolandF77]

We have encountered an assert on our downstream build when picking up this commit.
To reproduce: opt -S -passes=constraint-elimination wcl2.ll
wcl2.ll.gz

[antoniofrighetto]

Reduced to:

define void @test() {
entry:
  br label %while.cond1

while.cond1:                                      ; preds = %while.cond1, %entry
  %__first.addr.1 = phi ptr [ null, %entry ], [ %incdec.ptr, %while.cond1 ]
  %incdec.ptr = getelementptr ptr, ptr %__first.addr.1, i32 1
  br i1 false, label %while.cond1, label %do.body

do.body:                                          ; preds = %do.cond, %while.cond1
  %__first.addr.1.lcssa = phi ptr [ %__first.addr.1, %do.cond ], [ %__first.addr.1, %while.cond1 ]
  %cmp7 = icmp eq ptr %__first.addr.1.lcssa, null
  br i1 %cmp7, label %if.then8, label %do.cond

if.then8:                                         ; preds = %do.body
  ret void

do.cond:                                          ; preds = %do.body
  br label %do.body
}

[fhahn]

Thanks @RolandF77 & @antoniofrighetto . Added an extra test in 0d48a46 and should be fixed in 26ab444