This is an archive of the discontinued LLVM Phabricator instance.

Differential D48772

[AMDGPU] Add VALU to V_INTERP Instructions
ClosedPublic

Authored by rtaylor on Jun 29 2018, 7:25 AM.

Details

Reviewers
tpr
dstuttard
arsenm
nhaehnle
Commits
rG5f04458a61e7: [AMDGPU] Add VALU to V_INTERP Instructions
rL336339: [AMDGPU] Add VALU to V_INTERP Instructions
Summary

Wait states are not properly being inserted after buffer_store for v_interp instructions.

Add VALU to V_INTERP instructions so that the GCNHazardRecognizer can
check and insert the appropriate wait states when needed.

Change-Id: I3da47a7889a5fd98a4c5ecf02b806e9b9faa16b3

Diff Detail

Repository
rL LLVM

Event Timeline

rtaylor created this revision.Jun 29 2018, 7:25 AM
Herald added subscribers: llvm-commits, t-tye, tpr and 6 others. · View Herald TranscriptJun 29 2018, 7:25 AM
Harbormaster completed remote builds in B19879: Diff 153489.Jun 29 2018, 7:25 AM
rtaylor added reviewers: tpr, dstuttard, arsenm, nhaehnle.Jun 29 2018, 7:27 AM
arsenm added inline comments.Jun 29 2018, 7:59 AM
lib/Target/AMDGPU/SIInstructions.td
39 ↗(On Diff #153489)

Should probably be moved down to the base instruction class where the others set this

rtaylor updated this revision to Diff 153503.Jun 29 2018, 8:52 AM

Moved VALU=1 to the base instruction defs.

Harbormaster completed remote builds in B19882: Diff 153503.Jun 29 2018, 8:53 AM
arsenm added a comment.Jun 29 2018, 10:01 AM

I meant all the way down in VINTRPCommon. I really dislike using let blocks when avoidable

rtaylor updated this revision to Diff 153530.Jun 29 2018, 10:41 AM

Changed VALU to be set in base instruction class.

rtaylor updated this revision to Diff 153533.Jun 29 2018, 10:43 AM

Removed empty line insert in SIInstructions.td

arsenm accepted this revision.Jun 29 2018, 10:50 AM

LGTM with test using GCN-NEXT

test/CodeGen/AMDGPU/hazard-buffer-store-v-interp.mir
6–8 ↗(On Diff #153533)

Should use GCN-NEXT

This revision is now accepted and ready to land.Jun 29 2018, 10:50 AM
rtaylor updated this revision to Diff 153542.Jun 29 2018, 10:59 AM

Adding GCN-NEXT in test case

arsenm accepted this revision.Jun 29 2018, 11:01 AM

LGTM

Closed by commit rL336339: [AMDGPU] Add VALU to V_INTERP Instructions (authored by rtayl). · Explain WhyJul 5 2018, 5:07 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
trunk/
lib/
Target/
AMDGPU/
1 line
test/
CodeGen/
AMDGPU/
19 lines

Diff 154206

llvm/trunk/lib/Target/AMDGPU/SIInstrFormats.td

Show First 20 Lines • Show All 313 Lines • ▼ Show 20 Linesclass VINTRPCommon <dag outs, dag ins, string asm, list<dag> pattern> :
// memory access which we consider to be undefined behavior. // memory access which we consider to be undefined behavior.
// //
// So even though these instructions read memory, this memory is outside the // So even though these instructions read memory, this memory is outside the
// addressable memory space for the shader, and we consider these instructions // addressable memory space for the shader, and we consider these instructions
// to be readnone. // to be readnone.
let mayLoad = 0; let mayLoad = 0;
let mayStore = 0; let mayStore = 0;
let hasSideEffects = 0; let hasSideEffects = 0;
let VALU = 1;
} }
class EXPCommon<dag outs, dag ins, string asm, list<dag> pattern> : class EXPCommon<dag outs, dag ins, string asm, list<dag> pattern> :
InstSI<outs, ins, asm, pattern> { InstSI<outs, ins, asm, pattern> {
let EXP = 1; let EXP = 1;
let EXP_CNT = 1; let EXP_CNT = 1;
let mayLoad = 0; // Set to 1 if done bit is set. let mayLoad = 0; // Set to 1 if done bit is set.
let mayStore = 1; let mayStore = 1;
let UseNamedOperandTable = 1; let UseNamedOperandTable = 1;
let Uses = [EXEC]; let Uses = [EXEC];
let SchedRW = [WriteExport]; let SchedRW = [WriteExport];
} }
} // End Uses = [EXEC] } // End Uses = [EXEC]

llvm/trunk/test/CodeGen/AMDGPU/hazard-buffer-store-v-interp.mir

# RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs -run-pass post-RA-hazard-rec %s -o - | FileCheck -check-prefix=GCN -check-prefix=VI %s
# RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs -run-pass post-RA-hazard-rec %s -o - | FileCheck -check-prefix=GCN -check-prefix=GFX9 %s
# GCN-LABEL: name: hazard_buffer_store_v_interp
# GCN: bb.0.entry:
# GCN-NEXT: BUFFER_STORE_DWORDX4_OFFSET_exact
# GCN-NEXT: S_NOP
# GCN-NEXT: V_INTERP_P1_F32
name: hazard_buffer_store_v_interp
body: |
bb.0.entry:
liveins: $sgpr0, $sgpr1, $sgpr2, $sgpr3, $vgpr0, $vgpr1, $vgpr7, $vgpr8, $vgpr9, $vgpr10
BUFFER_STORE_DWORDX4_OFFSET_exact killed $vgpr7_vgpr8_vgpr9_vgpr10, $sgpr4_sgpr5_sgpr6_sgpr7, 0, 96, 0, 0, 0, implicit $exec
$vgpr7 = V_INTERP_P1_F32 $vgpr0, 0, 0, implicit $m0, implicit $exec
S_ENDPGM
...