This is an archive of the discontinued LLVM Phabricator instance.

Differential D68350

[AMDGPU][MC][GFX9][GFX10] Corrected parsing of optional operands
ClosedPublic

Authored by dp on Oct 2 2019, 11:21 AM.

Details

Reviewers
artem.tamazov
arsenm
Commits
rL374553: [AMDGPU][MC] Corrected parsing of optional operands
rG882c3e3db52d: [AMDGPU][MC] Corrected parsing of optional operands
Summary

See bug 43486: https://bugs.llvm.org/show_bug.cgi?id=43486

Diff Detail

Event Timeline

dp created this revision.Oct 2 2019, 11:21 AM
Herald added subscribers: jfb, t-tye, tpr and 7 others. · View Herald TranscriptOct 2 2019, 11:21 AM
dp marked an inline comment as done.Oct 2 2019, 11:30 AM
dp added inline comments.
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
6089

This check is no longer necessary. Moreover it does not work for the following test because 'off' is not a register:

global_atomic_add v0, v[1:2], v2, off glc slc
dp added reviewers: artem.tamazov, arsenm.Oct 2 2019, 11:32 AM
artem.tamazov accepted this revision.Oct 7 2019, 4:18 AM

Make it so!

This revision is now accepted and ready to land.Oct 7 2019, 4:18 AM
Closed by commit rG882c3e3db52d: [AMDGPU][MC] Corrected parsing of optional operands (authored by dp). · Explain WhyOct 11 2019, 7:08 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 11 2019, 7:08 AM
Herald added subscribers: llvm-commits, hiraditya. · View Herald Transcript

Revision Contents

PathSize
lib/
Target/
AMDGPU/
AsmParser/
16 lines
test/
MC/
AMDGPU/
5 lines

Diff 222884

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

Show First 20 Lines • Show All 6,080 Lines • ▼ Show 20 LinesOperandMatchResultTy AMDGPUAsmParser::parseOptionalOperand(OperandVector &Operands) {
// Current design assumes that all operands after the first optional operand // Current design assumes that all operands after the first optional operand
// are also optional. However implementation of some instructions violates // are also optional. However implementation of some instructions violates
// this rule (see e.g. flat/global atomic which have hardcoded 'glc' operands). // this rule (see e.g. flat/global atomic which have hardcoded 'glc' operands).
// //
// To alleviate this problem, we have to (implicitly) parse extra operands // To alleviate this problem, we have to (implicitly) parse extra operands
// to make sure autogenerated parser of custom operands never hit hardcoded // to make sure autogenerated parser of custom operands never hit hardcoded
// mandatory operands. // mandatory operands.
if (size == 1 || ((AMDGPUOperand &)*Operands[size - 1]).isRegKind()) {
dpAuthorUnsubmitted
Done
ReplyInline Actions

This check is no longer necessary. Moreover it does not work for the following test because 'off' is not a register:

global_atomic_add v0, v[1:2], v2, off glc slc
dp: This check is no longer necessary. Moreover it does not work for the following test because…
// We have parsed the first optional operand.
// Parse as many operands as necessary to skip all mandatory operands.
for (unsigned i = 0; i < MAX_OPR_LOOKAHEAD; ++i) { for (unsigned i = 0; i < MAX_OPR_LOOKAHEAD; ++i) {
if (res != MatchOperand_Success || if (res != MatchOperand_Success ||
getLexer().is(AsmToken::EndOfStatement)) break; isToken(AsmToken::EndOfStatement))
if (getLexer().is(AsmToken::Comma)) Parser.Lex(); break;
trySkipToken(AsmToken::Comma);
res = parseOptionalOpr(Operands); res = parseOptionalOpr(Operands);
} }
}
return res; return res;
} }
OperandMatchResultTy AMDGPUAsmParser::parseOptionalOpr(OperandVector &Operands) { OperandMatchResultTy AMDGPUAsmParser::parseOptionalOpr(OperandVector &Operands) {
OperandMatchResultTy res; OperandMatchResultTy res;
for (const OptionalOperand &Op : AMDGPUOptionalOperandTable) { for (const OptionalOperand &Op : AMDGPUOptionalOperandTable) {
// try to parse any optional operand here // try to parse any optional operand here
▲ Show 20 Lines • Show All 901 Lines • Show Last 20 Lines

test/MC/AMDGPU/flat-global.s

Show First 20 Lines • Show All 520 Lines • ▼ Show 20 Lines
// GFX10: encoding: [0x00,0x80,0x64,0xdc,0x03,0x01,0x7d,0x00] // GFX10: encoding: [0x00,0x80,0x64,0xdc,0x03,0x01,0x7d,0x00]
// GFX9: global_store_byte_d16_hi v[3:4], v1, off ; encoding: [0x00,0x80,0x64,0xdc,0x03,0x01,0x7f,0x00] // GFX9: global_store_byte_d16_hi v[3:4], v1, off ; encoding: [0x00,0x80,0x64,0xdc,0x03,0x01,0x7f,0x00]
// VI-ERR: instruction not supported on this GPU // VI-ERR: instruction not supported on this GPU
global_store_short_d16_hi v[3:4], v1, off global_store_short_d16_hi v[3:4], v1, off
// GFX10: encoding: [0x00,0x80,0x6c,0xdc,0x03,0x01,0x7d,0x00] // GFX10: encoding: [0x00,0x80,0x6c,0xdc,0x03,0x01,0x7d,0x00]
// GFX9: global_store_short_d16_hi v[3:4], v1, off ; encoding: [0x00,0x80,0x6c,0xdc,0x03,0x01,0x7f,0x00] // GFX9: global_store_short_d16_hi v[3:4], v1, off ; encoding: [0x00,0x80,0x6c,0xdc,0x03,0x01,0x7f,0x00]
// VI-ERR: instruction not supported on this GPU // VI-ERR: instruction not supported on this GPU
global_atomic_add v0, v[1:2], v2, off glc slc
// GFX10: global_atomic_add v0, v[1:2], v2, off glc slc ; encoding: [0x00,0x80,0xcb,0xdc,0x01,0x02,0x7d,0x00]
// GFX9: global_atomic_add v0, v[1:2], v2, off glc slc ; encoding: [0x00,0x80,0x0b,0xdd,0x01,0x02,0x7f,0x00]
// VI-ERR: error: invalid operand for instruction