-
Notifications
You must be signed in to change notification settings - Fork 14.6k
release/21.x: [MachinePipeliner] Fix incorrect dependency direction (#149436) #149950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@aankit-ca What do you think about merging this PR to the release branch? |
@llvm/pr-subscribers-backend-hexagon Author: None (llvmbot) ChangesBackport 6df012a Requested by: @kasuga-fj Full diff: https://github.com/llvm/llvm-project/pull/149950.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp b/llvm/lib/CodeGen/MachinePipeliner.cpp
index b38a4d1c55af9..90005bd181f3a 100644
--- a/llvm/lib/CodeGen/MachinePipeliner.cpp
+++ b/llvm/lib/CodeGen/MachinePipeliner.cpp
@@ -4279,8 +4279,8 @@ void LoopCarriedEdges::modifySUnits(std::vector<SUnit> &SUnits,
!TII->isGlobalMemoryObject(FromMI) &&
!TII->isGlobalMemoryObject(ToMI) && !isSuccOrder(From, To)) {
SDep Pred = Dep;
- Pred.setSUnit(Src);
- Dst->addPred(Pred);
+ Pred.setSUnit(From);
+ To->addPred(Pred);
}
}
}
diff --git a/llvm/test/CodeGen/Hexagon/swp-load-to-store-forward.mir b/llvm/test/CodeGen/Hexagon/swp-load-to-store-forward.mir
new file mode 100644
index 0000000000000..2960343564fca
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/swp-load-to-store-forward.mir
@@ -0,0 +1,50 @@
+# RUN: llc -mtriple=hexagon -run-pass pipeliner %s -o /dev/null
+
+# Check that edges that violate topological order are not added to the
+# SwingSchedulerDAG. This is a case where the crash was caused by PR 145878.
+
+--- |
+ target triple = "hexagon"
+
+ define void @crash_145878() {
+ entry:
+ br label %loop
+
+ loop: ; preds = %loop, %entry
+ %lsr.iv2 = phi i32 [ %lsr.iv.next, %loop ], [ 1, %entry ]
+ %lsr.iv = phi ptr [ %cgep3, %loop ], [ inttoptr (i32 -8 to ptr), %entry ]
+ %cgep = getelementptr i8, ptr %lsr.iv, i32 12
+ %load = load i32, ptr %cgep, align 4
+ store i32 %load, ptr %lsr.iv, align 4
+ %lsr.iv.next = add nsw i32 %lsr.iv2, -1
+ %iv.cmp.not = icmp eq i32 %lsr.iv.next, 0
+ %cgep3 = getelementptr i8, ptr %lsr.iv, i32 -8
+ br i1 %iv.cmp.not, label %exit, label %loop
+
+ exit: ; preds = %loop
+ ret void
+ }
+...
+---
+name: crash_145878
+tracksRegLiveness: true
+body: |
+ bb.0.entry:
+ successors: %bb.1(0x80000000)
+
+ %5:intregs = A2_tfrsi -8
+ J2_loop0i %bb.1, 1, implicit-def $lc0, implicit-def $sa0, implicit-def $usr
+
+ bb.1.loop (machine-block-address-taken):
+ successors: %bb.2(0x04000000), %bb.1(0x7c000000)
+
+ %1:intregs = PHI %5, %bb.0, %3, %bb.1
+ %6:intregs = L2_loadri_io %1, 12 :: (load (s32) from %ir.cgep)
+ S2_storeri_io %1, 0, killed %6 :: (store (s32) into %ir.lsr.iv)
+ %3:intregs = A2_addi %1, -8
+ ENDLOOP0 %bb.1, implicit-def $pc, implicit-def $lc0, implicit $sa0, implicit $lc0
+ J2_jump %bb.2, implicit-def dead $pc
+
+ bb.2.exit:
+ PS_jmpret $r31, implicit-def dead $pc
+...
|
Can you rebase and squash this PR so that I won't merge a merge commit. |
Done. Did I get this right for you? |
Yes looks correct! Just waiting for the CI to pass and I'll merge this. |
This patch fixes a bug introduced in llvm#145878. A dependency was added in the wrong direction, causing an assertion failure due to broken topological order. (cherry picked from commit 6df012a)
@kasuga-fj (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Backport 6df012a
Requested by: @kasuga-fj