This is an archive of the discontinued LLVM Phabricator instance.

Differential D123348

[AMDGPU] Fix inline asm causing assert during PreRARematerialize stage in scheduler pass
ClosedPublic

Authored by vangthao on Apr 7 2022, 5:51 PM.

Details

Reviewers
kerbowa
rampitec
foad
Commits
rGcd1071171c5c: [AMDGPU] Fix inline asm causing assert during PreRARematerialize stage in…

Diff Detail

Unit TestsFailed

TimeTest
60,140 msx64 debian > AddressSanitizer-x86_64-linux-dynamic.TestCases::scariness_score_test.cpp
60,060 msx64 debian > AddressSanitizer-x86_64-linux.TestCases::scariness_score_test.cpp
790 msx64 debian > SanitizerCommon-tsan-x86_64-Linux.Linux::decorate_proc_maps.cpp
60,050 msx64 debian > libFuzzer.libFuzzer::fuzzer-leak.test
60,080 msx64 debian > libFuzzer.libFuzzer::large.test
View Full Test Results (8 Failed)

Event Timeline

vangthao created this revision.Apr 7 2022, 5:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2022, 5:51 PM
Herald added subscribers: hsmhsm, foad, kerbowa and 10 others. · View Herald Transcript
vangthao requested review of this revision.Apr 7 2022, 5:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2022, 5:51 PM
Herald added subscribers: llvm-commits, wdng. · View Herald Transcript
vangthao added reviewers: kerbowa, rampitec.Apr 7 2022, 5:52 PM
arsenm added inline comments.Apr 7 2022, 6:24 PM
llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
744

Null dereference before null check

Harbormaster completed remote builds in B158598: Diff 421374.Apr 7 2022, 6:40 PM
foad added inline comments.Apr 8 2022, 2:15 AM
llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
744

getOneDef already returns a specific operand, so it seems odd to ignore this and look at operand 0 of the same instruction instead. Maybe check getOperandNo() == 0 instead if that's what you want?

745

How could Def ever be null here?

vangthao updated this revision to Diff 421536.Apr 8 2022, 8:26 AM

Remove redundant null check and use MachineOperand from getOneDef.

vangthao added inline comments.Apr 8 2022, 8:27 AM
llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
744

You're right, thanks for catching this!

foad accepted this revision.Apr 8 2022, 8:40 AM

LGTM.

This revision is now accepted and ready to land.Apr 8 2022, 8:40 AM
Harbormaster completed remote builds in B158706: Diff 421536.Apr 8 2022, 9:17 AM
This revision was landed with ongoing or failed builds.Apr 8 2022, 9:24 AM
Closed by commit rGcd1071171c5c: [AMDGPU] Fix inline asm causing assert during PreRARematerialize stage in… (authored by vangthao). · Explain Why
This revision was automatically updated to reflect the committed changes.
vangthao added a commit: rGcd1071171c5c: [AMDGPU] Fix inline asm causing assert during PreRARematerialize stage in….

Revision Contents

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

Diff 421374

llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp

