From 7314508612fdd2811b8cf952f48c2dd671e60726 Mon Sep 17 00:00:00 2001 From: Julian Nagele Date: Fri, 20 Dec 2024 15:44:15 +0100 Subject: [PATCH 1/2] [SCEV] Fix exit condition for recursive loop guard collection (#120442) When assumptions are present `Terms.size()` does not actually count the number of conditions collected from dominating branches; introduce a separate counter. Fixes https://github.com/llvm/llvm-project/issues/120237 (cherry picked from commit acfd26a93be3fb70013560f3fb894eb9086e7e32) --- llvm/lib/Analysis/ScalarEvolution.cpp | 6 ++-- ...t-guard-info-with-multiple-predecessors.ll | 33 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp index f4fd6c4f46a7d..1a5cdca9daeb1 100644 --- a/llvm/lib/Analysis/ScalarEvolution.cpp +++ b/llvm/lib/Analysis/ScalarEvolution.cpp @@ -15532,6 +15532,7 @@ void ScalarEvolution::LoopGuards::collectFromBlock( // predecessors that can be found that have unique successors leading to the // original header. // TODO: share this logic with isLoopEntryGuardedByCond. + unsigned NumCollectedConditions = 0; std::pair Pair(Pred, Block); for (; Pair.first; Pair = SE.getPredecessorWithUniqueSuccessorForBB(Pair.first)) { @@ -15543,10 +15544,11 @@ void ScalarEvolution::LoopGuards::collectFromBlock( Terms.emplace_back(LoopEntryPredicate->getCondition(), LoopEntryPredicate->getSuccessor(0) == Pair.second); + NumCollectedConditions++; // If we are recursively collecting guards stop after 2 - // predecessors to limit compile-time impact for now. - if (Depth > 0 && Terms.size() == 2) + // conditions to limit compile-time impact for now. + if (Depth > 0 && NumCollectedConditions == 2) break; } // Finally, if we stopped climbing the predecessor chain because diff --git a/llvm/test/Analysis/ScalarEvolution/backedge-taken-count-guard-info-with-multiple-predecessors.ll b/llvm/test/Analysis/ScalarEvolution/backedge-taken-count-guard-info-with-multiple-predecessors.ll index 71d66ef04ade1..81fe96a2f30c0 100644 --- a/llvm/test/Analysis/ScalarEvolution/backedge-taken-count-guard-info-with-multiple-predecessors.ll +++ b/llvm/test/Analysis/ScalarEvolution/backedge-taken-count-guard-info-with-multiple-predecessors.ll @@ -277,3 +277,36 @@ epilogue: exit: ret void } + +declare void @llvm.assume(i1) + +; Checks that the presence of assumptions does not interfere with +; exiting loop guard collection via following loop predecessors. +define void @pr120442(i1 %c.1, i1 %c.2) { +; CHECK-LABEL: 'pr120442' +; CHECK-NEXT: Determining loop execution counts for: @pr120442 +; CHECK-NEXT: Loop %inner.header: backedge-taken count is i32 0 +; CHECK-NEXT: Loop %inner.header: constant max backedge-taken count is i32 0 +; CHECK-NEXT: Loop %inner.header: symbolic max backedge-taken count is i32 0 +; CHECK-NEXT: Loop %inner.header: Trip multiple is 1 +entry: + call void @llvm.assume(i1 %c.1) + call void @llvm.assume(i1 %c.2) + br label %outer.header + +outer.header: + %phi7 = phi i32 [ 0, %bb ], [ 0, %entry ] + br label %inner.header + +bb: + br i1 false, label %outer.header, label %bb + +inner.header: + %phi = phi i32 [ %add, %inner.header ], [ 0, %outer.header ] + %add = add i32 %phi, 1 + %icmp = icmp ugt i32 %add, 0 + br i1 %icmp, label %exit, label %inner.header + +exit: + ret void +} From 07608853d88aca3a9957b092f48b402f276bdc17 Mon Sep 17 00:00:00 2001 From: Julian Nagele Date: Tue, 31 Dec 2024 10:24:48 +0100 Subject: [PATCH 2/2] [SCEV] Make sure starting block is marked as visited when recursively collecting loop guards. (#120749) When `collectFromBlock` is called without a predecessor (in particular for loops that don't have a unique predecessor outside the loop) we never start climbing the predecessor chain, and thus don't mark the starting block as visited. Fixes https://github.com/llvm/llvm-project/issues/120615. (cherry picked from commit f035351af785b7349ab7bcd55149c781ceca24cb) --- llvm/lib/Analysis/ScalarEvolution.cpp | 1 + ...t-guard-info-with-multiple-predecessors.ll | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp index 1a5cdca9daeb1..90743bd108ed7 100644 --- a/llvm/lib/Analysis/ScalarEvolution.cpp +++ b/llvm/lib/Analysis/ScalarEvolution.cpp @@ -15533,6 +15533,7 @@ void ScalarEvolution::LoopGuards::collectFromBlock( // original header. // TODO: share this logic with isLoopEntryGuardedByCond. unsigned NumCollectedConditions = 0; + VisitedBlocks.insert(Block); std::pair Pair(Pred, Block); for (; Pair.first; Pair = SE.getPredecessorWithUniqueSuccessorForBB(Pair.first)) { diff --git a/llvm/test/Analysis/ScalarEvolution/backedge-taken-count-guard-info-with-multiple-predecessors.ll b/llvm/test/Analysis/ScalarEvolution/backedge-taken-count-guard-info-with-multiple-predecessors.ll index 81fe96a2f30c0..46dccf454f21a 100644 --- a/llvm/test/Analysis/ScalarEvolution/backedge-taken-count-guard-info-with-multiple-predecessors.ll +++ b/llvm/test/Analysis/ScalarEvolution/backedge-taken-count-guard-info-with-multiple-predecessors.ll @@ -310,3 +310,29 @@ inner.header: exit: ret void } + +; Checks correct traversal for loops without a unique predecessor +; outside the loop. +define void @pr120615() { +; CHECK-LABEL: pr120615 +; CHECK-NEXT: Determining loop execution counts for: @pr120615 +; CHECK-NEXT: Loop %header: backedge-taken count is i32 0 +; CHECK-NEXT: Loop %header: constant max backedge-taken count is i32 0 +; CHECK-NEXT: Loop %header: symbolic max backedge-taken count is i32 0 +; CHECK-NEXT: Loop %header: Trip multiple is 1 +entry: + br label %header + +bb: + br label %header + +header: + %0 = phi i32 [ %1, %header ], [ 0, %bb ], [ 0, %entry ] + %1 = add i32 %0, 1 + %icmp = icmp slt i32 %0, 0 + br i1 %icmp, label %header, label %exit + +exit: + ret void + +}