This is an archive of the discontinued LLVM Phabricator instance.

Differential D89079

[AMDGPU] Fix mai hazard VALU to LD/ST
ClosedPublic

Authored by kerbowa on Oct 8 2020, 4:38 PM.

Details

Reviewers
rampitec
cdevadas
Commits
rGa4f35ab23281: [AMDGPU] Fix mai hazard VALU to LD/ST
Summary

Fixes: SWDEV-251863

Diff Detail

Event Timeline

kerbowa created this revision.Oct 8 2020, 4:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 8 2020, 4:38 PM
Herald added subscribers: llvm-commits, hiraditya, t-tye and 7 others. · View Herald Transcript
kerbowa requested review of this revision.Oct 8 2020, 4:38 PM
Herald added a subscriber: wdng. · View Herald TranscriptOct 8 2020, 4:38 PM
rampitec added inline comments.Oct 8 2020, 4:41 PM
llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp
1381–1383

Wt -> Wr

rampitec accepted this revision.Oct 8 2020, 4:44 PM

LGTM with a nit.

This revision is now accepted and ready to land.Oct 8 2020, 4:44 PM
kerbowa updated this revision to Diff 297082.Oct 8 2020, 4:50 PM

Wt -> Wr

rampitec added a comment.Oct 8 2020, 4:50 PM

LGTM

Harbormaster completed remote builds in B74502: Diff 297077.Oct 8 2020, 5:12 PM
This revision was landed with ongoing or failed builds.Oct 8 2020, 5:13 PM
Closed by commit rGa4f35ab23281: [AMDGPU] Fix mai hazard VALU to LD/ST (authored by kerbowa). · Explain Why
This revision was automatically updated to reflect the committed changes.
kerbowa added a commit: rGa4f35ab23281: [AMDGPU] Fix mai hazard VALU to LD/ST.
Harbormaster completed remote builds in B74507: Diff 297082.Oct 8 2020, 5:27 PM

Revision Contents

PathSize
llvm/
lib/
Target/
AMDGPU/
11 lines
test/
CodeGen/
AMDGPU/
14 lines

Diff 297082

llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp

