[llvm][CodeGen] Address the issue of multiple resource reservations in window scheduling #100301
Conversation
This file contains 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
This is what MIR is for? |
…n window scheduling
kaiyan96
force-pushed
the
bugfix_ws_multi_resource_reserve
branch
from
4064349 to
116575b
Compare
|
@llvm/pr-subscribers-backend-hexagon Author: Kai Yan (kaiyan96) ChangesAddress the issue of multiple resource reservations in window scheduling Full diff: https://github.com/llvm/llvm-project/pull/100301.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/WindowScheduler.cpp b/llvm/lib/CodeGen/WindowScheduler.cpp
index cb878f7425481..c42c81b045807 100644
--- a/llvm/lib/CodeGen/WindowScheduler.cpp
+++ b/llvm/lib/CodeGen/WindowScheduler.cpp
@@ -451,7 +451,6 @@ int WindowScheduler::calculateMaxCycle(ScheduleDAGInstrs &DAG,
}
RM.reserveResources(*SU, CurCycle);
}
- RM.reserveResources(*SU, CurCycle);
OriToCycle[getOriMI(&MI)] = CurCycle;
LLVM_DEBUG(dbgs() << "\tCycle " << CurCycle << " [S."
<< getOriStage(getOriMI(&MI), Offset) << "]: " << MI);
diff --git a/llvm/test/CodeGen/Hexagon/swp-ws-resource-reserve.mir b/llvm/test/CodeGen/Hexagon/swp-ws-resource-reserve.mir
new file mode 100644
index 0000000000000..deae31ffe9f95
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/swp-ws-resource-reserve.mir
@@ -0,0 +1,99 @@
+# REQUIRES: asserts
+# RUN: llc --march=hexagon %s -run-pass=pipeliner -debug-only=pipeliner \
+# RUN: -window-sched=force -filetype=null -verify-machineinstrs 2>&1 \
+# RUN: -window-search-ratio=100 -window-search-num=100 -window-diff-limit=1 \
+# RUN: | FileCheck %s
+
+# We want to verify that all three V6_vaddw instructions are emitted in the same cycle.
+# CHECK: Best window offset is {{[0-9]+}} and Best II is {{[0-9]+}}.
+# CHECK-NEXT: Cycle [[CycleNum:[0-9]+]] [[[StageNum:S.[0-9]+]]]: {{%[0-9]+}}:hvxvr = V6_vaddw {{%[0-9]+}}:hvxvr, {{%[0-9]+}}:hvxvr
+# CHECK-NEXT: Cycle [[CycleNum]] [[[StageNum]]]: {{%[0-9]+}}:hvxvr = V6_vaddw {{%[0-9]+}}:hvxvr, {{%[0-9]+}}:hvxvr
+# CHECK-NEXT: Cycle [[CycleNum]] [[[StageNum]]]: {{%[0-9]+}}:hvxvr = V6_vaddw {{%[0-9]+}}:hvxvr, {{%[0-9]+}}:hvxvr
+
+--- |
+ define void @add_parallel(i32 %N, ptr noalias %x, ptr noalias %y) {
+ entry:
+ %isZeroLength = icmp eq i32 %N, 0
+ br i1 %isZeroLength, label %loop.exit, label %loop.preheader
+
+ loop.preheader: ; preds = %entry
+ %half_splat = tail call <32 x i32> @llvm.hexagon.V6.lvsplatw.128B(i32 1056964608)
+ %one_splat = tail call <32 x i32> @llvm.hexagon.V6.lvsplatw.128B(i32 1065353216)
+ %two_splat = tail call <32 x i32> @llvm.hexagon.V6.lvsplatw.128B(i32 1073741824)
+ br label %loop.body
+
+ loop.exit: ; preds = %loop.body, %entry
+ ret void
+
+ loop.body: ; preds = %loop.body, %loop.preheader
+ %lsr.iv1 = phi ptr [ %cgep2, %loop.body ], [ %x, %loop.preheader ]
+ %lsr.iv = phi ptr [ %cgep1, %loop.body ], [ %y, %loop.preheader ]
+ %index = phi i32 [ 0, %loop.preheader ], [ %index.next, %loop.body ]
+ %vec_x1 = load <32 x i32>, ptr %lsr.iv1, align 128
+ %vec_add_1 = tail call <32 x i32> @llvm.hexagon.V6.vaddw.128B(<32 x i32> %one_splat, <32 x i32> %vec_x1)
+ %vec_add_2 = tail call <32 x i32> @llvm.hexagon.V6.vaddw.128B(<32 x i32> %half_splat, <32 x i32> %vec_x1)
+ %vec_add_3 = tail call <32 x i32> @llvm.hexagon.V6.vaddw.128B(<32 x i32> %two_splat, <32 x i32> %vec_x1)
+ %vec_add_4 = tail call <32 x i32> @llvm.hexagon.V6.vaddw.128B(<32 x i32> %vec_add_1, <32 x i32> %vec_add_2)
+ %vec_add_5 = tail call <32 x i32> @llvm.hexagon.V6.vaddw.128B(<32 x i32> %vec_add_1, <32 x i32> %vec_add_3)
+ %vec_add_6 = tail call <32 x i32> @llvm.hexagon.V6.vaddw.128B(<32 x i32> %vec_add_5, <32 x i32> %vec_add_4)
+ store <32 x i32> %vec_add_6, ptr %lsr.iv, align 128
+ %index.next = add nuw i32 %index, 32
+ %continue = icmp ult i32 %index.next, %N
+ %cgep1 = getelementptr i8, ptr %lsr.iv, i32 128
+ %cgep2 = getelementptr i8, ptr %lsr.iv1, i32 128
+ br i1 %continue, label %loop.body, label %loop.exit
+ }
+
+ declare <32 x i32> @llvm.hexagon.V6.lvsplatw.128B(i32)
+ declare <32 x i32> @llvm.hexagon.V6.vaddw.128B(<32 x i32>, <32 x i32>)
+...
+---
+name: add_parallel
+tracksRegLiveness: true
+body: |
+ bb.0.entry:
+ successors: %bb.2(0x30000000), %bb.1(0x50000000)
+ liveins: $r0, $r1, $r2
+
+ %11:intregs = COPY $r2
+ %10:intregs = COPY $r1
+ %9:intregs = COPY $r0
+ %12:predregs = C2_cmpeqi %9, 0
+ J2_jumpt killed %12, %bb.2, implicit-def dead $pc
+ J2_jump %bb.1, implicit-def dead $pc
+
+ bb.1.loop.preheader:
+ successors: %bb.3(0x80000000)
+
+ %14:intregs = A2_tfrsi 1056964608
+ %0:hvxvr = V6_lvsplatw killed %14
+ %15:intregs = A2_tfrsi 1065353216
+ %1:hvxvr = V6_lvsplatw killed %15
+ %16:intregs = A2_tfrsi 1073741824
+ %2:hvxvr = V6_lvsplatw killed %16
+ %25:intregs = A2_addi %9, 31
+ %26:intregs = S2_lsr_i_r %25, 5
+ %27:intregs = COPY %26
+ J2_loop0r %bb.3, %27, implicit-def $lc0, implicit-def $sa0, implicit-def $usr
+ J2_jump %bb.3, implicit-def dead $pc
+
+ bb.2.loop.exit:
+ PS_jmpret $r31, implicit-def dead $pc
+
+ bb.3.loop.body (machine-block-address-taken):
+ successors: %bb.3(0x7c000000), %bb.2(0x04000000)
+
+ %3:intregs = PHI %10, %bb.1, %8, %bb.3
+ %4:intregs = PHI %11, %bb.1, %7, %bb.3
+ %17:hvxvr, %8:intregs = V6_vL32b_pi %3, 128 :: (load (s1024) from %ir.lsr.iv1)
+ %18:hvxvr = V6_vaddw %1, %17
+ %19:hvxvr = V6_vaddw %0, %17
+ %20:hvxvr = V6_vaddw %2, %17
+ %21:hvxvr = V6_vaddw %18, killed %19
+ %22:hvxvr = V6_vaddw %18, killed %20
+ %23:hvxvr = V6_vaddw killed %22, killed %21
+ %7:intregs = V6_vS32b_pi %4, 128, killed %23 :: (store (s1024) into %ir.lsr.iv)
+ ENDLOOP0 %bb.3, implicit-def $pc, implicit-def $lc0, implicit $sa0, implicit $lc0
+ J2_jump %bb.2, implicit-def dead $pc
+
+...
|
arsenm
reviewed
Jul 25, 2024
| successors: %bb.2(0x30000000), %bb.1(0x50000000) | ||
| liveins: $r0, $r1, $r2 | ||
|
|
||
| %11:intregs = COPY $r2 |
There was a problem hiding this comment.
run-pass=none to cleanup register numbers
|
|
||
| %3:intregs = PHI %10, %bb.1, %8, %bb.3 | ||
| %4:intregs = PHI %11, %bb.1, %7, %bb.3 | ||
| %17:hvxvr, %8:intregs = V6_vL32b_pi %3, 128 :: (load (s1024) from %ir.lsr.iv1) |
There was a problem hiding this comment.
Can you drop the IR references and IR section?
There was a problem hiding this comment.
MMO will reference this IR, and we require MMO to get rid of the barrier dependency.
There was a problem hiding this comment.
You can keep the MMO without the IR reference. You only need the IR reference if this relies on AA, which it ideally wouldn't
There was a problem hiding this comment.
I apologize for our previous response not being very clear. In the window scheduler, we need the noalias attribute in the IR to ensure that there are no barrier dependencies between memory access instructions. This involves performing AA. Therefore, we believe that the IR code is necessary. Or, is there any other ways to achieve this goal?
There was a problem hiding this comment.
Could also restructure it so areMemAccessesTrivially disjoint can detect the lack of aliasing without relying on the AA
|
…n window scheduling - Fix by comment #1
huaatian
reviewed
Jul 25, 2024
| @@ -451,7 +451,6 @@ int WindowScheduler::calculateMaxCycle(ScheduleDAGInstrs &DAG, | |||
| } | |||
| RM.reserveResources(*SU, CurCycle); | |||
| } | |||
| RM.reserveResources(*SU, CurCycle); | |||
There was a problem hiding this comment.
This line is really redundant. It will cause the instruction to occupy too many resources and fail to emit properly.
arsenm
approved these changes
Aug 2, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/3930 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/175/builds/2723 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/185/builds/2741 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/137/builds/2741 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/4926 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/2746 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/2800 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/108/builds/2017 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/2086 Here is the relevant piece of the build log for the reference: |
kaiyan96
added a commit
to kaiyan96/llvm-project
that referenced
this pull request
Aug 2, 2024
…ations in window scheduling (llvm#100301)" This reverts commit aa0a33b.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/2459 Here is the relevant piece of the build log for the reference: |
|
#101658 |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/1762 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/4029 Here is the relevant piece of the build log for the reference: |
banach-space
pushed a commit
to banach-space/llvm-project
that referenced
this pull request
Aug 7, 2024
…n window scheduling (llvm#100301) Address the issue of multiple resource reservations in window scheduling
banach-space
pushed a commit
to banach-space/llvm-project
that referenced
this pull request
Aug 7, 2024
…ations in window scheduling (llvm#100301)" (llvm#101658) This reverts commit aa0a33b.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Address the issue of multiple resource reservations in window scheduling