[AMDGPU][MC] Corrected parsing of branch offsets

[dp]

Summary

Summary of changes:

• Enabled constant expressions as operands of branch instructions, s_call_b64 etc (bug 40820);
• Improved errors reporting (bug 41303).

[artem.tamazov]

LGTM

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

union {
     const MCExpr *Expr;
   };
 
 public:
   bool isToken() const override {
     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 {
     return Kind == Immediate;
   }
 
   bool isInlinableImm(MVT type) const;
   bool isLiteralImm(MVT type) const;
private:
   bool trySkipId(const StringRef Id);
   bool trySkipId(const StringRef Id, const AsmToken::TokenKind Kind);
   bool trySkipToken(const AsmToken::TokenKind Kind);
   bool skipToken(const AsmToken::TokenKind Kind, const StringRef ErrMsg);
   bool parseString(StringRef &Val, const StringRef ErrMsg = "expected a string");
   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;
   SMLoc getLoc() const;
   void lex();
 
 public:
   OperandMatchResultTy parseOptionalOperand(OperandVector &Operands);

 }
 
 bool
 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)) {
     Val = getToken().getStringContents();
     lex();
     return true;
   } else {
     Error(getLoc(), ErrMsg);
     return false;

 }
 
 //===----------------------------------------------------------------------===//
 // sopp branch targets
 //===----------------------------------------------------------------------===//
 
 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
+}
+
 //===----------------------------------------------------------------------===//
 // Boolean holding registers
 //===----------------------------------------------------------------------===//
 
 OperandMatchResultTy
 AMDGPUAsmParser::parseBoolReg(OperandVector &Operands) {
   return parseReg(Operands);
 }

test/MC/AMDGPU/branch-comment.s

 s_branch 65535
 // BIN: s_branch 65535 // 00000000001C: BF82FFFF <keep_symbol+0x10>
 
 s_branch 32768
 // BIN: s_branch 32768 // 000000000020: BF828000 <keep_symbol+0xfffffffffffe0018>
 
 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

 
 s_call_b64 s[100:101], 12609
 // GFX9:     s_call_b64 s[100:101], 12609 ; encoding: [0x41,0x31,0xe4,0xba]
 // NOSICIVI: error: instruction not supported on this GPU
 
 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

 s_waitcnt vmcnt(x)
 // GCN: error: expected absolute expression
 
 s_waitcnt x
 // GCN: error: expected absolute expression
 
 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

 
 s_endpgm_saved
 // VI: s_endpgm_saved ; encoding: [0x00,0x00,0x9b,0xbf]
 // NOSICI: error: instruction not supported on this GPU
 
 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]