Skip to content

Commit

Permalink
[AMDGPU][llvm-mc] Fixes to support buffer atomics.
Browse files Browse the repository at this point in the history 
Fixes for MUBUF_Atomic instructions to make operand list valid:
 - For RTN insns, make a copy of $vdata_in operand as $vdata.
 - Do not add operand for GLC, it is hardcoded and comes as a token.
Workaround to avoid adding multiple default optional operands.
Tests added.

Differential Revision: http://reviews.llvm.org/D20257

llvm-svn: 270049
  • Loading branch information
@atamazov
atamazov committed May 19, 2016
1 parent 5f94ced commit 8ce1f71
Show file tree
Hide file tree
Showing 5 changed files with 413 additions and 12 deletions.
86 changes: 81 additions & 5 deletions llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,17 @@ const char* const OpGsSymbolic[] = {

using namespace llvm;

// In some cases (e.g. buffer atomic instructions) MatchOperandParserImpl()
// may invoke tryCustomParseOperand() multiple times with the same MCK value.
// That leads to adding of the same "default" operand multiple times in a row,
// which is wrong. The workaround adds only the 1st default operand, while for
// the rest the "dummy" operands being added. The reason for dummies is that if
// we just skip adding an operand, then parser would get stuck in endless loop.
// Dummies shall be removed prior matching & emitting MCInsts.
//
// Comment out this macro to disable the workaround.
#define WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS

namespace {

struct OptionalOperand;
Expand All @@ -99,6 +110,9 @@ class AMDGPUOperand : public MCParsedAsmOperand {
Immediate,
Register,
Expression
#ifdef WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
,Dummy
#endif
} Kind;

SMLoc StartLoc, EndLoc;
Expand Down Expand Up @@ -204,6 +218,12 @@ class AMDGPUOperand : public MCParsedAsmOperand {
}
}

#ifdef WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
bool isDummy() const {
return Kind == Dummy;
}
#endif

bool isToken() const override {
return Kind == Token;
}
Expand Down Expand Up @@ -440,6 +460,11 @@ class AMDGPUOperand : public MCParsedAsmOperand {
case Expression:
OS << "<expr " << *Expr << '>';
break;
#ifdef WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
case Dummy:
OS << "<dummy>";
break;
#endif
}
}

Expand Down Expand Up @@ -490,6 +515,15 @@ class AMDGPUOperand : public MCParsedAsmOperand {
return Op;
}

#ifdef WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
static AMDGPUOperand::Ptr CreateDummy(SMLoc S) {
auto Op = llvm::make_unique<AMDGPUOperand>(Dummy);
Op->StartLoc = S;
Op->EndLoc = S;
return Op;
}
#endif

bool isSWaitCnt() const;
bool isHwreg() const;
bool isSendMsg() const;
Expand Down Expand Up @@ -545,6 +579,7 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
bool ParseSectionDirectiveHSARodataReadonlyAgent();
bool AddNextRegisterToList(unsigned& Reg, unsigned& RegWidth, RegisterKind RegKind, unsigned Reg1, unsigned RegNum);
bool ParseAMDGPURegister(RegisterKind& RegKind, unsigned& Reg, unsigned& RegNum, unsigned& RegWidth);
void cvtMubufImpl(MCInst &Inst, const OperandVector &Operands, bool IsAtomic, bool IsAtomicReturn);

