[LoopInterchange] Forbid interchange when the dependency is confused #78533
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvm#47259 A confused dependency(*f) in loop should prevent interchange in loops i and j. void test_deps() { for (int i = 0; i <= 3; i++) for (int j = 0; j <= 3; j++) { *f ^= 0x1000; c[j][i] = *f; } }
Member
|
@llvm/pr-subscribers-llvm-transforms Author: None (ShivaChen) ChangesA confused dependency(*f) in loop should prevent interchange in loops i and j. Full diff: https://github.com/llvm/llvm-project/pull/78533.diff 6 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
index 277f530ee25fc1..0ea8244924c0f3 100644
--- a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
@@ -123,6 +123,13 @@ static bool populateDependencyMatrix(CharMatrix &DepMatrix, unsigned Level,
// Track Output, Flow, and Anti dependencies.
if (auto D = DI->depends(Src, Dst, true)) {
assert(D->isOrdered() && "Expected an output, flow or anti dep.");
+
+ if (D->isConfused()) {
+ LLVM_DEBUG(dbgs() << " Confused dependency between:\n"
+ << " " << *Src << "\n"
+ << " " << *Dst << "\n");
+ return false;
+ }
// If the direction vector is negative, normalize it to
// make it non-negative.
if (D->normalize(SE))
diff --git a/llvm/test/Transforms/LoopInterchange/lcssa.ll b/llvm/test/Transforms/LoopInterchange/lcssa.ll
index b41eba4ef56173..d3a0675caa7997 100644
--- a/llvm/test/Transforms/LoopInterchange/lcssa.ll
+++ b/llvm/test/Transforms/LoopInterchange/lcssa.ll
@@ -180,7 +180,7 @@ for.end16: ; preds = %for.exit
; REMARK: Interchanged
; REMARK-NEXT: lcssa_05
-define void @lcssa_05(ptr %ptr) {
+define void @lcssa_05(ptr noalias %ptr) {
entry:
br label %outer.header
@@ -225,7 +225,7 @@ for.end16: ; preds = %for.exit
; REMARK: UnsupportedExitPHI
; REMARK-NEXT: lcssa_06
-define void @lcssa_06(ptr %ptr, ptr %ptr1) {
+define void @lcssa_06(ptr %ptr, ptr noalias %ptr1) {
entry:
br label %outer.header
diff --git a/llvm/test/Transforms/LoopInterchange/pr43326-ideal-access-pattern.ll b/llvm/test/Transforms/LoopInterchange/pr43326-ideal-access-pattern.ll
index def68ca3cd07eb..3b31c2634be71e 100644
--- a/llvm/test/Transforms/LoopInterchange/pr43326-ideal-access-pattern.ll
+++ b/llvm/test/Transforms/LoopInterchange/pr43326-ideal-access-pattern.ll
@@ -27,7 +27,7 @@
; REMARKS-NEXT: Name: Interchanged
; REMARKS-NEXT: Function: pr43326-triply-nested
-define void @pr43326-triply-nested(ptr %e, ptr %f) {
+define void @pr43326-triply-nested(ptr noalias %e, ptr %f) {
entry:
br label %for.outermost.header
diff --git a/llvm/test/Transforms/LoopInterchange/pr47259.ll b/llvm/test/Transforms/LoopInterchange/pr47259.ll
new file mode 100644
index 00000000000000..2a365a62795f81
--- /dev/null
+++ b/llvm/test/Transforms/LoopInterchange/pr47259.ll
@@ -0,0 +1,55 @@
+; REQUIRES: asserts
+; RUN: opt < %s -passes=loop-interchange -cache-line-size=64 -verify-dom-info -verify-loop-info \
+; RUN: -S -debug 2>&1 | FileCheck %s
+
+;; Test that a confused dependency in loop should prevent interchange in
+;; loops i and j.
+;;
+;; void test_deps() {
+;; for (int i = 0; i <= 3; i++)
+;; for (int j = 0; j <= 3; j++) {
+;; *f ^= 0x1000;
+;; c[j][i] = *f;
+;; }
+;; }
+
+
+; CHECK: Confused dependency between:
+; CHECK: store i32 %xor, ptr %arrayidx6, align 4
+; CHECK: %1 = load i32, ptr %0, align 4
+; CHECK-NOT: Loops interchanged.
+
+@a = global i32 0, align 4
+@f = global ptr @a, align 8
+@c = global [4 x [4 x i32]] zeroinitializer, align 8
+
+define void @test_deps() {
+entry:
+ %0 = load ptr, ptr @f, align 8
+ br label %for.cond1.preheader
+
+; Loop:
+for.cond1.preheader: ; preds = %entry, %for.cond.cleanup3
+ %indvars.iv16 = phi i64 [ 0, %entry ], [ %indvars.iv.next17, %for.cond.cleanup3 ]
+ br label %for.body4
+
+for.cond.cleanup3: ; preds = %for.body4
+ %indvars.iv.next17 = add nuw nsw i64 %indvars.iv16, 1
+ %exitcond18 = icmp ne i64 %indvars.iv.next17, 4
+ br i1 %exitcond18, label %for.cond1.preheader, label %for.cond.cleanup
+
+for.body4: ; preds = %for.cond1.preheader, %for.body4
+ %indvars.iv = phi i64 [ 0, %for.cond1.preheader ], [ %indvars.iv.next, %for.body4 ]
+ %1 = load i32, ptr %0, align 4
+ %xor = xor i32 %1, 4096
+ store i32 %xor, ptr %0, align 4
+ %arrayidx6 = getelementptr inbounds [4 x [4 x i32]], ptr @c, i64 0, i64 %indvars.iv, i64 %indvars.iv16
+ store i32 %xor, ptr %arrayidx6, align 4
+ %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+ %exitcond = icmp ne i64 %indvars.iv.next, 4
+ br i1 %exitcond, label %for.body4, label %for.cond.cleanup3
+
+; Exit blocks
+for.cond.cleanup: ; preds = %for.cond.cleanup3
+ ret void
+}
diff --git a/llvm/test/Transforms/LoopInterchange/reductions-across-inner-and-outer-loop.ll b/llvm/test/Transforms/LoopInterchange/reductions-across-inner-and-outer-loop.ll
index fa2021a15a0801..40cf1e9623b8c7 100644
--- a/llvm/test/Transforms/LoopInterchange/reductions-across-inner-and-outer-loop.ll
+++ b/llvm/test/Transforms/LoopInterchange/reductions-across-inner-and-outer-loop.ll
@@ -156,7 +156,7 @@ for1.loopexit: ; preds = %for1.inc
; REMARKS-NEXT: Name: UnsupportedPHIOuter
; REMARKS-NEXT: Function: test4
-define i64 @test4(ptr %Arr, ptr %dst) {
+define i64 @test4(ptr %Arr, ptr noalias %dst) {
entry:
%gep.dst = getelementptr inbounds i64, ptr %dst, i64 42
br label %for1.header
@@ -234,7 +234,7 @@ for1.loopexit: ; preds = %for1.inc
; REMARKS-NEXT: Name: Interchanged
; REMARKS-NEXT: Function: test5
-define float @test5(ptr %Arr, ptr %Arr2) {
+define float @test5(ptr %Arr, ptr noalias %Arr2) {
entry:
br label %outer.header
@@ -274,7 +274,7 @@ for.exit: ; preds = %outer.inc
; REMARKS-NEXT: Name: UnsupportedPHIOuter
; REMARKS-NEXT: Function: test6
-define float @test6(ptr %Arr, ptr %Arr2) {
+define float @test6(ptr %Arr, ptr noalias %Arr2) {
entry:
br label %outer.header
diff --git a/llvm/test/Transforms/LoopInterchange/vector-gep-operand.ll b/llvm/test/Transforms/LoopInterchange/vector-gep-operand.ll
index 03e3b4b7408b5c..b4f9d3f2a3685d 100644
--- a/llvm/test/Transforms/LoopInterchange/vector-gep-operand.ll
+++ b/llvm/test/Transforms/LoopInterchange/vector-gep-operand.ll
@@ -6,35 +6,25 @@
define void @test(ptr noalias %src, ptr %dst) {
; CHECK-LABEL: @test(
; CHECK-NEXT: entry:
-; CHECK-NEXT: br label [[INNER_PREHEADER:%.*]]
-; CHECK: outer.header.preheader:
; CHECK-NEXT: br label [[OUTER_HEADER:%.*]]
; CHECK: outer.header:
-; CHECK-NEXT: [[I:%.*]] = phi i32 [ [[I_NEXT:%.*]], [[OUTER_LATCH:%.*]] ], [ 0, [[OUTER_HEADER_PREHEADER:%.*]] ]
-; CHECK-NEXT: br label [[INNER_SPLIT1:%.*]]
-; CHECK: inner.preheader:
+; CHECK-NEXT: [[I:%.*]] = phi i32 [ [[I_NEXT:%.*]], [[OUTER_LATCH:%.*]] ], [ 0, [[ENTRY:%.*]] ]
; CHECK-NEXT: br label [[INNER:%.*]]
; CHECK: inner:
-; CHECK-NEXT: [[J:%.*]] = phi i64 [ [[TMP0:%.*]], [[INNER_SPLIT:%.*]] ], [ 0, [[INNER_PREHEADER]] ]
-; CHECK-NEXT: br label [[OUTER_HEADER_PREHEADER]]
-; CHECK: inner.split1:
+; CHECK-NEXT: [[J:%.*]] = phi i64 [ 0, [[OUTER_HEADER]] ], [ [[J_NEXT:%.*]], [[INNER]] ]
; CHECK-NEXT: [[SRC_GEP:%.*]] = getelementptr inbounds [256 x float], ptr [[SRC:%.*]], <2 x i64> <i64 0, i64 1>, i64 [[J]]
; CHECK-NEXT: [[SRC_0:%.*]] = extractelement <2 x ptr> [[SRC_GEP]], i32 0
; CHECK-NEXT: [[LV_0:%.*]] = load float, ptr [[SRC_0]], align 4
; CHECK-NEXT: [[ADD_0:%.*]] = fadd float [[LV_0]], 1.000000e+00
; CHECK-NEXT: [[DST_GEP:%.*]] = getelementptr inbounds float, ptr [[DST:%.*]], i64 [[J]]
; CHECK-NEXT: store float [[ADD_0]], ptr [[DST_GEP]], align 4
-; CHECK-NEXT: [[J_NEXT:%.*]] = add nuw nsw i64 [[J]], 1
+; CHECK-NEXT: [[J_NEXT]] = add nuw nsw i64 [[J]], 1
; CHECK-NEXT: [[INNER_EXITCOND:%.*]] = icmp eq i64 [[J_NEXT]], 100
-; CHECK-NEXT: br label [[OUTER_LATCH]]
-; CHECK: inner.split:
-; CHECK-NEXT: [[TMP0]] = add nuw nsw i64 [[J]], 1
-; CHECK-NEXT: [[TMP1:%.*]] = icmp eq i64 [[TMP0]], 100
-; CHECK-NEXT: br i1 [[TMP1]], label [[EXIT:%.*]], label [[INNER]]
+; CHECK-NEXT: br i1 [[INNER_EXITCOND]], label [[OUTER_LATCH]], label [[INNER]]
; CHECK: outer.latch:
; CHECK-NEXT: [[I_NEXT]] = add nuw nsw i32 [[I]], 1
; CHECK-NEXT: [[OUTER_EXITCOND:%.*]] = icmp eq i32 [[I_NEXT]], 100
-; CHECK-NEXT: br i1 [[OUTER_EXITCOND]], label [[INNER_SPLIT]], label [[OUTER_HEADER]]
+; CHECK-NEXT: br i1 [[OUTER_EXITCOND]], label [[EXIT:%.*]], label [[OUTER_HEADER]]
; CHECK: exit:
; CHECK-NEXT: ret void
;
|
artagnon
reviewed
Sep 13, 2024
Contributor
artagnon
left a comment
artagnon
left a comment
There was a problem hiding this comment.
The core change LGTM; a few minor suggestions for the tests.
Comment on lines
+22
to
+24
| @a = global i32 0, align 4 | ||
| @f = global ptr @a, align 8 | ||
| @c = global [4 x [4 x i32]] zeroinitializer, align 8 |
Contributor
There was a problem hiding this comment.
Do these need to be globals? Can they not be passed as arguments to the function?
Comment on lines
+2
to
+3
| ; RUN: opt < %s -passes=loop-interchange -cache-line-size=64 -verify-dom-info -verify-loop-info \ | ||
| ; RUN: -S -debug 2>&1 | FileCheck %s |
Contributor
There was a problem hiding this comment.
-disable-output -debug-only=loop-interchange? Also, is -cache-line-size=64 required for the test?
Comment on lines
+42
to
+50
| %indvars.iv = phi i64 [ 0, %for.cond1.preheader ], [ %indvars.iv.next, %for.body4 ] | ||
| %1 = load i32, ptr %0, align 4 | ||
| %xor = xor i32 %1, 4096 | ||
| store i32 %xor, ptr %0, align 4 | ||
| %arrayidx6 = getelementptr inbounds [4 x [4 x i32]], ptr @c, i64 0, i64 %indvars.iv, i64 %indvars.iv16 | ||
| store i32 %xor, ptr %arrayidx6, align 4 | ||
| %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1 | ||
| %exitcond = icmp ne i64 %indvars.iv.next, 4 | ||
| br i1 %exitcond, label %for.body4, label %for.cond.cleanup3 |
Contributor
There was a problem hiding this comment.
It would be nice if the test were cleaned up a bit. Example: s/indvars.iv/iv.inner/, s/for.body.4/inner.loop/, s/for.cond1.preheader/outer.loop/?
| ; REMARK-NEXT: lcssa_06 | ||
|
|
||
| define void @lcssa_06(ptr %ptr, ptr %ptr1) { | ||
| define void @lcssa_06(ptr %ptr, ptr noalias %ptr1) { |
Contributor
There was a problem hiding this comment.
Are all these other noalias changes required as part of the patch? If so, I can recommend marking both arguments noalias, and mentioning why in the commit message (eg. if these tests were faulty to begin with).
Contributor
|
Kindly write this as "Fixes ..." and move it to the last line of the message, so that the issue will be auto-closed. |
artagnon
reviewed
Sep 13, 2024
| @f = global ptr @a, align 8 | ||
| @c = global [4 x [4 x i32]] zeroinitializer, align 8 | ||
|
|
||
| define void @test_deps() { |
Contributor
There was a problem hiding this comment.
I would recommend naming this function confused_deps, as the "test" part is already implied.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#47259
A confused dependency(*f) in loop should prevent interchange in loops i and j.