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

Fri Nov 15 14:18:28 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] [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

