[AMDGPU][MC] Refactored parsing of dpp ctrl

[dp]

Summary

Summary of changes:

• simplified code to improve maintainability;
• replaced lex() with higher level parser functions;
• improved errors handling.

[dp]

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

7349

Operands are required for future extensions.

[rampitec]

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

7405

getLoc()? Loc is a single use variable.

[dp]

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

7405

If the expression value is out of bounds, and we call getLoc() later, we will refer an incorrect error position (the next token after the expression).

In the following test

v_mov_b32_dpp v5, v1 quad_perm:[3,2,1,4] row_mask:0x0 bank_mask:0x0

assembler wil point to ']', rather than the offending '4'.

[rampitec]

LGTM

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

public:
   void cvtMIMG(MCInst &Inst, const OperandVector &Operands,
                bool IsAtomic = false);
   void cvtMIMGAtomic(MCInst &Inst, const OperandVector &Operands);
   void cvtIntersectRay(MCInst &Inst, const OperandVector &Operands);
 
   OperandMatchResultTy parseDim(OperandVector &Operands);
   OperandMatchResultTy parseDPP8(OperandVector &Operands);
   OperandMatchResultTy parseDPPCtrl(OperandVector &Operands);
+  bool isSupportedDPPCtrl(StringRef Ctrl, const OperandVector &Operands);
+  int64_t parseDPPCtrlSel(StringRef Ctrl);
+  int64_t parseDPPCtrlPerm();
   AMDGPUOperand::Ptr defaultRowMask() const;
   AMDGPUOperand::Ptr defaultBankMask() const;
   AMDGPUOperand::Ptr defaultBoundCtrl() const;
   AMDGPUOperand::Ptr defaultFI() const;
   void cvtDPP(MCInst &Inst, const OperandVector &Operands, bool IsDPP8 = false);
   void cvtDPP8(MCInst &Inst, const OperandVector &Operands) { cvtDPP(Inst, Operands, true); }
 
   OperandMatchResultTy parseSDWASel(OperandVector &Operands, StringRef Prefix,
if (Res != MatchOperand_Success) {
       if (!Parser.hasPendingError()) {
         // FIXME: use real operand location rather than the current location.
         StringRef Msg =
           (Res == MatchOperand_ParseFail) ? "failed parsing operand." :
                                             "not a valid operand.";
         Error(getLoc(), Msg);
       }
       while (!trySkipToken(AsmToken::EndOfStatement)) {
-        Parser.Lex();
+        lex();
       }
       return true;
     }
   }
 
   return false;
 }
 
if (!trySkipId("dim", AsmToken::Colon))
     return MatchOperand_NoMatch;
 
   // We want to allow "dim:1D" etc., but the initial 1 is tokenized as an
   // integer.
   std::string Token;
   if (isToken(AsmToken::Integer)) {
     SMLoc Loc = getToken().getEndLoc();
     Token = std::string(getTokenStr());
-    Parser.Lex();
+    lex();
     if (getLoc() != Loc)
       return MatchOperand_ParseFail;
   }
   if (!isToken(AsmToken::Identifier))
     return MatchOperand_ParseFail;
   Token += getTokenStr();
 
   StringRef DimId = Token;
   if (DimId.startswith("SQ_RSRC_IMG_"))
     DimId = DimId.substr(12);
 
   const AMDGPU::MIMGDimInfo *DimInfo = AMDGPU::getMIMGDimInfoByAsmSuffix(DimId);
   if (!DimInfo)
     return MatchOperand_ParseFail;
 