public:
enum AMDGPUMatchResultTy {
Expand Down Expand Up @@ -633,8 +668,9 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
OperandMatchResultTy parseSOppBrTarget(OperandVector &Operands);
AMDGPUOperand::Ptr defaultHwreg() const;


void cvtMubuf(MCInst &Inst, const OperandVector &Operands);
void cvtMubuf(MCInst &Inst, const OperandVector &Operands) { cvtMubufImpl(Inst, Operands, false, false); }
void cvtMubufAtomic(MCInst &Inst, const OperandVector &Operands) { cvtMubufImpl(Inst, Operands, true, false); }
void cvtMubufAtomicReturn(MCInst &Inst, const OperandVector &Operands) { cvtMubufImpl(Inst, Operands, true, true); }
AMDGPUOperand::Ptr defaultMubufOffset() const;
AMDGPUOperand::Ptr defaultGLC() const;
AMDGPUOperand::Ptr defaultSLC() const;
Expand Down Expand Up @@ -926,6 +962,17 @@ bool AMDGPUAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
bool MatchingInlineAsm) {
MCInst Inst;

#ifdef WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
// Remove dummies prior matching. Iterate backwards becase vector::erase()
// invalidates all iterators which refer after erase point.
for (auto I = Operands.rbegin(), E = Operands.rend(); I != E; ) {
auto X = I++;
if (static_cast<AMDGPUOperand*>(X->get())->isDummy()) {
Operands.erase(X.base() -1);
}
}
#endif

switch (MatchInstructionImpl(Operands, Inst, ErrorInfo, MatchingInlineAsm)) {
default: break;
case Match_Success:
Expand Down Expand Up @@ -1430,6 +1477,25 @@ AMDGPUAsmParser::parseIntWithPrefix(const char *Prefix, OperandVector &Operands,
}

Operands.push_back(AMDGPUOperand::CreateImm(Value, S, ImmTy));

#ifdef WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
if (Value == Default && AddDefault) {
// Reverse lookup in previously added operands (skip just added one)
// for the first non-dummy operand. If it is of the same type,
// then replace just added default operand with dummy.
for (auto I = Operands.rbegin(), E = Operands.rend(); I != E; ++I) {
if (I == Operands.rbegin())
continue;
if (static_cast<AMDGPUOperand*>(I->get())->isDummy())
continue;
if (static_cast<AMDGPUOperand*>(I->get())->isImmTy(ImmTy)) {
Operands.pop_back();
Operands.push_back(AMDGPUOperand::CreateDummy(S)); // invalidates iterators
break;
}
}
}
#endif
return MatchOperand_Success;
}

Expand Down Expand Up @@ -2047,9 +2113,11 @@ AMDGPUOperand::Ptr AMDGPUAsmParser::defaultTFE() const {
return AMDGPUOperand::CreateImm(0, SMLoc(), AMDGPUOperand::ImmTyTFE);
}

void AMDGPUAsmParser::cvtMubuf(MCInst &Inst,
const OperandVector &Operands) {
void AMDGPUAsmParser::cvtMubufImpl(MCInst &Inst,
const OperandVector &Operands,
bool IsAtomic, bool IsAtomicReturn) {
OptionalImmIndexMap OptionalIdx;
assert(IsAtomicReturn ? IsAtomic : true);

for (unsigned i = 1, e = Operands.size(); i != e; ++i) {
AMDGPUOperand &Op = ((AMDGPUOperand &)*Operands[i]);
Expand Down Expand Up @@ -2077,8 +2145,16 @@ void AMDGPUAsmParser::cvtMubuf(MCInst &Inst,
OptionalIdx[Op.getImmTy()] = i;
}

// Copy $vdata_in operand and insert as $vdata for MUBUF_Atomic RTN insns.
if (IsAtomicReturn) {
MCInst::iterator I = Inst.begin(); // $vdata_in is always at the beginning.
Inst.insert(I, *I);
}

addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyOffset);
addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyGLC);
if (!IsAtomic) { // glc is hard-coded.
addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyGLC);
}
addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTySLC);
addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyTFE);
}
Expand Down
13 changes: 7 additions & 6 deletions llvm/lib/Target/AMDGPU/SIInstrInfo.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2836,7 +2836,7 @@ multiclass MUBUF_Atomic <mubuf op, string name, RegisterClass rc,
let mayStore = 1, mayLoad = 1, hasPostISelHook = 1, hasSideEffects = 1 in {

// No return variants
let glc = 0 in {
let glc = 0, AsmMatchConverter = "cvtMubufAtomic" in {

defm _ADDR64 : MUBUFAtomicAddr64_m <
op, name#"_addr64", (outs),
Expand Down Expand Up @@ -2883,13 +2883,14 @@ multiclass MUBUF_Atomic <mubuf op, string name, RegisterClass rc,

// Variant that return values
let glc = 1, Constraints = "$vdata = $vdata_in",
AsmMatchConverter = "cvtMubufAtomicReturn",
DisableEncoding = "$vdata_in" in {

defm _RTN_ADDR64 : MUBUFAtomicAddr64_m <
op, name#"_rtn_addr64", (outs rc:$vdata),
(ins rc:$vdata_in, VReg_64:$vaddr, SReg_128:$srsrc,
SCSrc_32:$soffset, offset:$offset, slc:$slc),
name#" $vdata, $vaddr, $srsrc, $soffset addr64"#"$offset"#" glc"#"$slc",
name#" $vdata, $vaddr, $srsrc, $soffset addr64$offset glc$slc",
[(set vt:$vdata,
(atomic (MUBUFAddr64Atomic v4i32:$srsrc, i64:$vaddr, i32:$soffset,
i16:$offset, i1:$slc), vt:$vdata_in))], 1
Expand All @@ -2899,7 +2900,7 @@ multiclass MUBUF_Atomic <mubuf op, string name, RegisterClass rc,
op, name#"_rtn_offset", (outs rc:$vdata),
(ins rc:$vdata_in, SReg_128:$srsrc, SCSrc_32:$soffset,
offset:$offset, slc:$slc),
name#" $vdata, off, $srsrc, $soffset $offset glc$slc",
name#" $vdata, off, $srsrc, $soffset$offset glc$slc",
[(set vt:$vdata,
(atomic (MUBUFOffsetAtomic v4i32:$srsrc, i32:$soffset, i16:$offset,
i1:$slc), vt:$vdata_in))], 1
Expand All @@ -2910,7 +2911,7 @@ multiclass MUBUF_Atomic <mubuf op, string name, RegisterClass rc,
op, name#"_rtn_offen", (outs rc:$vdata),
(ins rc:$vdata_in, VGPR_32:$vaddr, SReg_128:$srsrc, SCSrc_32:$soffset,
offset:$offset, slc:$slc),
name#" $vdata, $vaddr, $srsrc, $soffset offen"#"$offset"#" glc"#"$slc",
name#" $vdata, $vaddr, $srsrc, $soffset offen$offset glc$slc",
[], 1
>;
}
Expand All @@ -2920,7 +2921,7 @@ multiclass MUBUF_Atomic <mubuf op, string name, RegisterClass rc,
op, name#"_rtn_idxen", (outs rc:$vdata),
(ins rc:$vdata_in, VGPR_32:$vaddr, SReg_128:$srsrc, SCSrc_32:$soffset,
offset:$offset, slc:$slc),
name#" $vdata, $vaddr, $srsrc, $soffset idxen"#"$offset"#" glc"#"$slc",
name#" $vdata, $vaddr, $srsrc, $soffset idxen$offset glc$slc",
[], 1
>;
}
Expand All @@ -2930,7 +2931,7 @@ multiclass MUBUF_Atomic <mubuf op, string name, RegisterClass rc,
op, name#"_rtn_bothen", (outs rc:$vdata),
(ins rc:$vdata_in, VReg_64:$vaddr, SReg_128:$srsrc, SCSrc_32:$soffset,
offset:$offset, slc:$slc),
name#" $vdata, $vaddr, $srsrc, $soffset idxen offen"#"$offset"#" glc"#"$slc",
name#" $vdata, $vaddr, $srsrc, $soffset idxen offen$offset glc$slc",
[], 1
>;
}
Expand Down

0 comments on commit 8ce1f71

Please sign in to comment.