This is an archive of the discontinued LLVM Phabricator instance.

Differential D114202

[AMDGPU] Fix SIPostRABundler crash on null register used by dbg value
ClosedPublic

Authored by rampitec on Nov 18 2021, 3:53 PM.

Details

Reviewers
kerbowa
arsenm
Commits
rGf8e615462b4f: [AMDGPU] Fix SIPostRABundler crash on null register used by dbg value
Summary

Recently we started generate DBG_VALUEs with $noreg operands.
This crashes SIPostRABundler, and it should not iterate these
registers anyway.

Fixes: SWDEV-311733

Diff Detail

Event Timeline

rampitec created this revision.Nov 18 2021, 3:53 PM
Herald added subscribers: ormris, foad, hiraditya and 7 others. · View Herald TranscriptNov 18 2021, 3:53 PM
rampitec requested review of this revision.Nov 18 2021, 3:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 18 2021, 3:53 PM
Herald added a subscriber: wdng. · View Herald Transcript
arsenm added inline comments.Nov 18 2021, 4:05 PM
llvm/lib/Target/AMDGPU/SIPostRABundler.cpp
93

This allows all meta instructions in the bundle region. Should it skip all of them?

rampitec added inline comments.Nov 18 2021, 4:10 PM
llvm/lib/Target/AMDGPU/SIPostRABundler.cpp
93

I do not think so. This a least handles KILL which is also meta.

rampitec edited the summary of this revision. (Show Details)Nov 18 2021, 4:17 PM
Harbormaster completed remote builds in B135005: Diff 388343.Nov 18 2021, 4:39 PM
kerbowa accepted this revision.Nov 18 2021, 4:52 PM

LGTM

This revision is now accepted and ready to land.Nov 18 2021, 4:52 PM
Closed by commit rGf8e615462b4f: [AMDGPU] Fix SIPostRABundler crash on null register used by dbg value (authored by rampitec). · Explain WhyNov 18 2021, 5:01 PM
This revision was automatically updated to reflect the committed changes.
rampitec added a commit: rGf8e615462b4f: [AMDGPU] Fix SIPostRABundler crash on null register used by dbg value.

Revision Contents

PathSize
llvm/
lib/
Target/
AMDGPU/
3 lines
test/
CodeGen/
AMDGPU/
18 lines

Diff 388356

llvm/lib/Target/AMDGPU/SIPostRABundler.cpp

Show First 20 Lines • Show All 84 Lines • ▼ Show 20 Lines for (Register Def : Defs)
return true; return true;
} }
return false; return false;
} }
void SIPostRABundler::collectUsedRegUnits(const MachineInstr &MI, void SIPostRABundler::collectUsedRegUnits(const MachineInstr &MI,
BitVector &UsedRegUnits) const { BitVector &UsedRegUnits) const {
if (MI.isDebugInstr())
arsenmUnsubmitted
Not Done
ReplyInline Actions

This allows all meta instructions in the bundle region. Should it skip all of them?

arsenm: This allows all meta instructions in the bundle region. Should it skip all of them?
rampitecAuthorUnsubmitted
Done
ReplyInline Actions

I do not think so. This a least handles KILL which is also meta.

rampitec: I do not think so. This a least handles KILL which is also meta.
return;
for (const MachineOperand &Op : MI.operands()) { for (const MachineOperand &Op : MI.operands()) {
if (!Op.isReg() || !Op.readsReg()) if (!Op.isReg() || !Op.readsReg())
continue; continue;
Register Reg = Op.getReg(); Register Reg = Op.getReg();
assert(!Op.getSubReg() && assert(!Op.getSubReg() &&
"subregister indexes should not be present after RA"); "subregister indexes should not be present after RA");
▲ Show 20 Lines • Show All 113 Lines • Show Last 20 Lines

llvm/test/CodeGen/AMDGPU/postra-bundle-memops.mir

Show First 20 Lines • Show All 314 Lines • ▼ Show 20 Lines bb.0:
; GCN-NEXT: } ; GCN-NEXT: }
$vgpr0 = GLOBAL_LOAD_DWORD $vgpr3_vgpr4, 0, 0, implicit $exec $vgpr0 = GLOBAL_LOAD_DWORD $vgpr3_vgpr4, 0, 0, implicit $exec
$vgpr1 = GLOBAL_LOAD_DWORD $vgpr5_vgpr6, 0, 0, implicit $exec $vgpr1 = GLOBAL_LOAD_DWORD $vgpr5_vgpr6, 0, 0, implicit $exec
KILL killed $vgpr3 KILL killed $vgpr3
KILL $vgpr4 KILL $vgpr4
KILL killed $vgpr5 KILL killed $vgpr5
KILL killed $vgpr6 KILL killed $vgpr6
... ...
---
name: post_bundle_kill_and_null_reg_dbginfo
body: |
bb.0:
liveins: $vgpr3_vgpr4, $vgpr5_vgpr6
; GCN-LABEL: name: post_bundle_kill_and_null_reg_dbginfo
; GCN: BUNDLE implicit-def $vgpr0, implicit-def $vgpr0_lo16, implicit-def $vgpr0_hi16, implicit-def $vgpr1, implicit-def $vgpr1_lo16, implicit-def $vgpr1_hi16, implicit $vgpr3_vgpr4, implicit $exec, implicit $vgpr5_vgpr6 {
; GCN-NEXT: $vgpr0 = GLOBAL_LOAD_DWORD $vgpr3_vgpr4, 0, 0, implicit $exec
; GCN-NEXT: DBG_VALUE $noreg, $noreg
; GCN-NEXT: $vgpr1 = GLOBAL_LOAD_DWORD $vgpr5_vgpr6, 0, 0, implicit $exec
; GCN-NEXT: }
$vgpr0 = GLOBAL_LOAD_DWORD $vgpr3_vgpr4, 0, 0, implicit $exec
DBG_VALUE $noreg, $noreg
$vgpr1 = GLOBAL_LOAD_DWORD $vgpr5_vgpr6, 0, 0, implicit $exec
KILL killed $vgpr3_vgpr4, killed $vgpr5_vgpr6
...