Show First 20 Lines • Show All 735 Lines • ▼ Show 20 Lines for (unsigned I = 0, E = MRI.getNumVirtRegs(); I != E; ++I) {
// We are only collecting defs that are live-through or defined in another // We are only collecting defs that are live-through or defined in another
// block and used inside this region. This means that the register must be // block and used inside this region. This means that the register must be
// in the live-in set for this region, else skip this def. // in the live-in set for this region, else skip this def.
if (HighRPLiveIns.find(Reg) == HighRPLiveIns.end()) if (HighRPLiveIns.find(Reg) == HighRPLiveIns.end())
continue; continue;
MachineInstr *Def = MRI.getOneDef(Reg)->getParent(); MachineInstr *Def = MRI.getOneDef(Reg)->getParent();
if (!Def || Def->getOperand(0).getSubReg() != 0 || MachineOperand &Op = Def->getOperand(0);
arsenmUnsubmitted
Not Done
ReplyInline Actions

Null dereference before null check

arsenm: Null dereference before null check
foadUnsubmitted
Not Done
ReplyInline Actions

getOneDef already returns a specific operand, so it seems odd to ignore this and look at operand 0 of the same instruction instead. Maybe check getOperandNo() == 0 instead if that's what you want?

foad: getOneDef already returns a specific operand, so it seems odd to ignore this and look at…
vangthaoAuthorUnsubmitted
Done
ReplyInline Actions

You're right, thanks for catching this!

vangthao: You're right, thanks for catching this!
if (!Def || !Op.isReg() || Op.getSubReg() != 0 ||
foadUnsubmitted
Not Done
ReplyInline Actions

How could Def ever be null here?

foad: How could Def ever be null here?
!isTriviallyReMaterializable(*Def, AA)) !isTriviallyReMaterializable(*Def, AA))
continue; continue;
MachineInstr *UseI = &*MRI.use_instr_nodbg_begin(Reg); MachineInstr *UseI = &*MRI.use_instr_nodbg_begin(Reg);
if (Def->getParent() == UseI->getParent()) if (Def->getParent() == UseI->getParent())
continue; continue;
RematerializableInsts.push_back(std::make_pair(Def, UseI)); RematerializableInsts.push_back(std::make_pair(Def, UseI));
▲ Show 20 Lines • Show All 178 Lines • Show Last 20 Lines

llvm/test/CodeGen/AMDGPU/machine-scheduler-sink-trivial-remats.mir

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 5,945 Lines • ▼ Show 20 Lines ; predcessors: %bb.1
S_NOP 0, implicit %12, implicit %13 S_NOP 0, implicit %12, implicit %13
S_NOP 0, implicit %14, implicit %15 S_NOP 0, implicit %14, implicit %15
S_NOP 0, implicit %16, implicit %17 S_NOP 0, implicit %16, implicit %17
S_NOP 0, implicit %18, implicit %19 S_NOP 0, implicit %18, implicit %19
S_NOP 0, implicit %20, implicit %21 S_NOP 0, implicit %20, implicit %21
S_NOP 0, implicit %22 S_NOP 0, implicit %22
S_ENDPGM 0 S_ENDPGM 0
... ...
---
name: test_occ_9_crash_inline_asm
tracksRegLiveness: true
machineFunctionInfo:
isEntryFunction: true
body: |
; GFX908-LABEL: name: test_occ_9_crash_inline_asm
; GFX908: bb.0:
; GFX908-NEXT: successors: %bb.1(0x80000000)
; GFX908-NEXT: {{ $}}
; GFX908-NEXT: %0:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 0, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %1:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 1, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %2:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 2, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %3:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 3, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %4:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 4, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %5:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 5, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %6:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 6, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %7:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 7, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %8:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 8, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %9:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 9, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %10:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 10, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %11:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 11, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %12:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 12, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %13:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 13, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %14:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 14, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %15:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 15, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %16:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 16, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %17:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 17, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %18:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 18, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %19:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 19, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %20:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 20, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: %21:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 21, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: INLINEASM &"v_or_b32 $0, 0, $1", 32 /* isconvergent attdialect */, 327690 /* regdef:SReg_1_with_sub0 */, def %22, 327689 /* reguse:SReg_1_with_sub0 */, %4
; GFX908-NEXT: {{ $}}
; GFX908-NEXT: bb.1:
; GFX908-NEXT: successors: %bb.2(0x80000000)
; GFX908-NEXT: {{ $}}
; GFX908-NEXT: %24:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 24, implicit $exec, implicit $mode, implicit-def $m0
; GFX908-NEXT: S_NOP 0, implicit %24
; GFX908-NEXT: {{ $}}
; GFX908-NEXT: bb.2:
; GFX908-NEXT: %23:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 23, implicit $exec, implicit $mode
; GFX908-NEXT: S_NOP 0, implicit %23
; GFX908-NEXT: S_NOP 0, implicit %0, implicit %1
; GFX908-NEXT: S_NOP 0, implicit %2, implicit %3
; GFX908-NEXT: S_NOP 0, implicit %4, implicit %5
; GFX908-NEXT: S_NOP 0, implicit %6, implicit %7
; GFX908-NEXT: S_NOP 0, implicit %8, implicit %9
; GFX908-NEXT: S_NOP 0, implicit %10, implicit %11
; GFX908-NEXT: S_NOP 0, implicit %12, implicit %13
; GFX908-NEXT: S_NOP 0, implicit %14, implicit %15
; GFX908-NEXT: S_NOP 0, implicit %16, implicit %17
; GFX908-NEXT: S_NOP 0, implicit %18, implicit %19
; GFX908-NEXT: S_NOP 0, implicit %20, implicit %21
; GFX908-NEXT: S_NOP 0, implicit %22
; GFX908-NEXT: S_ENDPGM 0
bb.0:
successors: %bb.1
%0:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 0, implicit $exec, implicit $mode, implicit-def $m0
%1:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 1, implicit $exec, implicit $mode, implicit-def $m0
%2:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 2, implicit $exec, implicit $mode, implicit-def $m0
%3:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 3, implicit $exec, implicit $mode, implicit-def $m0
%4:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 4, implicit $exec, implicit $mode, implicit-def $m0
%5:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 5, implicit $exec, implicit $mode, implicit-def $m0
%6:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 6, implicit $exec, implicit $mode, implicit-def $m0
%7:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 7, implicit $exec, implicit $mode, implicit-def $m0
%8:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 8, implicit $exec, implicit $mode, implicit-def $m0
%9:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 9, implicit $exec, implicit $mode, implicit-def $m0
%10:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 10, implicit $exec, implicit $mode, implicit-def $m0
%11:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 11, implicit $exec, implicit $mode, implicit-def $m0
%12:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 12, implicit $exec, implicit $mode, implicit-def $m0
%13:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 13, implicit $exec, implicit $mode, implicit-def $m0
%14:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 14, implicit $exec, implicit $mode, implicit-def $m0
%15:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 15, implicit $exec, implicit $mode, implicit-def $m0
%16:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 16, implicit $exec, implicit $mode, implicit-def $m0
%17:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 17, implicit $exec, implicit $mode, implicit-def $m0
%18:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 18, implicit $exec, implicit $mode, implicit-def $m0
%19:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 19, implicit $exec, implicit $mode, implicit-def $m0
%20:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 20, implicit $exec, implicit $mode, implicit-def $m0
%21:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 21, implicit $exec, implicit $mode, implicit-def $m0
INLINEASM &"v_or_b32 $0, 0, $1", 32, 327690, def %22:vgpr_32, 327689, %4
%23:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 23, implicit $exec, implicit $mode
bb.1:
; predecessors: %bb.0
successors: %bb.2
%24:vgpr_32 = nofpexcept V_CVT_I32_F64_e32 24, implicit $exec, implicit $mode, implicit-def $m0
S_NOP 0, implicit %24
bb.2:
; predcessors: %bb.1
S_NOP 0, implicit %23
S_NOP 0, implicit %0, implicit %1
S_NOP 0, implicit %2, implicit %3
S_NOP 0, implicit %4, implicit %5
S_NOP 0, implicit %6, implicit %7
S_NOP 0, implicit %8, implicit %9
S_NOP 0, implicit %10, implicit %11
S_NOP 0, implicit %12, implicit %13
S_NOP 0, implicit %14, implicit %15
S_NOP 0, implicit %16, implicit %17
S_NOP 0, implicit %18, implicit %19
S_NOP 0, implicit %20, implicit %21
S_NOP 0, implicit %22
S_ENDPGM 0
...