AMDGPU/AsmParser: Add support for parsing symbol operands

[• tstellarAMD]

Summary

We can now reference symbols directly in operands, like this:
s_mov_b32 s0, global

[• tstellarAMD]

Add testcase.

[arsenm]

I thought symbols usually had some special prefix character to distinguish them

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

149

This looks redundant with the cast<>

1339

Typo difficlut

[• tstellarAMD]

Remove an extra call to Lex().

[• tstellarAMD]

Fix typo.

[• tstellarAMD]

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

149

It's not redundant, because Expr could be any of the MCExpr sub-classes. For example in the added tests case, Expr is sometimes an MCBinaryExpr.

[vpykhtin]

LGTM.

[artem.tamazov]

Please address comment about simple tests and go ahead.

test/MC/AMDGPU/global-expr.s

7–8

I woudl be nice to add also tests for simple absolute expressions, like

.set foo 4
s_mov_b32 s0, foo+2 // should load 6 to s0

[• tstellarAMD]

In D21038#450845, @artem.tamazov wrote:

Please address comment about simple tests and go ahead.

There was an unrelated bug with these expression, so I've added the test with D21236, which also fixes the bug.

[artem.tamazov]

It seems that labels in branches were not anticipated in this patch.
The quickfix is under review, http://reviews.llvm.org/D22133.

Shall we try another approach for this feature; this one looks like a hack?..
Another problem (different story, but newertheless):

.set foo 40
s_branch foo

Branch offset will be something like (40-PC) but correct one should be +40, I guess. Shall we differentiate between labels and other kinds of symbols?

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

if (Imm.Type == ImmTyNone && ApplyModifiers && Imm.Modifiers != 0) {
         Val = Negate ? -Val : Val;
       }
       Inst.addOperand(MCOperand::createImm(Val));
     } else {
       Inst.addOperand(MCOperand::createImm(getImm()));
     }
   }
 
