[LoopVersioning] Add a check to see if the input loop is in LCSSA form #116443
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
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: llvm#36998
VedantParanjape
force-pushed
the
main
branch
from
28ff01f
to
bfc6152
Compare
do you mean to say not familiar? I am not super familiar with this pass as well, but I think because it uses LoopAccessInfo and LoopInfo and they cannot be queried per Loop? so it's a function pass. |
|
sorry yeah, "not familiar" |
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-llvm-transforms Author: Vedant Paranjape (VedantParanjape) ChangesLoop 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! 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 Full diff: https://github.com/llvm/llvm-project/pull/116443.diff 2 Files Affected:
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"
+
+@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() {
+; 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
|
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