-  Parser.Lex();
+  lex();
 
   Operands.push_back(AMDGPUOperand::CreateImm(this, DimInfo->Encoding, S,
                                               AMDGPUOperand::ImmTyDim));
   return MatchOperand_Success;
 }
 
 OperandMatchResultTy AMDGPUAsmParser::parseDPP8(OperandVector &Operands) {
   SMLoc S = getLoc();
OperandMatchResultTy AMDGPUAsmParser::parseDPP8(OperandVector &Operands) {
   unsigned DPP8 = 0;
   for (size_t i = 0; i < 8; ++i)
     DPP8 |= (Sels[i] << (i * 3));
 
   Operands.push_back(AMDGPUOperand::CreateImm(this, DPP8, S, AMDGPUOperand::ImmTyDPP8));
   return MatchOperand_Success;
 }
 
-OperandMatchResultTy
-AMDGPUAsmParser::parseDPPCtrl(OperandVector &Operands) {
-  using namespace AMDGPU::DPP;
+bool
+AMDGPUAsmParser::isSupportedDPPCtrl(StringRef Ctrl,
+                                    const OperandVector &Operands) {
+  if (Ctrl == "row_share" ||
+      Ctrl == "row_xmask")
+    return isGFX10Plus();
 
-  SMLoc S = getLoc();
-  StringRef Prefix;
-  int64_t Int;
+  if (Ctrl == "wave_shl" ||
+      Ctrl == "wave_shr" ||
+      Ctrl == "wave_rol" ||
+      Ctrl == "wave_ror" ||
+      Ctrl == "row_bcast")
+    return isVI() || isGFX9();
+
+  return Ctrl == "row_mirror" ||
+         Ctrl == "row_half_mirror" ||
+         Ctrl == "quad_perm" ||
+         Ctrl == "row_shl" ||
+         Ctrl == "row_shr" ||
+         Ctrl == "row_ror";
+}
 
-  if (isToken(AsmToken::Identifier)) {
-    Prefix = getTokenStr();
-  } else {
-    return MatchOperand_NoMatch;
+int64_t
+AMDGPUAsmParser::parseDPPCtrlPerm() {
+  // quad_perm:[%d,%d,%d,%d]
+
+  if (!skipToken(AsmToken::LBrac, "expected an opening square bracket"))
+    return -1;
+
+  int64_t Val = 0;
+  for (int i = 0; i < 4; ++i) {
+    if (i > 0 && !skipToken(AsmToken::Comma, "expected a comma"))
+      return -1;
+
+    int64_t Temp;
+    SMLoc Loc = getLoc();
+    if (getParser().parseAbsoluteExpression(Temp))
+      return -1;
+    if (Temp < 0 || Temp > 3) {
+      Error(Loc, "expected a 2-bit value");
+      return -1;
     }
 
-  if (Prefix == "row_mirror") {
-    Int = DppCtrl::ROW_MIRROR;
-    Parser.Lex();
-  } else if (Prefix == "row_half_mirror") {
-    Int = DppCtrl::ROW_HALF_MIRROR;
-    Parser.Lex();
-  } else {
-    // Check to prevent parseDPPCtrlOps from eating invalid tokens
-    if (Prefix != "quad_perm"
-        && Prefix != "row_shl"
-        && Prefix != "row_shr"
-        && Prefix != "row_ror"
-        && Prefix != "wave_shl"
-        && Prefix != "wave_rol"
-        && Prefix != "wave_shr"
-        && Prefix != "wave_ror"
-        && Prefix != "row_bcast"
-        && Prefix != "row_share"
-        && Prefix != "row_xmask") {
-      return MatchOperand_NoMatch;
-    }
+    Val += (Temp << i * 2);
 
-    if (!isGFX10Plus() && (Prefix == "row_share" || Prefix == "row_xmask"))
-      return MatchOperand_NoMatch;
+  if (!skipToken(AsmToken::RBrac, "expected a closing square bracket"))
+    return -1;
 
-    if (!isVI() && !isGFX9() &&
-        (Prefix == "wave_shl" || Prefix == "wave_shr" ||
-         Prefix == "wave_rol" || Prefix == "wave_ror" ||
-         Prefix == "row_bcast"))
-      return MatchOperand_NoMatch;
+  return Val;
+}
 
-    Parser.Lex();
-    if (!isToken(AsmToken::Colon))
-      return MatchOperand_ParseFail;
+int64_t
+AMDGPUAsmParser::parseDPPCtrlSel(StringRef Ctrl) {
+  using namespace AMDGPU::DPP;
 
-    if (Prefix == "quad_perm") {
-      // quad_perm:[%d,%d,%d,%d]
-      Parser.Lex();
-      if (!trySkipToken(AsmToken::LBrac))
-        return MatchOperand_ParseFail;
+  // sel:%d
 
-      if (getParser().parseAbsoluteExpression(Int) || !(0 <= Int && Int <=3))
-        return MatchOperand_ParseFail;
+  int64_t Val;
+  SMLoc Loc = getLoc();
 
-      for (int i = 0; i < 3; ++i) {
-        if (!trySkipToken(AsmToken::Comma))
-          return MatchOperand_ParseFail;
+  if (getParser().parseAbsoluteExpression(Val))
+    return -1;
 
-        int64_t Temp;
-        if (getParser().parseAbsoluteExpression(Temp) || !(0 <= Temp && Temp <=3))
-          return MatchOperand_ParseFail;
-        const int shift = i*2 + 2;
-        Int += (Temp << shift);
-      }
+  struct DppCtrlCheck {
+    int64_t Ctrl;
+    int Lo;
+    int Hi;
+  };
 
-      if (!trySkipToken(AsmToken::RBrac))
-        return MatchOperand_ParseFail;
-    } else {
-      // sel:%d
-      Parser.Lex();
-      if (getParser().parseAbsoluteExpression(Int))
-        return MatchOperand_ParseFail;
+  DppCtrlCheck Check = StringSwitch<DppCtrlCheck>(Ctrl)
+    .Case("wave_shl",  {DppCtrl::WAVE_SHL1,       1,  1})
+    .Case("wave_rol",  {DppCtrl::WAVE_ROL1,       1,  1})
+    .Case("wave_shr",  {DppCtrl::WAVE_SHR1,       1,  1})
+    .Case("wave_ror",  {DppCtrl::WAVE_ROR1,       1,  1})
+    .Case("row_shl",   {DppCtrl::ROW_SHL0,        1, 15})
+    .Case("row_shr",   {DppCtrl::ROW_SHR0,        1, 15})
+    .Case("row_ror",   {DppCtrl::ROW_ROR0,        1, 15})
+    .Case("row_share", {DppCtrl::ROW_SHARE_FIRST, 0, 15})
+    .Case("row_xmask", {DppCtrl::ROW_XMASK_FIRST, 0, 15})
+    .Default({-1});
+
+  bool Valid;
+  if (Check.Ctrl == -1) {
+    Valid = (Ctrl == "row_bcast" && (Val == 15 || Val == 31));
+    Val = (Val == 15)? DppCtrl::BCAST15 : DppCtrl::BCAST31;
+    Valid = Check.Lo <= Val && Val <= Check.Hi;
+    Val = (Check.Lo == Check.Hi) ? Check.Ctrl : (Check.Ctrl | Val);
+  }
 
-      if (Prefix == "row_shl" && 1 <= Int && Int <= 15) {
-        Int |= DppCtrl::ROW_SHL0;
-      } else if (Prefix == "row_shr" && 1 <= Int && Int <= 15) {
-        Int |= DppCtrl::ROW_SHR0;
-      } else if (Prefix == "row_ror" && 1 <= Int && Int <= 15) {
-        Int |= DppCtrl::ROW_ROR0;
-      } else if (Prefix == "wave_shl" && 1 == Int) {
-        Int = DppCtrl::WAVE_SHL1;
-      } else if (Prefix == "wave_rol" && 1 == Int) {
-        Int = DppCtrl::WAVE_ROL1;
-      } else if (Prefix == "wave_shr" && 1 == Int) {
-        Int = DppCtrl::WAVE_SHR1;
-      } else if (Prefix == "wave_ror" && 1 == Int) {
-        Int = DppCtrl::WAVE_ROR1;
-      } else if (Prefix == "row_bcast") {
-        if (Int == 15) {
-          Int = DppCtrl::BCAST15;
-        } else if (Int == 31) {
-          Int = DppCtrl::BCAST31;
-        } else {
-          return MatchOperand_ParseFail;
-        }
-      } else if (Prefix == "row_share" && 0 <= Int && Int <= 15) {
-        Int |= DppCtrl::ROW_SHARE_FIRST;
-      } else if (Prefix == "row_xmask" && 0 <= Int && Int <= 15) {
-        Int |= DppCtrl::ROW_XMASK_FIRST;
-      } else {
-        return MatchOperand_ParseFail;
+  if (!Valid) {
+    Error(Loc, Twine("invalid ", Ctrl) + Twine(" value"));
+    return -1;
+  }
+
+  return Val;
+
+OperandMatchResultTy
+AMDGPUAsmParser::parseDPPCtrl(OperandVector &Operands) {
+  using namespace AMDGPU::DPP;
+
+  if (!isToken(AsmToken::Identifier) ||
+      !isSupportedDPPCtrl(getTokenStr(), Operands))
+    return MatchOperand_NoMatch;
+
+  SMLoc S = getLoc();
+  int64_t Val = -1;
+  StringRef Ctrl;
+
+  parseId(Ctrl);
+
+  if (Ctrl == "row_mirror") {
+    Val = DppCtrl::ROW_MIRROR;
+  } else if (Ctrl == "row_half_mirror") {
+    Val = DppCtrl::ROW_HALF_MIRROR;
+    if (skipToken(AsmToken::Colon, "expected a colon")) {
+      if (Ctrl == "quad_perm") {
+        Val = parseDPPCtrlPerm();
+      } else {
+        Val = parseDPPCtrlSel(Ctrl);
       }
     }
   }
 
-  Operands.push_back(AMDGPUOperand::CreateImm(this, Int, S, AMDGPUOperand::ImmTyDppCtrl));
+  if (Val == -1)
+    return MatchOperand_ParseFail;
+
+  Operands.push_back(
+    AMDGPUOperand::CreateImm(this, Val, S, AMDGPUOperand::ImmTyDppCtrl));
   return MatchOperand_Success;
 }
 
 AMDGPUOperand::Ptr AMDGPUAsmParser::defaultRowMask() const {
   return AMDGPUOperand::CreateImm(this, 0xf, SMLoc(), AMDGPUOperand::ImmTyDppRowMask);
 }
 
 AMDGPUOperand::Ptr AMDGPUAsmParser::defaultEndpgmImmOperands() const {

llvm/test/MC/AMDGPU/gfx10_err_pos.s

 // CHECK-NEXT:{{^}}                         ^
 
 s_atomic_swap s5, s[2:3], 0x1FFFFF
 // CHECK: error: expected a 21-bit signed offset
 // CHECK-NEXT:{{^}}s_atomic_swap s5, s[2:3], 0x1FFFFF
 // CHECK-NEXT:{{^}}                          ^
 
 //==============================================================================
+// expected a 2-bit value
+
+v_mov_b32_dpp v5, v1 quad_perm:[3,2,1,4] row_mask:0x0 bank_mask:0x0
+// CHECK: error: expected a 2-bit value
+// CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 quad_perm:[3,2,1,4] row_mask:0x0 bank_mask:0x0
+// CHECK-NEXT:{{^}}                                      ^
+
+v_mov_b32_dpp v5, v1 quad_perm:[3,-1,1,3] row_mask:0x0 bank_mask:0x0
+// CHECK: error: expected a 2-bit value
+// CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 quad_perm:[3,-1,1,3] row_mask:0x0 bank_mask:0x0
+// CHECK-NEXT:{{^}}                                  ^
+
+//==============================================================================
 // expected a 3-bit value
 
 v_mov_b32_dpp v5, v1 dpp8:[-1,1,2,3,4,5,6,7]
 // CHECK: error: expected a 3-bit value
 // CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 dpp8:[-1,1,2,3,4,5,6,7]
 // CHECK-NEXT:{{^}}                           ^
 
 v_mov_b32_dpp v5, v1 dpp8:[0,1,2,8,4,5,6,7]

 // CHECK-NEXT:{{^}}v_pk_add_u16 v1, v2, v3 op_sel:[0,0,0,0,0]
 // CHECK-NEXT:{{^}}                                       ^
 
 v_mov_b32_dpp v5, v1 dpp8:[0,1,2,3,4,5,6,7)
 // CHECK: error: expected a closing square bracket
 // CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 dpp8:[0,1,2,3,4,5,6,7)
 // CHECK-NEXT:{{^}}                                          ^
 
+v_mov_b32_dpp v5, v1 quad_perm:[3,2,1,0) row_mask:0x0 bank_mask:0x0
+// CHECK: error: expected a closing square bracket
+// CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 quad_perm:[3,2,1,0) row_mask:0x0 bank_mask:0x0
+// CHECK-NEXT:{{^}}                                       ^
+
 //==============================================================================
 // expected a colon
 
 ds_swizzle_b32 v8, v2 offset
 // CHECK: error: expected a colon
 // CHECK-NEXT:{{^}}ds_swizzle_b32 v8, v2 offset
 // CHECK-NEXT:{{^}}                            ^
 

 // CHECK-NEXT:{{^}}v_pk_add_u16 v1, v2, v3 op_sel:[0 0]
 // CHECK-NEXT:{{^}}                                  ^
 
 v_mov_b32_dpp v5, v1 dpp8:[0,1,2,3,4,5,6]
 // CHECK: error: expected a comma
 // CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 dpp8:[0,1,2,3,4,5,6]
 // CHECK-NEXT:{{^}}                                        ^
 
+v_mov_b32_dpp v5, v1 quad_perm:[3,2] row_mask:0x0 bank_mask:0x0
+// CHECK: error: expected a comma
+// CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 quad_perm:[3,2] row_mask:0x0 bank_mask:0x0
+// CHECK-NEXT:{{^}}                                   ^
+
 //==============================================================================
 // expected a comma or a closing parenthesis
 
 s_setreg_b32  hwreg(1 2,3), s2
 // CHECK: error: expected a comma or a closing parenthesis
 // CHECK-NEXT:{{^}}s_setreg_b32  hwreg(1 2,3), s2
 // CHECK-NEXT:{{^}}                      ^
 

 // CHECK-NEXT:{{^}}tbuffer_store_format_xyzw v[1:4], off, ttmp[4:7], s0 format:BUF_NUM_FORMAT_UINT]
 // CHECK-NEXT:{{^}}                                                            ^
 
 v_mov_b32_dpp v5, v1 dpp8:[0,1,2,x,4,5,6,7]
 // CHECK: error: expected absolute expression
 // CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 dpp8:[0,1,2,x,4,5,6,7]
 // CHECK-NEXT:{{^}}                                 ^
 
+v_mov_b32_dpp v5, v1 quad_perm:[3,x,1,0] row_mask:0x0 bank_mask:0x0
+// CHECK: error: expected absolute expression
+// CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 quad_perm:[3,x,1,0] row_mask:0x0 bank_mask:0x0
+// CHECK-NEXT:{{^}}                                  ^
+
+v_mov_b32_dpp v5, v1 row_share:x row_mask:0x0 bank_mask:0x0
+// CHECK: error: expected absolute expression
+// CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 row_share:x row_mask:0x0 bank_mask:0x0
+// CHECK-NEXT:{{^}}                               ^
+
 //==============================================================================
 // expected a message name or an absolute expression
 
 s_sendmsg sendmsg(MSG_GSX, GS_OP_CUT, 0)
 // CHECK: error: expected a message name or an absolute expression
 // CHECK-NEXT:{{^}}s_sendmsg sendmsg(MSG_GSX, GS_OP_CUT, 0)
 // CHECK-NEXT:{{^}}                  ^
 

 //==============================================================================
 // expected an opening square bracket
 
 v_mov_b32_dpp v5, v1 dpp8:(0,1,2,3,4,5,6,7)
 // CHECK: error: expected an opening square bracket
 // CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 dpp8:(0,1,2,3,4,5,6,7)
 // CHECK-NEXT:{{^}}                          ^
 
+v_mov_b32_dpp v5, v1 quad_perm:(3,2,1,0) row_mask:0x0 bank_mask:0x0
+// CHECK: error: expected an opening square bracket
+// CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 quad_perm:(3,2,1,0) row_mask:0x0 bank_mask:0x0
+// CHECK-NEXT:{{^}}                               ^
+
 //==============================================================================
 // expected an operation name or an absolute expression
 
 s_sendmsg sendmsg(MSG_GS, GS_OP_CUTX, 0)
 // CHECK: error: expected an operation name or an absolute expression
 // CHECK-NEXT:{{^}}s_sendmsg sendmsg(MSG_GS, GS_OP_CUTX, 0)
 // CHECK-NEXT:{{^}}                          ^
 

 // invalid register name
 
 s_mov_b64 s[10:11], [x0,s1]
 // CHECK: error: invalid register name
 // CHECK-NEXT:{{^}}s_mov_b64 s[10:11], [x0,s1]
 // CHECK-NEXT:{{^}}                     ^
 
 //==============================================================================
+// invalid row_share value
+
+v_mov_b32_dpp v5, v1 row_share:16 row_mask:0x0 bank_mask:0x0
+// CHECK: error: invalid row_share value
+// CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 row_share:16 row_mask:0x0 bank_mask:0x0
+// CHECK-NEXT:{{^}}                               ^
+
+v_mov_b32_dpp v5, v1 row_share:-1 row_mask:0x0 bank_mask:0x0
+// CHECK: error: invalid row_share value
+// CHECK-NEXT:{{^}}v_mov_b32_dpp v5, v1 row_share:-1 row_mask:0x0 bank_mask:0x0
+// CHECK-NEXT:{{^}}                               ^
+
+//==============================================================================
 // invalid syntax, expected 'neg' modifier
 
 v_ceil_f32 v0, --1
 // CHECK: error: invalid syntax, expected 'neg' modifier
 // CHECK-NEXT:{{^}}v_ceil_f32 v0, --1
 // CHECK-NEXT:{{^}}               ^
 
 //==============================================================================

llvm/test/MC/AMDGPU/regression/bug28538.s

 // RUN: not llvm-mc -arch=amdgcn -mcpu=tonga %s 2>&1 | FileCheck %s --check-prefix=NOVI --implicit-check-not=error:
 // RUN: not llvm-mc -arch=amdgcn %s 2>&1 | FileCheck %s --check-prefix=NOSICI --implicit-check-not=error:
 // RUN: not llvm-mc -arch=amdgcn -mcpu=tahiti %s 2>&1 | FileCheck %s --check-prefix=NOSICI --implicit-check-not=error:
 // RUN: not llvm-mc -arch=amdgcn -mcpu=bonaire %s 2>&1 | FileCheck %s --check-prefix=NOSICI --implicit-check-not=error:
 
 // NOSICI: error: not a valid operand.
-// NOVI: error: failed parsing operand
+// NOVI: error: invalid row_bcast value
 v_mov_b32 v0, v0 row_bcast:0
 
 // NOSICI: error: not a valid operand.
-// NOVI: error: failed parsing operand
+// NOVI: error: invalid row_bcast value
 v_mov_b32 v0, v0 row_bcast:13