This is an archive of the discontinued LLVM Phabricator instance.

Differential D26633

AMDGPU/GCN: Exit early in hazard recognizer if there is no vreg argument
ClosedPublic

Authored by jvesely on Nov 14 2016, 1:27 PM.

Details

Reviewers
• tstellarAMD
arsenm
Commits
rGe8cc395e4f64: AMDGPU/GCN: Exit early in hazard recognizer if there is no vreg argument
rL287056: AMDGPU/GCN: Exit early in hazard recognizer if there is no vreg argument
Summary

wbinvl.* are vector instructions that do not use vector registers.

Diff Detail

Repository
rL LLVM

Event Timeline

jvesely updated this revision to Diff 77874.Nov 14 2016, 1:27 PM
jvesely retitled this revision from to AMDGPU/GCN: Exit early in hazard recognizer if there is no vreg argument.
jvesely updated this object.
jvesely added reviewers: • tstellarAMD, arsenm.
jvesely set the repository for this revision to rL LLVM.
jvesely added a subscriber: llvm-commits.
Herald edited edge metadata. · View Herald TranscriptNov 14 2016, 1:27 PM
Herald added subscribers: tony-tye, yaxunl, nhaehnle and 2 others. · View Herald Transcript
• tstellarAMD accepted this revision.Nov 14 2016, 3:46 PM
• tstellarAMD edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Nov 14 2016, 3:46 PM
arsenm added inline comments.Nov 14 2016, 4:04 PM
lib/Target/AMDGPU/GCNHazardRecognizer.cpp
399–403

Could you invert this so it's an if (== -1) return -1

jvesely updated this revision to Diff 77921.Nov 14 2016, 4:52 PM
jvesely edited edge metadata.

cleanup

jvesely marked an inline comment as done.Nov 14 2016, 4:53 PM
arsenm accepted this revision.Nov 14 2016, 4:54 PM
arsenm edited edge metadata.

LGTM

jvesely updated this revision to Diff 77932.Nov 14 2016, 6:05 PM
jvesely edited edge metadata.

v2: only checks for M?BUF instructions.

fixes regression in inserted-wait-states.mir

jvesely requested a review of this revision.Nov 15 2016, 11:13 AM
jvesely edited edge metadata.
arsenm accepted this revision.Nov 15 2016, 12:04 PM
arsenm edited edge metadata.

LGTM

This revision is now accepted and ready to land.Nov 15 2016, 12:04 PM
Closed by commit rL287056: AMDGPU/GCN: Exit early in hazard recognizer if there is no vreg argument (authored by jvesely). · Explain WhyNov 15 2016, 4:05 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
lib/
Target/
AMDGPU/
7 lines
test/
CodeGen/
AMDGPU/
4 lines

Diff 77921

lib/Target/AMDGPU/GCNHazardRecognizer.cpp

Context not available.
const MCInstrDesc &Desc = MI.getDesc(); const MCInstrDesc &Desc = MI.getDesc();
int VDataIdx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::vdata); int VDataIdx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::vdata);
int VDataRCID = -1; if (VDataIdx == -1)
if (VDataIdx != -1) return -1;
VDataRCID = Desc.OpInfo[VDataIdx].RegClass;
int VDataRCID = Desc.OpInfo[VDataIdx].RegClass;
if (TII->isMUBUF(MI) || TII->isMTBUF(MI)) { if (TII->isMUBUF(MI) || TII->isMTBUF(MI)) {
arsenmUnsubmitted
Done
ReplyInline Actions

Could you invert this so it's an if (== -1) return -1

arsenm: Could you invert this so it's an if (== -1) return -1
// For MUBUF/MTBUF instructions this hazard only exists if the // For MUBUF/MTBUF instructions this hazard only exists if the
Context not available.

test/CodeGen/AMDGPU/llvm.amdgcn.buffer.wbinvl1.vol.ll

Context not available.
; GCN-NEXT: ; BB#0: ; GCN-NEXT: ; BB#0:
; CI-NEXT: buffer_wbinvl1_vol ; encoding: [0x00,0x00,0xc0,0xe1,0x00,0x00,0x00,0x00] ; CI-NEXT: buffer_wbinvl1_vol ; encoding: [0x00,0x00,0xc0,0xe1,0x00,0x00,0x00,0x00]
; VI-NEXT: buffer_wbinvl1_vol ; encoding: [0x00,0x00,0xfc,0xe0,0x00,0x00,0x00,0x00] ; VI-NEXT: buffer_wbinvl1_vol ; encoding: [0x00,0x00,0xfc,0xe0,0x00,0x00,0x00,0x00]
; GCN-NEXT: s_endpgm ; GCN: s_endpgm
define void @test_buffer_wbinvl1_vol() #0 { define void @test_buffer_wbinvl1_vol() #0 {
call void @llvm.amdgcn.buffer.wbinvl1.vol() call void @llvm.amdgcn.buffer.wbinvl1.vol()
; This used to crash in hazard recognizer
store i8 0, i8 addrspace(1)* undef, align 1
ret void ret void
} }
Context not available.