[PATCH] R600/SI: Add assembler support for s_load_dword* instructions

Matt Arsenault Matthew.Arsenault at amd.com
Fri Nov 14 11:55:00 PST 2014


On 11/14/2014 03:35 AM, Tom Stellard wrote:
> ---
>   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"))
Why are some of these registers still capitalized?
> +    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))
I don't think you need APInt here. You should be able to use a regular 
integer type instead of APInt + getSExtValue
> +      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);
Extra parentheses outside
> +    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);
It's easier to add a u suffix to the 4 than to cast to int
> +    }
> +  }
> +
> +  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;
> +    }
> +  }
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

> +
> +  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");
> +    }
I think the convention is to not capitalize any error messages
>     }
> +  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>;
> +  }
>   
>   }
>   
> -- 1.8.1.5 _______________________________________________ 
> llvm-commits mailing list llvm-commits at cs.uiuc.edu 
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141114/9169c7a9/attachment.html>


More information about the llvm-commits mailing list