This is an archive of the discontinued LLVM Phabricator instance.

Differential D92654

[AMDGPU] Fix default value of glc for mubuf rtn atomics
ClosedPublic

Authored by Petar.Avramovic on Dec 4 2020, 6:20 AM.

Details

Reviewers
foad
rampitec
dp
Commits
rG3a042dcd2e1a: [AMDGPU] Fix default value of glc for mubuf rtn atomics
Summary

Mubuf rtn atomics use GLC_1 thus default value for glc operand
should be -1, see https://reviews.llvm.org/D90730.
This allows us to report error when rtn atomic requires glc=1
but does not have glc operand in input.

Diff Detail

Event Timeline

Petar.Avramovic created this revision.Dec 4 2020, 6:20 AM
Herald added subscribers: kerbowa, jfb, hiraditya and 8 others. · View Herald TranscriptDec 4 2020, 6:20 AM
Petar.Avramovic requested review of this revision.Dec 4 2020, 6:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 4 2020, 6:20 AM
Herald added subscribers: llvm-commits, wdng. · View Herald Transcript
foad added a comment.Dec 4 2020, 6:36 AM

Looks right to me but I'd like @dp or @rampitec to check it too.

dp accepted this revision.Dec 7 2020, 3:16 AM

LGTM

This revision is now accepted and ready to land.Dec 7 2020, 3:16 AM
Closed by commit rG3a042dcd2e1a: [AMDGPU] Fix default value of glc for mubuf rtn atomics (authored by Petar.Avramovic). · Explain WhyDec 7 2020, 5:00 AM
This revision was automatically updated to reflect the committed changes.
Petar.Avramovic added a commit: rG3a042dcd2e1a: [AMDGPU] Fix default value of glc for mubuf rtn atomics.

Revision Contents

PathSize
llvm/
lib/
Target/
AMDGPU/
AsmParser/
3 lines
test/
MC/
AMDGPU/
6 lines
2 lines

Diff 309881

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

Show First 20 Lines • Show All 6,685 Lines • ▼ Show 20 Lines if (IsLdsOpcode && !HasLdsModifier) {
if (NoLdsOpcode != -1) { // Got lds version - correct it. if (NoLdsOpcode != -1) { // Got lds version - correct it.
Inst.setOpcode(NoLdsOpcode); Inst.setOpcode(NoLdsOpcode);
IsLdsOpcode = false; IsLdsOpcode = false;
} }
} }
addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyOffset); addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyOffset);
if (!IsAtomic || IsAtomicReturn) { if (!IsAtomic || IsAtomicReturn) {
addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyGLC); addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyGLC,
IsAtomicReturn ? -1 : 0);
} }
addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTySLC); addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTySLC);
if (!IsLdsOpcode) { // tfe is not legal with lds opcodes if (!IsLdsOpcode) { // tfe is not legal with lds opcodes
addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyTFE); addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyTFE);
} }
if (isGFX10Plus()) if (isGFX10Plus())
▲ Show 20 Lines • Show All 1,186 Lines • Show Last 20 Lines

llvm/test/MC/AMDGPU/gfx1030_err.s

Show First 20 Lines • Show All 134 Lines • ▼ Show 20 Lines
ds_max_src2_f64 v1 offset:65535 ds_max_src2_f64 v1 offset:65535
// GFX10: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU // GFX10: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
ds_write_src2_b32 v1 offset:65535 ds_write_src2_b32 v1 offset:65535
// GFX10: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU // GFX10: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
ds_write_src2_b64 v1 offset:65535 ds_write_src2_b64 v1 offset:65535
// GFX10: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU // GFX10: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
buffer_atomic_csub v5, off, s[8:11], s3 offset:4095
// GFX10: :[[@LINE-1]]:{{[0-9]+}}: error: instruction must use glc
global_atomic_csub v2, v[0:1], v2, off offset:100 slc
// GFX10: :[[@LINE-1]]:{{[0-9]+}}: error: instruction must use glc

llvm/test/MC/AMDGPU/gfx1030_new.s

Show All 21 Lines
// GFX10: encoding: [0x00,0x80,0xd1,0xdc,0x00,0x02,0x7d,0x02] // GFX10: encoding: [0x00,0x80,0xd1,0xdc,0x00,0x02,0x7d,0x02]
global_atomic_csub v2, v0, v2, s[2:3] glc global_atomic_csub v2, v0, v2, s[2:3] glc
// GFX10: encoding: [0x00,0x80,0xd1,0xdc,0x00,0x02,0x02,0x02] // GFX10: encoding: [0x00,0x80,0xd1,0xdc,0x00,0x02,0x02,0x02]
global_atomic_csub v2, v0, v2, s[2:3] offset:100 glc slc global_atomic_csub v2, v0, v2, s[2:3] offset:100 glc slc
// GFX10: encoding: [0x64,0x80,0xd3,0xdc,0x00,0x02,0x02,0x02] // GFX10: encoding: [0x64,0x80,0xd3,0xdc,0x00,0x02,0x02,0x02]
buffer_atomic_csub v5, off, s[8:11], s3 buffer_atomic_csub v5, off, s[8:11], s3 glc
// GFX10: encoding: [0x00,0x40,0xd0,0xe0,0x00,0x05,0x02,0x03] // GFX10: encoding: [0x00,0x40,0xd0,0xe0,0x00,0x05,0x02,0x03]
buffer_atomic_csub v5, off, s[8:11], s3 offset:4095 glc buffer_atomic_csub v5, off, s[8:11], s3 offset:4095 glc
// GFX10: encoding: [0xff,0x4f,0xd0,0xe0,0x00,0x05,0x02,0x03] // GFX10: encoding: [0xff,0x4f,0xd0,0xe0,0x00,0x05,0x02,0x03]
buffer_atomic_csub v5, off, s[8:11], -1 offset:4095 glc buffer_atomic_csub v5, off, s[8:11], -1 offset:4095 glc
// GFX10: encoding: [0xff,0x4f,0xd0,0xe0,0x00,0x05,0x02,0xc1] // GFX10: encoding: [0xff,0x4f,0xd0,0xe0,0x00,0x05,0x02,0xc1]
▲ Show 20 Lines • Show All 74 Lines • Show Last 20 Lines