[llvm][CodeGen] Fixed max cycle calculation with zero-cost instructions for window scheduler #99454
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
…ns for window scheduler
|
@llvm/pr-subscribers-backend-hexagon Author: Kai Yan (kaiyan96) ChangesWe discovered some scheduling failures occurring when zero-cost instructions were involved. This issue will be addressed by this patch. Full diff: https://github.com/llvm/llvm-project/pull/99454.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/WindowScheduler.cpp b/llvm/lib/CodeGen/WindowScheduler.cpp
index 0777480499e55..595f7e2e202dd 100644
--- a/llvm/lib/CodeGen/WindowScheduler.cpp
+++ b/llvm/lib/CodeGen/WindowScheduler.cpp
@@ -437,12 +437,16 @@ int WindowScheduler::calculateMaxCycle(ScheduleDAGInstrs &DAG,
int PredCycle = getOriCycle(PredMI);
ExpectCycle = std::max(ExpectCycle, PredCycle + (int)Pred.getLatency());
}
- // ResourceManager can be used to detect resource conflicts between the
- // current MI and the previously inserted MIs.
- while (!RM.canReserveResources(*SU, CurCycle) || CurCycle < ExpectCycle) {
- ++CurCycle;
- if (CurCycle == (int)WindowIILimit)
- return CurCycle;
+ // Zero cost instructions do not need to check resource.
+ if (!TII->isZeroCost(MI.getOpcode())) {
+ // ResourceManager can be used to detect resource conflicts between the
+ // current MI and the previously inserted MIs.
+ while (!RM.canReserveResources(*SU, CurCycle) || CurCycle < ExpectCycle) {
+ ++CurCycle;
+ if (CurCycle == (int)WindowIILimit)
+ return CurCycle;
+ }
+ RM.reserveResources(*SU, CurCycle);
}
RM.reserveResources(*SU, CurCycle);
OriToCycle[getOriMI(&MI)] = CurCycle;
diff --git a/llvm/test/CodeGen/Hexagon/swp-ws-zero-cost.mir b/llvm/test/CodeGen/Hexagon/swp-ws-zero-cost.mir
new file mode 100644
index 0000000000000..24f227d83ea0d
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/swp-ws-zero-cost.mir
@@ -0,0 +1,42 @@
+# REQUIRES: asserts
+# RUN: llc --march=hexagon %s -run-pass=pipeliner -debug-only=pipeliner \
+# RUN: -window-sched=force -filetype=null -verify-machineinstrs 2>&1 \
+# RUN: | FileCheck %s
+
+# CHECK-NOT: Can't find a valid II. Keep searching...
+
+---
+name: relu
+tracksRegLiveness: true
+body: |
+ bb.0:
+ successors: %bb.2(0x30000000), %bb.1(0x50000000)
+ liveins: $r0, $r1, $r2
+ %0:intregs = COPY $r2
+ %1:intregs = COPY $r1
+ %2:intregs = COPY $r0
+ %3:predregs = C2_cmpeqi %2, 0
+ J2_jumpt killed %3, %bb.2, implicit-def dead $pc
+ J2_jump %bb.1, implicit-def dead $pc
+ bb.1:
+ successors: %bb.3(0x80000000)
+ %4:hvxvr = V6_vd0
+ %5:intregs = A2_addi %2, 31
+ %6:intregs = S2_lsr_i_r %5, 5
+ %7:intregs = COPY %6
+ J2_loop0r %bb.3, %7, implicit-def $lc0, implicit-def $sa0, implicit-def $usr
+ J2_jump %bb.3, implicit-def dead $pc
+ bb.2:
+ PS_jmpret $r31, implicit-def dead $pc
+ bb.3 (machine-block-address-taken):
+ successors: %bb.3(0x7c000000), %bb.2(0x04000000)
+ %8:intregs = PHI %1, %bb.1, %9, %bb.3
+ %10:intregs = PHI %0, %bb.1, %14, %bb.3
+ %11:hvxvr, %9:intregs = V6_vL32b_pi %8, 128
+ %12:intregs = COPY %10
+ %13:hvxvr = V6_vmaxw killed %11, %4
+ %14:intregs = V6_vS32b_pi %12, 128, killed %13
+ ENDLOOP0 %bb.3, implicit-def $pc, implicit-def $lc0, implicit $sa0, implicit $lc0
+ J2_jump %bb.2, implicit-def dead $pc
+...
+
|
arsenm
reviewed
Jul 18, 2024
| if (CurCycle == (int)WindowIILimit) | ||
| return CurCycle; | ||
| // Zero cost instructions do not need to check resource. | ||
| if (!TII->isZeroCost(MI.getOpcode())) { |
There was a problem hiding this comment.
Why does this hook exist? How is it different from isMetaInstruction?
There was a problem hiding this comment.
Also copy should need resource checks?
There was a problem hiding this comment.
Why does this hook exist? How is it different from isMetaInstruction?
- Metainstructions do not participate in scheduling, but zero-cost instructions do.
COPYis a typical example of a zero-cost instruction
Also copy should need resource checks?
COPYdo not occupy hardware resources so they do not require resource checks, they still need to be handled during instruction scheduling to ensure proper execution.
There was a problem hiding this comment.
COPY do not occupy hardware resources
This is a very target specific assumption but ok
arsenm
reviewed
Jul 18, 2024
| # RUN: -window-sched=force -filetype=null -verify-machineinstrs 2>&1 \ | ||
| # RUN: | FileCheck %s | ||
|
|
||
| # CHECK-NOT: Can't find a valid II. Keep searching... |
There was a problem hiding this comment.
A check-not check for a specific message is fragile and practically useless. This needs positive checks
There was a problem hiding this comment.
CHECK-NEXT and -implicit-check-not are more reliable than putting a CHECK-NOT in one place
arsenm
approved these changes
Jul 19, 2024
| # RUN: -window-sched=force -filetype=null -verify-machineinstrs 2>&1 \ | ||
| # RUN: | FileCheck %s | ||
|
|
||
| # CHECK-NOT: Can't find a valid II. Keep searching... |
There was a problem hiding this comment.
CHECK-NEXT and -implicit-check-not are more reliable than putting a CHECK-NOT in one place
yuxuanchen1997
pushed a commit
that referenced
this pull request
Jul 25, 2024
…ns for window scheduler (#99454) Summary: We discovered some scheduling failures occurring when zero-cost instructions were involved. This issue will be addressed by this patch. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250685
|
This change crashes enum { false } div64_u64_rem(long, long long *);
struct dp_tu_calc_input {
int nlanes;
int dsc_en;
};
struct tu_algo_data {
long long pclk_fp;
};
long long dp_panel_update_tu_timings___trans_tmp_5,
_dp_ctrl_calc_tu___trans_tmp_8, _dp_ctrl_calc_tu_temp2_fp;
long _dp_ctrl_calc_tu_temp1_fp;
__attribute__((__malloc__)) void *kzalloc();
long long drm_fixp_from_fraction(long b) {
_Bool b_neg = 0;
long b_abs = b_neg ?: b;
unsigned long long rem;
long long res_abs = div64_u64_rem(0, &rem);
int i = 32;
do {
res_abs <<= 1;
if (rem >= b_abs) {
res_abs |= 1;
rem -= b_abs;
}
} while (--i);
return res_abs;
}
void _dp_ctrl_calc_tu(struct dp_tu_calc_input *in) {
struct tu_algo_data *tu = kzalloc();
struct dp_tu_calc_input __trans_tmp_1 = *in;
struct tu_algo_data __trans_tmp_2 = *tu;
{
struct dp_tu_calc_input in = __trans_tmp_1;
__trans_tmp_2;
int nlanes = in.nlanes;
long temp3_fp;
if (in.dsc_en)
goto fec_check;
_dp_ctrl_calc_tu_temp2_fp = drm_fixp_from_fraction(1);
temp3_fp = 0;
long a = temp3_fp;
a;
nlanes;
tu->pclk_fp = _dp_ctrl_calc_tu_temp2_fp;
fec_check:
_dp_ctrl_calc_tu_temp1_fp = drm_fixp_from_fraction(1000);
long b = _dp_ctrl_calc_tu_temp1_fp;
unsigned result = b;
dp_panel_update_tu_timings___trans_tmp_5 = result;
}
long long b = tu->pclk_fp, result = b;
_dp_ctrl_calc_tu___trans_tmp_8 = result;
}
void dp_ctrl_calc_tu_parameters() {
struct dp_tu_calc_input in;
_dp_ctrl_calc_tu(&in);
} |
|
Thank you for the bug report. We will take care of it promptly. |
Harini0924
pushed a commit
to Harini0924/llvm-project
that referenced
this pull request
Aug 1, 2024
…ns for window scheduler (llvm#99454) We discovered some scheduling failures occurring when zero-cost instructions were involved. This issue will be addressed by this patch.
kaiyan96
added a commit
to kaiyan96/llvm-project
that referenced
this pull request
Aug 12, 2024
…ns for window scheduler (llvm#99454) We discovered some scheduling failures occurring when zero-cost instructions were involved. This issue will be addressed by this patch.
tru
pushed a commit
to kaiyan96/llvm-project
that referenced
this pull request
Aug 20, 2024
…ns for window scheduler (llvm#99454) We discovered some scheduling failures occurring when zero-cost instructions were involved. This issue will be addressed by this patch.
tru
pushed a commit
to kaiyan96/llvm-project
that referenced
this pull request
Sep 1, 2024
…ns for window scheduler (llvm#99454) We discovered some scheduling failures occurring when zero-cost instructions were involved. This issue will be addressed by this patch.
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.
We discovered some scheduling failures occurring when zero-cost instructions were involved. This issue will be addressed by this patch.