Show First 20 Lines • Show All 1,362 Lines • ▼ Show 20 Linesint GCNHazardRecognizer::checkMAILdStHazards(MachineInstr *MI) {
for (const MachineOperand &Op : MI->explicit_uses()) { for (const MachineOperand &Op : MI->explicit_uses()) {
if (!Op.isReg() || !TRI.isVGPR(MF.getRegInfo(), Op.getReg())) if (!Op.isReg() || !TRI.isVGPR(MF.getRegInfo(), Op.getReg()))
continue; continue;
Register Reg = Op.getReg(); Register Reg = Op.getReg();
const int AccVgprReadLdStWaitStates = 2; const int AccVgprReadLdStWaitStates = 2;
const int VALUWriteAccVgprReadLdStDepVALUWaitStates = 1; const int VALUWriteAccVgprRdWrLdStDepVALUWaitStates = 1;
const int MaxWaitStates = 2; const int MaxWaitStates = 2;
int WaitStatesNeededForUse = AccVgprReadLdStWaitStates - int WaitStatesNeededForUse = AccVgprReadLdStWaitStates -
getWaitStatesSinceDef(Reg, IsAccVgprReadFn, MaxWaitStates); getWaitStatesSinceDef(Reg, IsAccVgprReadFn, MaxWaitStates);
WaitStatesNeeded = std::max(WaitStatesNeeded, WaitStatesNeededForUse); WaitStatesNeeded = std::max(WaitStatesNeeded, WaitStatesNeededForUse);
if (WaitStatesNeeded == MaxWaitStates) if (WaitStatesNeeded == MaxWaitStates)
return WaitStatesNeeded; // Early exit. return WaitStatesNeeded; // Early exit.
auto IsVALUAccVgprReadCheckFn = [Reg, this] (MachineInstr *MI) { auto IsVALUAccVgprRdWrCheckFn = [Reg, this](MachineInstr *MI) {
if (MI->getOpcode() != AMDGPU::V_ACCVGPR_READ_B32) if (MI->getOpcode() != AMDGPU::V_ACCVGPR_READ_B32 &&
MI->getOpcode() != AMDGPU::V_ACCVGPR_WRITE_B32)
rampitecUnsubmitted
Not Done
ReplyInline Actions

Wt -> Wr

rampitec: Wt -> Wr
return false; return false;
auto IsVALUFn = [] (MachineInstr *MI) { auto IsVALUFn = [] (MachineInstr *MI) {
return SIInstrInfo::isVALU(*MI) && !SIInstrInfo::isMAI(*MI); return SIInstrInfo::isVALU(*MI) && !SIInstrInfo::isMAI(*MI);
}; };
return getWaitStatesSinceDef(Reg, IsVALUFn, 2 /*MaxWaitStates*/) < return getWaitStatesSinceDef(Reg, IsVALUFn, 2 /*MaxWaitStates*/) <
std::numeric_limits<int>::max(); std::numeric_limits<int>::max();
}; };
WaitStatesNeededForUse = VALUWriteAccVgprReadLdStDepVALUWaitStates - WaitStatesNeededForUse = VALUWriteAccVgprRdWrLdStDepVALUWaitStates -
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-    WaitStatesNeededForUse = VALUWriteAccVgprRdWrLdStDepVALUWaitStates -
-      getWaitStatesSince(IsVALUAccVgprRdWrCheckFn, MaxWaitStates);
+    WaitStatesNeededForUse =
+        VALUWriteAccVgprRdWrLdStDepVALUWaitStates -
+        getWaitStatesSince(IsVALUAccVgprRdWrCheckFn, MaxWaitStates);
Lint: Pre-merge checks: clang-format: please reformat the code ``` - WaitStatesNeededForUse =…
getWaitStatesSince(IsVALUAccVgprReadCheckFn, MaxWaitStates); getWaitStatesSince(IsVALUAccVgprRdWrCheckFn, MaxWaitStates);
WaitStatesNeeded = std::max(WaitStatesNeeded, WaitStatesNeededForUse); WaitStatesNeeded = std::max(WaitStatesNeeded, WaitStatesNeededForUse);
} }
return WaitStatesNeeded; return WaitStatesNeeded;
} }
bool GCNHazardRecognizer::ShouldPreferAnother(SUnit *SU) { bool GCNHazardRecognizer::ShouldPreferAnother(SUnit *SU) {
if (!SU->isInstr()) if (!SU->isInstr())
Show All 21 Lines

llvm/test/CodeGen/AMDGPU/mai-hazards.mir

Show First 20 Lines • Show All 474 Lines • ▼ Show 20 Lines
body: | body: |
bb.0: bb.0:
$vgpr0 = V_MOV_B32_e32 1, implicit $exec $vgpr0 = V_MOV_B32_e32 1, implicit $exec
$vgpr2 = V_ACCVGPR_READ_B32 killed $agpr0, implicit $exec $vgpr2 = V_ACCVGPR_READ_B32 killed $agpr0, implicit $exec
$vgpr4 = FLAT_LOAD_DWORD $vgpr0_vgpr1, 0, 0, 0, 0, implicit $exec, implicit $flat_scr $vgpr4 = FLAT_LOAD_DWORD $vgpr0_vgpr1, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
... ...
--- ---
# GCN-LABEL: name: valu_write_vgpr_accvgpr_write_load_1_and_3_depend
# GCN: V_MOV_B32
# GCN-NEXT: V_ACCVGPR_WRITE_B32
# GCN-NEXT: S_NOP 0
# GCN-NEXT: FLAT_LOAD_DWORD
name: valu_write_vgpr_accvgpr_write_load_1_and_3_depend
body: |
bb.0:
$vgpr0 = V_MOV_B32_e32 1, implicit $exec
$agpr0 = V_ACCVGPR_WRITE_B32 killed $vgpr2, implicit $exec
$vgpr4 = FLAT_LOAD_DWORD $vgpr0_vgpr1, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
...
---
# GCN-LABEL: name: valu_write_vgpr_accvgpr_read_load_2_and_3_depend # GCN-LABEL: name: valu_write_vgpr_accvgpr_read_load_2_and_3_depend
# GCN: V_MOV_B32 # GCN: V_MOV_B32
# GCN-NEXT: V_ACCVGPR_READ_B32 # GCN-NEXT: V_ACCVGPR_READ_B32
# GCN-NEXT: S_NOP 0 # GCN-NEXT: S_NOP 0
# GCN-NEXT: S_NOP 0 # GCN-NEXT: S_NOP 0
# GCN-NEXT: FLAT_LOAD_DWORD # GCN-NEXT: FLAT_LOAD_DWORD
name: valu_write_vgpr_accvgpr_read_load_2_and_3_depend name: valu_write_vgpr_accvgpr_read_load_2_and_3_depend
body: | body: |
bb.0: bb.0:
$vgpr0 = V_MOV_B32_e32 1, implicit $exec $vgpr0 = V_MOV_B32_e32 1, implicit $exec
$vgpr2 = V_ACCVGPR_READ_B32 killed $agpr0, implicit $exec $vgpr2 = V_ACCVGPR_READ_B32 killed $agpr0, implicit $exec
$vgpr4 = FLAT_LOAD_DWORD $vgpr2_vgpr3, 0, 0, 0, 0, implicit $exec, implicit $flat_scr $vgpr4 = FLAT_LOAD_DWORD $vgpr2_vgpr3, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
... ...
--- ---