Skip to content

Commit c9cdaff

Browse files
authored
[AMDGPU] Fix operand definitions for atomic scalar memory instructions. (llvm#71799)
CPol and CPol_GLC1 operand classes have identical predicates, which means AsmParser cannot differentiate between the RTN and non-RTN variants of the instructions. When it currently selects the wrong instruction, a hack in cvtSMEMAtomic() corrects the op-code. Using the new predicated-value operands makes this hack and the whole conversion function not needed. Other uses of CPol_GLC1 operands are to be addressed separately. Resolves about half of the remaining ~1000 pairs of ambiguous instructions. Part of <llvm#69256>.
1 parent 8474bfd commit c9cdaff

File tree

4 files changed

+21
-64
lines changed

4 files changed

+21
-64
lines changed

llvm/lib/Target/AMDGPU/AMDGPUInstructions.td

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,20 @@ class ImmOperand<ValueType type, string name = NAME, bit optional = 0,
165165
def s16imm : ImmOperand<i16, "S16Imm", 0, "printU16ImmOperand">;
166166
def u16imm : ImmOperand<i16, "U16Imm", 0, "printU16ImmOperand">;
167167

168+
class ValuePredicatedOperand<CustomOperand op, string valuePredicate,
169+
bit optional = 0>
170+
: CustomOperand<op.Type, optional> {
171+
let ImmTy = op.ImmTy;
172+
defvar OpPredicate = op.ParserMatchClass.PredicateMethod;
173+
let PredicateMethod =
174+
"getPredicate([](const AMDGPUOperand &Op) -> bool { "#
175+
"return Op."#OpPredicate#"() && "#valuePredicate#"; })";
176+
let ParserMethod = op.ParserMatchClass.ParserMethod;
177+
let DefaultValue = op.DefaultValue;
178+
let DefaultMethod = op.DefaultMethod;
179+
let PrintMethod = op.PrintMethod;
180+
}
181+
168182
//===--------------------------------------------------------------------===//
169183
// Custom Operands
170184
//===--------------------------------------------------------------------===//

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

Lines changed: 4 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -909,6 +909,10 @@ class AMDGPUOperand : public MCParsedAsmOperand {
909909
bool isWaitVDST() const;
910910
bool isWaitEXP() const;
911911

912+
auto getPredicate(std::function<bool(const AMDGPUOperand &Op)> P) const {
913+
return std::bind(P, *this);
914+
}
915+
912916
StringRef getToken() const {
913917
assert(isToken());
914918
return StringRef(Tok.Data, Tok.Length);
@@ -1773,7 +1777,6 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
17731777

17741778
void cvtVOP3Interp(MCInst &Inst, const OperandVector &Operands);
17751779
void cvtVINTERP(MCInst &Inst, const OperandVector &Operands);
1776-
void cvtSMEMAtomic(MCInst &Inst, const OperandVector &Operands);
17771780

17781781
bool parseDimId(unsigned &Encoding);
17791782
ParseStatus parseDim(OperandVector &Operands);
@@ -7722,66 +7725,6 @@ void AMDGPUAsmParser::cvtMubufImpl(MCInst &Inst,
77227725
addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyCPol, 0);
77237726
}
77247727

7725-
//===----------------------------------------------------------------------===//
7726-
// SMEM
7727-
//===----------------------------------------------------------------------===//
7728-
7729-
void AMDGPUAsmParser::cvtSMEMAtomic(MCInst &Inst, const OperandVector &Operands) {
7730-
OptionalImmIndexMap OptionalIdx;
7731-
bool IsAtomicReturn = false;
7732-
7733-
for (unsigned i = 1, e = Operands.size(); i != e; ++i) {
7734-
AMDGPUOperand &Op = ((AMDGPUOperand &)*Operands[i]);
7735-
if (!Op.isCPol())
7736-
continue;
7737-
IsAtomicReturn = Op.getImm() & AMDGPU::CPol::GLC;
7738-
break;
7739-
}
7740-
7741-
if (!IsAtomicReturn) {
7742-
int NewOpc = AMDGPU::getAtomicNoRetOp(Inst.getOpcode());
7743-
if (NewOpc != -1)
7744-
Inst.setOpcode(NewOpc);
7745-
}
7746-
7747-
IsAtomicReturn = MII.get(Inst.getOpcode()).TSFlags &
7748-
SIInstrFlags::IsAtomicRet;
7749-
7750-
for (unsigned i = 1, e = Operands.size(); i != e; ++i) {
7751-
AMDGPUOperand &Op = ((AMDGPUOperand &)*Operands[i]);
7752-
7753-
// Add the register arguments
7754-
if (Op.isReg()) {
7755-
Op.addRegOperands(Inst, 1);
7756-
if (IsAtomicReturn && i == 1)
7757-
Op.addRegOperands(Inst, 1);
7758-
continue;
7759-
}
7760-
7761-
// Handle the case where soffset is an immediate
7762-
if (Op.isImm() && Op.getImmTy() == AMDGPUOperand::ImmTyNone) {
7763-
Op.addImmOperands(Inst, 1);
7764-
continue;
7765-
}
7766-
7767-
// Handle tokens like 'offen' which are sometimes hard-coded into the
7768-
// asm string. There are no MCInst operands for these.
7769-
if (Op.isToken()) {
7770-
continue;
7771-
}
7772-
assert(Op.isImm());
7773-
7774-
// Handle optional arguments
7775-
OptionalIdx[Op.getImmTy()] = i;
7776-
}
7777-
7778-
if ((int)Inst.getNumOperands() <=
7779-
AMDGPU::getNamedOperandIdx(Inst.getOpcode(), AMDGPU::OpName::offset))
7780-
addOptionalImmOperand(Inst, Operands, OptionalIdx,
7781-
AMDGPUOperand::ImmTySMEMOffsetMod);
7782-
addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyCPol, 0);
7783-
}
7784-
77857728
//===----------------------------------------------------------------------===//
77867729
// smrd
77877730
//===----------------------------------------------------------------------===//

llvm/lib/Target/AMDGPU/SIInstrInfo.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,6 +1078,8 @@ def highmod : NamedBitOperand<"high", "High">;
10781078
def CPol : CustomOperand<i32, 1>;
10791079
def CPol_0 : DefaultOperand<CPol, 0>;
10801080
def CPol_GLC1 : DefaultOperand<CPol, 1>;
1081+
def CPol_GLC : ValuePredicatedOperand<CPol, "Op.getImm() & CPol::GLC">;
1082+
def CPol_NonGLC : ValuePredicatedOperand<CPol, "!(Op.getImm() & CPol::GLC)", 1>;
10811083

10821084
def TFE : NamedBitOperand<"tfe">;
10831085
def UNorm : NamedBitOperand<"unorm">;

llvm/lib/Target/AMDGPU/SMInstructions.td

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,6 @@ class SM_Atomic_Pseudo <string opName,
234234

235235
let IsAtomicNoRet = !not(isRet);
236236
let IsAtomicRet = isRet;
237-
238-
let AsmMatchConverter = "cvtSMEMAtomic";
239237
}
240238

241239
class SM_Pseudo_Atomic<string opName,
@@ -245,7 +243,7 @@ class SM_Pseudo_Atomic<string opName,
245243
bit isRet,
246244
string opNameWithSuffix =
247245
opName # offsets.Variant # !if(isRet, "_RTN", ""),
248-
Operand CPolTy = !if(isRet, CPol_GLC1, CPol)> :
246+
Operand CPolTy = !if(isRet, CPol_GLC, CPol_NonGLC)> :
249247
SM_Atomic_Pseudo<opName,
250248
!if(isRet, (outs dataClass:$sdst), (outs)),
251249
!con((ins dataClass:$sdata, baseClass:$sbase), offsets.Ins,

0 commit comments

Comments
 (0)