<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 11/14/2014 03:35 AM, Tom Stellard
wrote:<br>
</div>
<blockquote
cite="mid:1415964906-19306-1-git-send-email-thomas.stellard@amd.com"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">---
docs/R600Usage.rst | 4 +
lib/Target/R600/AsmParser/AMDGPUAsmParser.cpp | 159 +++++++++++++++++++++++---
lib/Target/R600/SIInstrInfo.td | 4 +-
3 files changed, 153 insertions(+), 14 deletions(-)
diff --git a/docs/R600Usage.rst b/docs/R600Usage.rst
index 48a30c8..2282d54 100644
--- a/docs/R600Usage.rst
+++ b/docs/R600Usage.rst
@@ -15,6 +15,10 @@ Assembler
The assembler is currently a work in progress and not yet complete. Below
are the currently supported features.
+SMRD Instructions
+-----------------
+The assembler currently supports only the s_load_dword* SMRD instructions.
+
SOPP Instructions
-----------------
diff --git a/lib/Target/R600/AsmParser/AMDGPUAsmParser.cpp b/lib/Target/R600/AsmParser/AMDGPUAsmParser.cpp
index 205de5f..9f817bf 100644
--- a/lib/Target/R600/AsmParser/AMDGPUAsmParser.cpp
+++ b/lib/Target/R600/AsmParser/AMDGPUAsmParser.cpp
@@ -69,7 +69,8 @@ public:
class AMDGPUOperand : public MCParsedAsmOperand {
enum KindTy {
Token,
- Immediate
+ Immediate,
+ Register
} Kind;
public:
@@ -84,16 +85,21 @@ public:
int64_t Val;
};
+ struct RegOp {
+ unsigned RegNo;
+ };
+
union {
TokOp Tok;
ImmOp Imm;
+ RegOp Reg;
};
void addImmOperands(MCInst &Inst, unsigned N) const {
Inst.addOperand(MCOperand::CreateImm(getImm()));
}
void addRegOperands(MCInst &Inst, unsigned N) const {
- llvm_unreachable("addRegOperands");
+ Inst.addOperand(MCOperand::CreateReg(getReg()));
}
StringRef getToken() const {
return StringRef(Tok.Data, Tok.Length);
@@ -111,11 +117,11 @@ public:
}
bool isReg() const override {
- return false;
+ return Kind == Register;
}
unsigned getReg() const override {
- return 0;
+ return Reg.RegNo;
}
bool isMem() const override {
@@ -145,13 +151,125 @@ public:
return Res;
}
+ static std::unique_ptr<AMDGPUOperand> CreateReg(unsigned RegNo, SMLoc S,
+ SMLoc E) {
+ auto Op = llvm::make_unique<AMDGPUOperand>(Register);
+ Op->Reg.RegNo = RegNo;
+ return Op;
+ }
+
bool isSWaitCnt() const;
};
}
bool AMDGPUAsmParser::ParseRegister(unsigned &RegNo, SMLoc &StartLoc, SMLoc &EndLoc) {
- return true;
+ const AsmToken Tok = Parser.getTok();
+ StartLoc = Tok.getLoc();
+ EndLoc = Tok.getEndLoc();
+ const StringRef &RegName = Tok.getString();
+ RegNo = 0;
+
+ // Handle special cases
+ if (RegName.equals("vcc_lo"))
+ RegNo = AMDGPU::VCC_LO;
+ else if (RegName.equals("vcc_hi"))
+ RegNo = AMDGPU::VCC_HI;
+ else if (RegName.equals("VCC"))</pre>
</div>
</blockquote>
Why are some of these registers still capitalized?<br>
<blockquote
cite="mid:1415964906-19306-1-git-send-email-thomas.stellard@amd.com"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">
+ RegNo = AMDGPU::VCC;
+ else if (RegName.equals("exec_lo"))
+ RegNo = AMDGPU::EXEC_LO;
+ else if (RegName.equals("exec_hi"))
+ RegNo = AMDGPU::EXEC_HI;
+ else if (RegName.equals("EXEC"))
+ RegNo = AMDGPU::EXEC;
+ else if (RegName.equals("M0"))
+ RegNo = AMDGPU::M0;
+ else if (RegName.equals("flat_scr_lo"))
+ RegNo = AMDGPU::FLAT_SCR_LO;
+ else if (RegName.equals("flat_scr_hi"))
+ RegNo = AMDGPU::FLAT_SCR_HI;
+ else if (RegName.equals("FLAT_SCR"))
+ RegNo = AMDGPU::FLAT_SCR;
+ else if (RegName.equals("SCC"))
+ RegNo = AMDGPU::SCC;
+
+ if (RegNo)
+ return false;
+
+ // Match vgprs and sgprs
+ if (RegName[0] != 's' && RegName[0] != 'v')
+ return true;
+
+ bool IsVgpr = RegName[0] == 'v';
+ unsigned RegWidth;
+ unsigned RegIndexInClass;
+ if (RegName.size() > 1) {
+ // We have a 32-bit register
+ RegWidth = 1;
+ APInt RegIndex;
+ if (RegName.substr(1).getAsInteger(10, RegIndex))</pre>
</div>
</blockquote>
I don't think you need APInt here. You should be able to use a
regular integer type instead of APInt + getSExtValue<br>
<blockquote
cite="mid:1415964906-19306-1-git-send-email-thomas.stellard@amd.com"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">
+ return true;
+ RegIndexInClass = RegIndex.getSExtValue();
+ Parser.Lex();
+ } else {
+ // We have a register greater than 32-bits.
+
+ int64_t RegLo, RegHi;
+ Parser.Lex();
+ if (getLexer().isNot(AsmToken::LBrac))
+ return true;
+
+ Parser.Lex();
+ if (getParser().parseAbsoluteExpression(RegLo))
+ return true;
+
+ if (getLexer().isNot(AsmToken::Colon))
+ return true;
+
+ Parser.Lex();
+ if (getParser().parseAbsoluteExpression(RegHi))
+ return true;
+
+ if (getLexer().isNot(AsmToken::RBrac))
+ return true;
+
+ Parser.Lex();
+ RegWidth = ((RegHi - RegLo) + 1);</pre>
</div>
</blockquote>
Extra parentheses outside<br>
<blockquote
cite="mid:1415964906-19306-1-git-send-email-thomas.stellard@amd.com"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">
+ if (IsVgpr) {
+ // VGPR registers aren't aligned.
+ RegIndexInClass = RegLo;
+ } else {
+ // SGPR registers are aligned. Max alignment is 4 dwords.
+ RegIndexInClass = RegLo / std::min((int)RegWidth, 4);</pre>
</div>
</blockquote>
It's easier to add a u suffix to the 4 than to cast to int<br>
<blockquote
cite="mid:1415964906-19306-1-git-send-email-thomas.stellard@amd.com"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">
+ }
+ }
+
+ unsigned RC;
+ const MCRegisterInfo *TRC = getContext().getRegisterInfo();
+ if (IsVgpr) {
+ switch (RegWidth) {
+ default: llvm_unreachable("Unknown register width");
+ case 1: RC = AMDGPU::VGPR_32RegClassID; break;
+ case 2: RC = AMDGPU::VReg_64RegClassID; break;
+ case 3: RC = AMDGPU::VReg_96RegClassID; break;
+ case 4: RC = AMDGPU::VReg_128RegClassID; break;
+ case 8: RC = AMDGPU::VReg_256RegClassID; break;
+ case 16: RC = AMDGPU::VReg_512RegClassID; break;
+ }
+ } else {
+ switch (RegWidth) {
+ default: llvm_unreachable("Unknown register width");
+ case 1: RC = AMDGPU::SGPR_32RegClassID; break;
+ case 2: RC = AMDGPU::SGPR_64RegClassID; break;
+ case 4: RC = AMDGPU::SReg_128RegClassID; break;
+ case 8: RC = AMDGPU::SReg_256RegClassID; break;
+ case 16: RC = AMDGPU::SReg_512RegClassID; break;
+ }
+ }</pre>
</div>
</blockquote>
These switches should be their own functions that return. Also I
think the prevailing style is the cases should be indented to the
same width as the start of the switch, but right now R600 has a mix
of both<br>
<br>
<blockquote
cite="mid:1415964906-19306-1-git-send-email-thomas.stellard@amd.com"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">
+
+ RegNo = TRC->getRegClass(RC).getRegister(RegIndexInClass);
+ return false;
}
@@ -207,6 +325,14 @@ AMDGPUAsmParser::parseOperand(OperandVector &Operands, StringRef Mnemonic) {
Operands.push_back(AMDGPUOperand::CreateImm(IntVal));
return MatchOperand_Success;
}
+ case AsmToken::Identifier: {
+ SMLoc S, E;
+ unsigned RegNo;
+ if (ParseRegister(RegNo, S, E))
+ return MatchOperand_NoMatch;
+ Operands.push_back(AMDGPUOperand::CreateReg(RegNo, S, E));
+ return MatchOperand_Success;
+ }
default:
return MatchOperand_NoMatch;
}
@@ -218,16 +344,23 @@ bool AMDGPUAsmParser::ParseInstruction(ParseInstructionInfo &Info,
// Add the instruction mnemonic
Operands.push_back(AMDGPUOperand::CreateToken(Name, NameLoc));
- if (getLexer().is(AsmToken::EndOfStatement))
- return false;
+ while (!getLexer().is(AsmToken::EndOfStatement)) {
- AMDGPUAsmParser::OperandMatchResultTy Res = parseOperand(Operands, Name);
- switch (Res) {
- case MatchOperand_Success: return false;
- case MatchOperand_ParseFail: return Error(NameLoc,
- "Failed parsing operand");
- case MatchOperand_NoMatch: return Error(NameLoc, "Not a valid operand");
+ AMDGPUAsmParser::OperandMatchResultTy Res = parseOperand(Operands, Name);
+
+ // Eat the comma if there is one.
+ if (getLexer().is(AsmToken::Comma))
+ Parser.Lex();
+
+ switch (Res) {
+ case MatchOperand_Success: break;
+ case MatchOperand_ParseFail: return Error(getLexer().getLoc(),
+ "Failed parsing operand");
+ case MatchOperand_NoMatch: return Error(getLexer().getLoc(),
+ "Not a valid operand");
+ }</pre>
</div>
</blockquote>
I think the convention is to not capitalize any error messages<br>
<blockquote
cite="mid:1415964906-19306-1-git-send-email-thomas.stellard@amd.com"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">
}
+ return false;
}
//===----------------------------------------------------------------------===//
diff --git a/lib/Target/R600/SIInstrInfo.td b/lib/Target/R600/SIInstrInfo.td
index ec4ac9a..346a601 100644
--- a/lib/Target/R600/SIInstrInfo.td
+++ b/lib/Target/R600/SIInstrInfo.td
@@ -392,7 +392,9 @@ multiclass SMRD_m <bits<5> op, string opName, bit imm, dag outs, dag ins,
def "" : SMRD_Pseudo <opName, outs, ins, pattern>;
- def _si : SMRD_Real_si <op, opName, imm, outs, ins, asm>;
+ let isCodeGenOnly = 0 in {
+ def _si : SMRD_Real_si <op, opName, imm, outs, ins, asm>;
+ }
}
<div class="moz-txt-sig">--
1.8.1.5
_______________________________________________
llvm-commits mailing list
<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a>
</div></pre>
</div>
</blockquote>
<br>
</body>
</html>