+  StringRef getExpressionAsToken() const {
+    assert(isExpr());
+    const MCSymbolRefExpr *S = cast<MCSymbolRefExpr>(Expr);
+    return S->getSymbol().getName();
+  }
+
+
   StringRef getToken() const {
+    assert(isToken());
+
+    if (Kind == Expression)
+      return getExpressionAsToken();
+
     return StringRef(Tok.Data, Tok.Length);
   }
 
   void addRegOperands(MCInst &Inst, unsigned N) const {
     Inst.addOperand(MCOperand::createReg(AMDGPU::getMCReg(getReg(), *Reg.STI)));
   }
 
   void addRegOrImmOperands(MCInst &Inst, unsigned N) const {
     if (isRegKind())
       addRegOperands(Inst, N);
+    else if (isExpr())
+      Inst.addOperand(MCOperand::createExpr(Expr));
     else
       addImmOperands(Inst, N);
   }
 
   void addRegOrImmWithInputModsOperands(MCInst &Inst, unsigned N) const {
     if (isRegKind()) {
       Inst.addOperand(MCOperand::createImm(Reg.Modifiers));
       addRegOperands(Inst, N);
     } else {
       Inst.addOperand(MCOperand::createImm(Imm.Modifiers));
       addImmOperands(Inst, N, false);
     }
   }
 
   void addSoppBrTargetOperands(MCInst &Inst, unsigned N) const {
     if (isImm())
       addImmOperands(Inst, N);
     else {
       assert(isExpr());
       Inst.addOperand(MCOperand::createExpr(Expr));
     }
   }
 
   bool isToken() const override {
-    return Kind == Token;
+    if (Kind == Token)
+      return true;
+
+    if (Kind != Expression || !Expr)
+      return false;
+
+    // When parsing operands, we can't always tell if something was meant to be
+    // a token, like 'gds', or an expression that references a global variable.
+    // In this case, we assume the string is an expression, and if we need to
+    // interpret is a token, then we treat the symbol name as the token.
+    return isa<MCSymbolRefExpr>(Expr);
   }
 
   bool isImm() const override {
     return Kind == Immediate;
   }
 
   bool isInlinableImm() const {
     if (!isImm() || Imm.Type != AMDGPUOperand::ImmTyNone /* Only plain
bool isSCSrc32() const {
     return isInlinableImm() || isRegClass(AMDGPU::SReg_32RegClassID);
   }
 
   bool isSCSrc64() const {
     return isInlinableImm() || isRegClass(AMDGPU::SReg_64RegClassID);
   }
 
   bool isSSrc32() const {
-    return isImm() || isSCSrc32();
+    return isImm() || isSCSrc32() || isExpr();
   }
 
   bool isSSrc64() const {
     // TODO: Find out how SALU supports extension of 32-bit literals to 64 bits.
     // See isVSrc64().
     return isImm() || isSCSrc64();
   }
 
if (ResTy == MatchOperand_Success || ResTy == MatchOperand_ParseFail ||
     return ResTy;
 
   ResTy = parseRegOrImm(Operands);
 
   if (ResTy == MatchOperand_Success)
     return ResTy;
 
   if (getLexer().getKind() == AsmToken::Identifier) {
+    // If this identifier is a symbol, we want to create an expression for it.
+    // It is a little difficult to distinguish between a symbol name, and
+    // an instruction flag like 'gds'.  In order to do this, we parse
+    // all tokens as expressions and then treate the symbol name as the token
+    // string when we want to interpret the operand as a token.
     const auto &Tok = Parser.getTok();
+    SMLoc S = Tok.getLoc();
+    const MCExpr *Expr = nullptr;
+    if (!Parser.parseExpression(Expr)) {
+      Operands.push_back(AMDGPUOperand::CreateExpr(Expr, S));
+      return MatchOperand_Success;
+    }
+
     Operands.push_back(AMDGPUOperand::CreateToken(Tok.getString(), Tok.getLoc()));
     Parser.Lex();
     return MatchOperand_Success;
   }
   return MatchOperand_NoMatch;
 }
 
 bool AMDGPUAsmParser::ParseInstruction(ParseInstructionInfo &Info,
unsigned AMDGPUAsmParser::validateTargetOperandClass(MCParsedAsmOperand &Op,
   case MCK_gds:
     return Operand.isGDS() ? Match_Success : Match_InvalidOperand;
   case MCK_glc:
     return Operand.isGLC() ? Match_Success : Match_InvalidOperand;
   case MCK_idxen:
     return Operand.isIdxen() ? Match_Success : Match_InvalidOperand;
   case MCK_offen:
     return Operand.isOffen() ? Match_Success : Match_InvalidOperand;
+  case MCK_SSrc32:
+    // When operands have expression values, they will return true for isToken,
+    // because it is not possible to distinguish between a token and an
+    // expression at parse time. MatchInstructionImpl() will always try to
+    // match an operand as a token, when isToken returns true, and when the
+    // name of the expression is not a valid token, the match will fail,
+    // so we need to handle it here.
+    return Operand.isSSrc32() ? Match_Success : Match_InvalidOperand;
   default: return Match_InvalidOperand;
   }
 }

test/MC/AMDGPU/global-expr.s

+// RUN: llvm-mc -arch=amdgcn -mcpu=fiji -show-encoding %s | FileCheck %s --check-prefix=VI
+
+
+.globl global
+.globl gds
+
+// Parse a global expression
+s_mov_b32 s0, global
+// VI: s_mov_b32 s0, global ; encoding: [0xff,0x00,0x80,0xbe,A,A,A,A]
+// VI-NEXT: ;   fixup A - offset: 4, value: global, kind: FK_PCRel_4
+
+// Use a token with the same name as a global
+ds_gws_init v2 gds
+// VI: ds_gws_init v2 gds ; encoding: [0x00,0x00,0x33,0xd8,0x02,0x00,0x00,0x00]
+
+// Use a global with the same name as a token
+s_mov_b32 s0, gds
+// VI: s_mov_b32 s0, gds ; encoding: [0xff,0x00,0x80,0xbe,A,A,A,A]
+// VI-NEXT: ;   fixup A - offset: 4, value: gds, kind: FK_PCRel_4
+
+// Use a binary expression
+s_mov_b32 s0, gds+4
+// VI: s_mov_b32 s0, gds+4 ; encoding: [0xff,0x00,0x80,0xbe,A,A,A,A]
+// VI-NEXT: ;   fixup A - offset: 4, value: gds+4, kind: FK_PCRel_4
+
+// Consecutive instructions with no blank line in between to make sure we
+// don't call Lex() too many times.
+
+s_add_u32 s0, s0, global+4
+s_addc_u32 s1, s1, 0
+// VI: s_add_u32 s0, s0, global+4
+// VI: s_addc_u32 s1, s1, 0