[AMDGPU][MC] Corrected parsing of branch offsets

See bug 40820: https://bugs.llvm.org/show_bug.cgi?id=40820

Reviewers: artem.tamazov, arsenm

Differential Revision: https://reviews.llvm.org/D64629

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@366571 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

@@ -216,14 +216,15 @@ class AMDGPUOperand : public MCParsedAsmOperand {
     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);
+    return isSymbolRefExpr();
+  }
+
+  bool isSymbolRefExpr() const {
+    return isExpr() && Expr && isa<MCSymbolRefExpr>(Expr);
   }
 
   bool isImm() const override {
@@ -1321,6 +1322,7 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
   void peekTokens(MutableArrayRef<AsmToken> Tokens);
   AsmToken::TokenKind getTokenKind() const;
   bool parseExpr(int64_t &Imm);
+  bool parseExpr(OperandVector &Operands);
   StringRef getTokenStr() const;
   AsmToken peekToken();
   AsmToken getToken() const;
@@ -5224,6 +5226,23 @@ AMDGPUAsmParser::parseExpr(int64_t &Imm) {
   return !getParser().parseAbsoluteExpression(Imm);
 }
 
+bool
+AMDGPUAsmParser::parseExpr(OperandVector &Operands) {
+  SMLoc S = getLoc();
+
+  const MCExpr *Expr;
+  if (Parser.parseExpression(Expr))
+    return false;
+
+  int64_t IntVal;
+  if (Expr->evaluateAsAbsolute(IntVal)) {
+    Operands.push_back(AMDGPUOperand::CreateImm(this, IntVal, S));
+  } else {
+    Operands.push_back(AMDGPUOperand::CreateExpr(this, Expr, S));
+  }
+  return true;
+}
+
 bool
 AMDGPUAsmParser::parseString(StringRef &Val, const StringRef ErrMsg) {
   if (isToken(AsmToken::String)) {
@@ -5605,25 +5624,29 @@ bool AMDGPUOperand::isGPRIdxMode() const {
 
 OperandMatchResultTy
 AMDGPUAsmParser::parseSOppBrTarget(OperandVector &Operands) {
-  SMLoc S = Parser.getTok().getLoc();
 
-  switch (getLexer().getKind()) {
-    default: return MatchOperand_ParseFail;
-    case AsmToken::Integer: {
-      int64_t Imm;
-      if (getParser().parseAbsoluteExpression(Imm))
-        return MatchOperand_ParseFail;
-      Operands.push_back(AMDGPUOperand::CreateImm(this, Imm, S));
-      return MatchOperand_Success;
-    }
+  // Make sure we are not parsing something
+  // that looks like a label or an expression but is not.
+  // This will improve error messages.
+  if (isRegister() || isModifier())
+    return MatchOperand_NoMatch;
 
-    case AsmToken::Identifier:
-      Operands.push_back(AMDGPUOperand::CreateExpr(this,
-          MCSymbolRefExpr::create(getContext().getOrCreateSymbol(
-                                  Parser.getTok().getString()), getContext()), S));
-      Parser.Lex();
-      return MatchOperand_Success;
+  if (parseExpr(Operands)) {
+
+    AMDGPUOperand &Opr = ((AMDGPUOperand &)*Operands[Operands.size() - 1]);
+    assert(Opr.isImm() || Opr.isExpr());
+    SMLoc Loc = Opr.getStartLoc();
+
+    // Currently we do not support arbitrary expressions as branch targets.
+    // Only labels and absolute expressions are accepted.
+    if (Opr.isExpr() && !Opr.isSymbolRefExpr()) {
+      Error(Loc, "expected an absolute expression or a label");
+    } else if (Opr.isImm() && !Opr.isS16Imm()) {
+      Error(Loc, "expected a 16-bit signed jump offset");
+    }
   }
+
+  return MatchOperand_Success; // avoid excessive error messages
 }
 
 //===----------------------------------------------------------------------===//

test/MC/AMDGPU/branch-comment.s

@@ -37,6 +37,3 @@ s_branch 32768
 
 s_branch 32767
 // BIN: s_branch 32767 // 000000000024: BF827FFF <keep_symbol+0x20018>
-
-s_branch 0x80000000ffff
-// BIN: s_branch 65535 // 000000000028: BF82FFFF <keep_symbol+0x1c>

test/MC/AMDGPU/sopk.s

@@ -330,3 +330,13 @@ s_call_b64 s[100:101], 12609
 s_call_b64 s[10:11], 49617
 // GFX9:     s_call_b64 s[10:11], 49617 ; encoding: [0xd1,0xc1,0x8a,0xba]
 // NOSICIVI: error: instruction not supported on this GPU
+
+offset = 4
+s_call_b64 s[0:1], offset + 4
+// GFX9:     s_call_b64 s[0:1], 8            ; encoding: [0x08,0x00,0x80,0xba]
+// NOSICIVI: error: instruction not supported on this GPU
+
+offset = 4
+s_call_b64 s[0:1], 4 + offset
+// GFX9:     s_call_b64 s[0:1], 8            ; encoding: [0x08,0x00,0x80,0xba]
+// NOSICIVI: error: instruction not supported on this GPU

test/MC/AMDGPU/sopp-err.s

@@ -157,3 +157,21 @@ s_waitcnt x
 
 s_waitcnt vmcnt(0
 // GCN: error: expected a closing parenthesis
+
+s_branch 0x80000000ffff
+// GCN: error: expected a 16-bit signed jump offset
+
+s_branch 0x10000
+// GCN: error: expected a 16-bit signed jump offset
+
+s_branch -32769
+// GCN: error: expected a 16-bit signed jump offset
+
+s_branch 1.0
+// GCN: error: expected a 16-bit signed jump offset
+
+s_branch s0
+// GCN: error: invalid operand for instruction
+
+s_branch offset:1
+// GCN: error: not a valid operand

test/MC/AMDGPU/sopp.s

@@ -382,3 +382,15 @@ s_endpgm_saved
 s_wakeup
 // VI: s_wakeup ; encoding: [0x00,0x00,0x83,0xbf]
 // NOSICI: error: instruction not supported on this GPU
+
+//===----------------------------------------------------------------------===//
+// absolute expressions as branch offsets
+//===----------------------------------------------------------------------===//
+
+offset = 3
+s_branch 1+offset
+// GCN: s_branch 4 ; encoding: [0x04,0x00,0x82,0xbf]
+
+offset = 3
+s_branch offset+1
+// GCN: s_branch 4 ; encoding: [0x04,0x00,0x82,0xbf]