[llvm] [LoopVersioning] Add a check to see if the input loop is in LCSSA form (PR #116443)

Vedant Paranjape via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 16 08:32:34 PST 2024


https://github.com/VedantParanjape updated https://github.com/llvm/llvm-project/pull/116443

>From bfc615214e5686a2b41a7a7a7285ff6e1e89e306 Mon Sep 17 00:00:00 2001
From: Vedant Paranjape <vedantparanjape160201 at gmail.com>
Date: Fri, 15 Nov 2024 17:03:45 -0500
Subject: [PATCH 1/4] [LoopVersioning] Add a check to see if the input loop is
 in LCSSA form

Loop Optimizations expect the input loop to be in LCSSA form. But it seems
that LoopVersioning doesn't have any check to see if the loop is actually in
LCSSA form. As a result, if we give it a loop which is not in LCSSA form but
still correct semantically, the resulting transformation fails to pass through
verifier pass with the following error.

Instruction does not dominate all uses!
%inc = add nsw i16 undef, 1
store i16 %inc, ptr @c, align 1

As the loop is not in LCSSA form, LoopVersioning's transformations leads to
invalid IR! As some instructions do not dominate all their uses.

This patch checks if a loop is in LCSSA form, if not it will call
formLCSSARecursively on the loop before passing it to LoopVersioning.

Fixes: #36998
---
 llvm/lib/Transforms/Utils/LoopVersioning.cpp  |  4 ++
 .../Transforms/LoopVersioning/crash-36998.ll  | 57 +++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 llvm/test/Transforms/LoopVersioning/crash-36998.ll

diff --git a/llvm/lib/Transforms/Utils/LoopVersioning.cpp b/llvm/lib/Transforms/Utils/LoopVersioning.cpp
index 8f8c40a4e73be1..b437ddb4b0a300 100644
--- a/llvm/lib/Transforms/Utils/LoopVersioning.cpp
+++ b/llvm/lib/Transforms/Utils/LoopVersioning.cpp
@@ -26,6 +26,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Cloning.h"
+#include "llvm/Transforms/Utils/LoopUtils.h"
 #include "llvm/Transforms/Utils/ScalarEvolutionExpander.h"
 
 using namespace llvm;
@@ -278,6 +279,9 @@ bool runImpl(LoopInfo *LI, LoopAccessInfoManager &LAIs, DominatorTree *DT,
     if (!LAI.hasConvergentOp() &&
         (LAI.getNumRuntimePointerChecks() ||
          !LAI.getPSE().getPredicate().isAlwaysTrue())) {
+      if (!L->isLCSSAForm(*DT))
+        llvm::formLCSSARecursively(*L, *DT, LI, SE);
+
       LoopVersioning LVer(LAI, LAI.getRuntimePointerChecking()->getChecks(), L,
                           LI, DT, SE);
       LVer.versionLoop();
diff --git a/llvm/test/Transforms/LoopVersioning/crash-36998.ll b/llvm/test/Transforms/LoopVersioning/crash-36998.ll
new file mode 100644
index 00000000000000..53bcb5f310c001
--- /dev/null
+++ b/llvm/test/Transforms/LoopVersioning/crash-36998.ll
@@ -0,0 +1,57 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -passes=loop-versioning -aa-pipeline='' -S < %s | FileCheck %s
+target triple = "x86_64-unknown-linux-gnu"
+
+ at a = external global i16, align 1
+ at b = external global i16, align 1
+ at c = external global i16, align 1
+
+define void @f2() {
+; CHECK-LABEL: define void @f2() {
+; CHECK-NEXT:  [[FOR_BODY_LVER_CHECK:.*:]]
+; CHECK-NEXT:    [[BOUND0:%.*]] = icmp ult ptr @b, getelementptr inbounds nuw (i8, ptr @a, i64 2)
+; CHECK-NEXT:    [[BOUND1:%.*]] = icmp ult ptr @a, getelementptr inbounds nuw (i8, ptr @b, i64 2)
+; CHECK-NEXT:    [[FOUND_CONFLICT:%.*]] = and i1 [[BOUND0]], [[BOUND1]]
+; CHECK-NEXT:    br i1 [[FOUND_CONFLICT]], label %[[FOR_BODY_PH_LVER_ORIG:.*]], label %[[FOR_BODY_PH:.*]]
+; CHECK:       [[FOR_BODY_PH_LVER_ORIG]]:
+; CHECK-NEXT:    br label %[[FOR_BODY_LVER_ORIG:.*]]
+; CHECK:       [[FOR_BODY_LVER_ORIG]]:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i16, ptr @a, align 1
+; CHECK-NEXT:    store i16 [[TMP0]], ptr @b, align 1
+; CHECK-NEXT:    [[INC_LVER_ORIG:%.*]] = add nsw i16 undef, 1
+; CHECK-NEXT:    br i1 false, label %[[FOR_BODY_LVER_ORIG]], label %[[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT:.*]]
+; CHECK:       [[FOR_BODY_PH]]:
+; CHECK-NEXT:    br label %[[FOR_BODY:.*]]
+; CHECK:       [[FOR_BODY]]:
+; CHECK-NEXT:    [[TMP1:%.*]] = load i16, ptr @a, align 1, !alias.scope [[META0:![0-9]+]]
+; CHECK-NEXT:    store i16 [[TMP1]], ptr @b, align 1, !alias.scope [[META3:![0-9]+]], !noalias [[META0]]
+; CHECK-NEXT:    [[INC:%.*]] = add nsw i16 undef, 1
+; CHECK-NEXT:    br i1 false, label %[[FOR_BODY]], label %[[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT1:.*]]
+; CHECK:       [[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT]]:
+; CHECK-NEXT:    [[INC_LCSSA_PH:%.*]] = phi i16 [ [[INC_LVER_ORIG]], %[[FOR_BODY_LVER_ORIG]] ]
+; CHECK-NEXT:    [[SPLIT2_PH:%.*]] = phi i16 [ [[INC_LVER_ORIG]], %[[FOR_BODY_LVER_ORIG]] ]
+; CHECK-NEXT:    br label %[[FOR_COND_FOR_END_CRIT_EDGE:.*]]
+; CHECK:       [[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT1]]:
+; CHECK-NEXT:    [[INC_LCSSA_PH2:%.*]] = phi i16 [ [[INC]], %[[FOR_BODY]] ]
+; CHECK-NEXT:    [[SPLIT2_PH3:%.*]] = phi i16 [ [[INC]], %[[FOR_BODY]] ]
+; CHECK-NEXT:    br label %[[FOR_COND_FOR_END_CRIT_EDGE]]
+; CHECK:       [[FOR_COND_FOR_END_CRIT_EDGE]]:
+; CHECK-NEXT:    [[INC_LCSSA:%.*]] = phi i16 [ [[INC_LCSSA_PH]], %[[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT]] ], [ [[INC_LCSSA_PH2]], %[[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT1]] ]
+; CHECK-NEXT:    [[SPLIT2:%.*]] = phi i16 [ [[SPLIT2_PH]], %[[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT]] ], [ [[SPLIT2_PH3]], %[[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT1]] ]
+; CHECK-NEXT:    store i16 [[INC_LCSSA]], ptr @c, align 1
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %for.body
+
+for.body:                                         ; preds = %for.body, %entry
+  %0 = load i16, ptr @a, align 1
+  store i16 %0, ptr @b, align 1
+  %inc = add nsw i16 undef, 1
+  br i1 false, label %for.body, label %for.cond.for.end_crit_edge
+
+for.cond.for.end_crit_edge:                       ; preds = %for.body
+  %split2 = phi i16 [ %inc, %for.body ]
+  store i16 %inc, ptr @c, align 1
+  ret void
+}
\ No newline at end of file

>From 682c55730cabf6e7c315fdece6940635e3568b53 Mon Sep 17 00:00:00 2001
From: Vedant Paranjape <22630228+VedantParanjape at users.noreply.github.com>
Date: Mon, 9 Dec 2024 15:32:40 -0500
Subject: [PATCH 2/4] Update llvm/lib/Transforms/Utils/LoopVersioning.cpp

Co-authored-by: Florian Hahn <flo at fhahn.com>
---
 llvm/lib/Transforms/Utils/LoopVersioning.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/Utils/LoopVersioning.cpp b/llvm/lib/Transforms/Utils/LoopVersioning.cpp
index b437ddb4b0a300..5ee551e6f0ccbd 100644
--- a/llvm/lib/Transforms/Utils/LoopVersioning.cpp
+++ b/llvm/lib/Transforms/Utils/LoopVersioning.cpp
@@ -280,7 +280,7 @@ bool runImpl(LoopInfo *LI, LoopAccessInfoManager &LAIs, DominatorTree *DT,
         (LAI.getNumRuntimePointerChecks() ||
          !LAI.getPSE().getPredicate().isAlwaysTrue())) {
       if (!L->isLCSSAForm(*DT))
-        llvm::formLCSSARecursively(*L, *DT, LI, SE);
+       formLCSSARecursively(*L, *DT, LI, SE);
 
       LoopVersioning LVer(LAI, LAI.getRuntimePointerChecking()->getChecks(), L,
                           LI, DT, SE);

>From 19f9ae21b9a8eb32e500eb98ca5829fedf1521f7 Mon Sep 17 00:00:00 2001
From: Vedant Paranjape <vedantparanjape160201 at gmail.com>
Date: Mon, 16 Dec 2024 11:15:36 -0500
Subject: [PATCH 3/4] fixed nitpick in the testcase

---
 .../Transforms/LoopVersioning/crash-36998.ll   | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/llvm/test/Transforms/LoopVersioning/crash-36998.ll b/llvm/test/Transforms/LoopVersioning/crash-36998.ll
index 53bcb5f310c001..e57962e537af85 100644
--- a/llvm/test/Transforms/LoopVersioning/crash-36998.ll
+++ b/llvm/test/Transforms/LoopVersioning/crash-36998.ll
@@ -1,13 +1,12 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
 ; RUN: opt -passes=loop-versioning -aa-pipeline='' -S < %s | FileCheck %s
-target triple = "x86_64-unknown-linux-gnu"
 
 @a = external global i16, align 1
 @b = external global i16, align 1
 @c = external global i16, align 1
 
-define void @f2() {
-; CHECK-LABEL: define void @f2() {
+define i16 @f2() {
+; CHECK-LABEL: define i16 @f2() {
 ; CHECK-NEXT:  [[FOR_BODY_LVER_CHECK:.*:]]
 ; CHECK-NEXT:    [[BOUND0:%.*]] = icmp ult ptr @b, getelementptr inbounds nuw (i8, ptr @a, i64 2)
 ; CHECK-NEXT:    [[BOUND1:%.*]] = icmp ult ptr @a, getelementptr inbounds nuw (i8, ptr @b, i64 2)
@@ -39,7 +38,7 @@ define void @f2() {
 ; CHECK-NEXT:    [[INC_LCSSA:%.*]] = phi i16 [ [[INC_LCSSA_PH]], %[[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT]] ], [ [[INC_LCSSA_PH2]], %[[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT1]] ]
 ; CHECK-NEXT:    [[SPLIT2:%.*]] = phi i16 [ [[SPLIT2_PH]], %[[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT]] ], [ [[SPLIT2_PH3]], %[[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT1]] ]
 ; CHECK-NEXT:    store i16 [[INC_LCSSA]], ptr @c, align 1
-; CHECK-NEXT:    ret void
+; CHECK-NEXT:    ret i16 [[SPLIT2]]
 ;
 entry:
   br label %for.body
@@ -53,5 +52,12 @@ for.body:                                         ; preds = %for.body, %entry
 for.cond.for.end_crit_edge:                       ; preds = %for.body
   %split2 = phi i16 [ %inc, %for.body ]
   store i16 %inc, ptr @c, align 1
-  ret void
-}
\ No newline at end of file
+  ret i16 %split2
+}
+;.
+; CHECK: [[META0]] = !{[[META1:![0-9]+]]}
+; CHECK: [[META1]] = distinct !{[[META1]], [[META2:![0-9]+]]}
+; CHECK: [[META2]] = distinct !{[[META2]], !"LVerDomain"}
+; CHECK: [[META3]] = !{[[META4:![0-9]+]]}
+; CHECK: [[META4]] = distinct !{[[META4]], [[META2]]}
+;.

>From cce211b6e5c1ab8cdd3532a60274f58ff3a84ed6 Mon Sep 17 00:00:00 2001
From: Vedant Paranjape <vedantparanjape160201 at gmail.com>
Date: Mon, 16 Dec 2024 11:32:20 -0500
Subject: [PATCH 4/4] remove undef from test

---
 llvm/test/Transforms/LoopVersioning/crash-36998.ll | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/llvm/test/Transforms/LoopVersioning/crash-36998.ll b/llvm/test/Transforms/LoopVersioning/crash-36998.ll
index e57962e537af85..1217d9405e9f89 100644
--- a/llvm/test/Transforms/LoopVersioning/crash-36998.ll
+++ b/llvm/test/Transforms/LoopVersioning/crash-36998.ll
@@ -5,8 +5,9 @@
 @b = external global i16, align 1
 @c = external global i16, align 1
 
-define i16 @f2() {
-; CHECK-LABEL: define i16 @f2() {
+define i16 @f2(i16 %a) {
+; CHECK-LABEL: define i16 @f2(
+; CHECK-SAME: i16 [[A:%.*]]) {
 ; CHECK-NEXT:  [[FOR_BODY_LVER_CHECK:.*:]]
 ; CHECK-NEXT:    [[BOUND0:%.*]] = icmp ult ptr @b, getelementptr inbounds nuw (i8, ptr @a, i64 2)
 ; CHECK-NEXT:    [[BOUND1:%.*]] = icmp ult ptr @a, getelementptr inbounds nuw (i8, ptr @b, i64 2)
@@ -17,14 +18,14 @@ define i16 @f2() {
 ; CHECK:       [[FOR_BODY_LVER_ORIG]]:
 ; CHECK-NEXT:    [[TMP0:%.*]] = load i16, ptr @a, align 1
 ; CHECK-NEXT:    store i16 [[TMP0]], ptr @b, align 1
-; CHECK-NEXT:    [[INC_LVER_ORIG:%.*]] = add nsw i16 undef, 1
+; CHECK-NEXT:    [[INC_LVER_ORIG:%.*]] = add nsw i16 [[A]], 1
 ; CHECK-NEXT:    br i1 false, label %[[FOR_BODY_LVER_ORIG]], label %[[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT:.*]]
 ; CHECK:       [[FOR_BODY_PH]]:
 ; CHECK-NEXT:    br label %[[FOR_BODY:.*]]
 ; CHECK:       [[FOR_BODY]]:
 ; CHECK-NEXT:    [[TMP1:%.*]] = load i16, ptr @a, align 1, !alias.scope [[META0:![0-9]+]]
 ; CHECK-NEXT:    store i16 [[TMP1]], ptr @b, align 1, !alias.scope [[META3:![0-9]+]], !noalias [[META0]]
-; CHECK-NEXT:    [[INC:%.*]] = add nsw i16 undef, 1
+; CHECK-NEXT:    [[INC:%.*]] = add nsw i16 [[A]], 1
 ; CHECK-NEXT:    br i1 false, label %[[FOR_BODY]], label %[[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT1:.*]]
 ; CHECK:       [[FOR_COND_FOR_END_CRIT_EDGE_LOOPEXIT]]:
 ; CHECK-NEXT:    [[INC_LCSSA_PH:%.*]] = phi i16 [ [[INC_LVER_ORIG]], %[[FOR_BODY_LVER_ORIG]] ]
@@ -46,7 +47,7 @@ entry:
 for.body:                                         ; preds = %for.body, %entry
   %0 = load i16, ptr @a, align 1
   store i16 %0, ptr @b, align 1
-  %inc = add nsw i16 undef, 1
+  %inc = add nsw i16 %a, 1
   br i1 false, label %for.body, label %for.cond.for.end_crit_edge
 
 for.cond.for.end_crit_edge:                       ; preds = %for.body



More information about the llvm-commits mailing list