[llvm] [SCEV] Retain SCEVSequentialMinMaxExpr if an operand may trigger UB. (PR #110824)

Florian Hahn via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 2 03:56:35 PDT 2024


https://github.com/fhahn created https://github.com/llvm/llvm-project/pull/110824

Retain SCEVSequentialMinMaxExpr if an operand may trigger UB, e.g. if there is an UDiv operand that may divide by 0 or poison

>From 9926704cffe0573e64214cfc520bf4f21b9bcc78 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 2 Oct 2024 11:42:49 +0100
Subject: [PATCH] [SCEV] Retain SCEVSequentialMinMaxExpr if an operand may
 trigger UB.

Retain SCEVSequentialMinMaxExpr if an operand may trigger UB,
e.g. if there is an UDiv operand that may divide by 0 or poison
---
 .../include/llvm/Analysis/ScalarEvolutionExpressions.h |  5 +++--
 llvm/lib/Analysis/ScalarEvolution.cpp                  | 10 ++++++++++
 .../ScalarEvolution/umin-seq-operand-may-trigger-ub.ll |  6 ++----
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h b/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h
index fd884f2a2f55b0..8adf5fb5bd6a72 100644
--- a/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h
+++ b/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h
@@ -511,8 +511,9 @@ class SCEVUMinExpr : public SCEVMinMaxExpr {
 /// This node is the base class for sequential/in-order min/max selections.
 /// Note that their fundamental difference from SCEVMinMaxExpr's is that they
 /// are early-returning upon reaching saturation point.
-/// I.e. given `0 umin_seq poison`, the result will be `0`,
-/// while the result of `0 umin poison` is `poison`.
+/// I.e. given `0 umin_seq poison`, the result will be `0`, while the result of
+/// `0 umin poison` is `poison`. When returning early, later expressions are not
+/// executed.
 class SCEVSequentialMinMaxExpr : public SCEVNAryExpr {
   friend class ScalarEvolution;
 
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index c939270ed39a65..cb2027a5845875 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -4304,6 +4304,16 @@ ScalarEvolution::getSequentialMinMaxExpr(SCEVTypes Kind,
   }
 
   for (unsigned i = 1, e = Ops.size(); i != e; ++i) {
+    bool MayBeUB = SCEVExprContains(Ops[i], [this](const SCEV *S) {
+      auto *UDiv = dyn_cast<SCEVUDivExpr>(S);
+      // The UDiv may be UB if the divisor is poison or zero. Unless the divisor
+      // is a non-zero constant, we have to assume the UDiv may be UB.
+      return UDiv && (!isa<SCEVConstant>(UDiv->getOperand(1)) ||
+                      !isKnownNonZero(UDiv->getOperand(1)));
+    });
+
+    if (MayBeUB)
+      continue;
     // We can replace %x umin_seq %y with %x umin %y if either:
     //  * %y being poison implies %x is also poison.
     //  * %x cannot be the saturating value (e.g. zero for umin).
diff --git a/llvm/test/Analysis/ScalarEvolution/umin-seq-operand-may-trigger-ub.ll b/llvm/test/Analysis/ScalarEvolution/umin-seq-operand-may-trigger-ub.ll
index 8085592111ff5a..f84f35d4e8ca20 100644
--- a/llvm/test/Analysis/ScalarEvolution/umin-seq-operand-may-trigger-ub.ll
+++ b/llvm/test/Analysis/ScalarEvolution/umin-seq-operand-may-trigger-ub.ll
@@ -4,15 +4,14 @@
 ; The UDiv in the latch may never be executed. The backedge-taken-count
 ; expressions must account for the fact that evaluating the UDiv
 ; unconditionally may trigger UB.
-; FIXME: umin_seq should be used instead of umin  for BTCs.
 define i64 @multi_exit_exit_count_with_udiv_by_value_in_latch(ptr %dst, i64 %N) {
 ; CHECK-LABEL: 'multi_exit_exit_count_with_udiv_by_value_in_latch'
 ; CHECK-NEXT:  Determining loop execution counts for: @multi_exit_exit_count_with_udiv_by_value_in_latch
-; CHECK-NEXT:  Loop %loop.header: <multiple exits> backedge-taken count is ((42 /u %N) umin (0 smax %N))
+; CHECK-NEXT:  Loop %loop.header: <multiple exits> backedge-taken count is ((0 smax %N) umin_seq (42 /u %N))
 ; CHECK-NEXT:    exit count for loop.header: (0 smax %N)
 ; CHECK-NEXT:    exit count for loop.latch: (42 /u %N)
 ; CHECK-NEXT:  Loop %loop.header: constant max backedge-taken count is i64 42
-; CHECK-NEXT:  Loop %loop.header: symbolic max backedge-taken count is ((42 /u %N) umin (0 smax %N))
+; CHECK-NEXT:  Loop %loop.header: symbolic max backedge-taken count is ((0 smax %N) umin_seq (42 /u %N))
 ; CHECK-NEXT:    symbolic max exit count for loop.header: (0 smax %N)
 ; CHECK-NEXT:    symbolic max exit count for loop.latch: (42 /u %N)
 ; CHECK-NEXT:  Loop %loop.header: Trip multiple is 1
@@ -41,7 +40,6 @@ exit:
 ; The UDiv in the latch may never be executed. The backedge-taken-count
 ; expressions must account for the fact that evaluating the UDiv
 ; unconditionally may trigger UB.
-; FIXME: umin_seq should be used instead of umin  for BTCs.
 define i64 @multi_exit_exit_count_with_udiv_by_value_in_latch_different_bounds(ptr %dst, i64 %N, i64 %M) {
 ; CHECK-LABEL: 'multi_exit_exit_count_with_udiv_by_value_in_latch_different_bounds'
 ; CHECK-NEXT:  Determining loop execution counts for: @multi_exit_exit_count_with_udiv_by_value_in_latch_different_bounds



More information about the llvm-commits mailing list