[MC] Support .dcb directives in assembler parser

[phosek]

Summary

These directives are already supported by GNU assembler.

[davide]

Same comments apply here, can you group switch conditions together (for consistency) and add some tests for the other error conditions?

[phosek]

Is there anything else you want me to fix?

[davide]

Sorry, lld kept me a little busy in the last two days =) I'll take a look at this again by the end of today. Sorry for the delay.

[davide]

Feel free to submit after fixing my second round of comments, no need for another back and forth. Thanks!

lib/MC/MCParser/AsmParser.cpp

275–276 ↗

(On Diff #72088)

This is not your fault (partly, but a large amount of the parse* methods don't have a doxygen comment).
Do you mind to add one to this one?

1919–1921 ↗

(On Diff #72088)

This and the condition at line 1933 can be merged together, no?

[phosek]

lib/MC/MCParser/AsmParser.cpp

1919–1921 ↗

(On Diff #72088)

That's true, it's a different directive but that's probably not an issue here.

llvm/trunk/lib/MC/MCParser/AsmParser.cpp

public:
   bool parseExpression(const MCExpr *&Res);
   bool parseExpression(const MCExpr *&Res, SMLoc &EndLoc) override;
   bool parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) override;
   bool parseParenExpression(const MCExpr *&Res, SMLoc &EndLoc) override;
   bool parseParenExprOfDepth(unsigned ParenDepth, const MCExpr *&Res,
                              SMLoc &EndLoc) override;
   bool parseAbsoluteExpression(int64_t &Res) override;
 
+  /// \brief Parse a floating point expression using the float \p Semantics
+  /// and set \p Res to the value.
+  bool parseRealValue(const fltSemantics &Semantics, APInt &Res);
+
   /// \brief Parse an identifier or string (as a quoted identifier)
   /// and set \p Res to the identifier contents.
   bool parseIdentifier(StringRef &Res) override;
   void eatToEndOfStatement() override;
 
   void checkForValidSection() override;
 
   /// }
private:
 
   // Generic (target and platform independent) directive parsing.
   enum DirectiveKind {
     DK_NO_DIRECTIVE, // Placeholder
     DK_SET, DK_EQU, DK_EQUIV, DK_ASCII, DK_ASCIZ, DK_STRING, DK_BYTE, DK_SHORT,
     DK_RELOC,
     DK_VALUE, DK_2BYTE, DK_LONG, DK_INT, DK_4BYTE, DK_QUAD, DK_8BYTE, DK_OCTA,
     DK_DC, DK_DC_A, DK_DC_B, DK_DC_D, DK_DC_L, DK_DC_S, DK_DC_W, DK_DC_X,
+    DK_DCB, DK_DCB_B, DK_DCB_D, DK_DCB_L, DK_DCB_S, DK_DCB_W, DK_DCB_X,
     DK_SINGLE, DK_FLOAT, DK_DOUBLE, DK_ALIGN, DK_ALIGN32, DK_BALIGN, DK_BALIGNW,
     DK_BALIGNL, DK_P2ALIGN, DK_P2ALIGNW, DK_P2ALIGNL, DK_ORG, DK_FILL, DK_ENDR,
     DK_BUNDLE_ALIGN_MODE, DK_BUNDLE_LOCK, DK_BUNDLE_UNLOCK,
     DK_ZERO, DK_EXTERN, DK_GLOBL, DK_GLOBAL,
     DK_LAZY_REFERENCE, DK_NO_DEAD_STRIP, DK_SYMBOL_RESOLVER,
     DK_PRIVATE_EXTERN, DK_REFERENCE, DK_WEAK_DEFINITION, DK_WEAK_REFERENCE,
     DK_WEAK_DEF_CAN_BE_HIDDEN, DK_COMM, DK_COMMON, DK_LCOMM, DK_ABORT,
     DK_INCLUDE, DK_INCBIN, DK_CODE16, DK_CODE16GCC, DK_REPT, DK_IRP, DK_IRPC,
private:
   // ".bundle_lock"
   bool parseDirectiveBundleLock();
   // ".bundle_unlock"
   bool parseDirectiveBundleUnlock();
 
   // ".space", ".skip"
   bool parseDirectiveSpace(StringRef IDVal);
 
+  // ".dcb"
+  bool parseDirectiveDCB(StringRef IDVal, unsigned Size);
+  bool parseDirectiveRealDCB(StringRef IDVal, const fltSemantics &);
+
   // .sleb128 (Signed=true) and .uleb128 (Signed=false)
   bool parseDirectiveLEB128(bool Signed);
 
   /// \brief Parse a directive like ".globl" which
   /// accepts a single symbol (which should be a label or an external).
   bool parseDirectiveSymbolAttribute(MCSymbolAttr Attr);
 
   bool parseDirectiveComm(bool IsLocal); // ".comm" and ".lcomm"
if (IDVal[0] == '.' && IDVal != ".") {
     case DK_DC_D:
       return parseDirectiveRealValue(APFloat::IEEEdouble);
     case DK_DC_L:
       return parseDirectiveValue(4);
     case DK_DC_S:
       return parseDirectiveRealValue(APFloat::IEEEsingle);
     case DK_DC_W:
       return parseDirectiveValue(2);
+    case DK_DCB:
+    case DK_DCB_W:
+      return parseDirectiveDCB(IDVal, 2);
+    case DK_DCB_B:
+      return parseDirectiveDCB(IDVal, 1);
+    case DK_DCB_D:
+      return parseDirectiveRealDCB(IDVal, APFloat::IEEEdouble);
+    case DK_DCB_L:
+      return parseDirectiveDCB(IDVal, 4);
+    case DK_DCB_S:
+      return parseDirectiveRealDCB(IDVal, APFloat::IEEEsingle);
     case DK_DC_X:
+    case DK_DCB_X:
       return TokError(Twine(IDVal) +
                       " not currently supported for this target");
     }
 
     return Error(IDLoc, "unknown directive");
   }
 
   // __asm _emit or __asm __emit
while (true) {
         return true;
     }
   }
 
   Lex();
   return false;
 }
 
-/// parseDirectiveRealValue
-///  ::= (.single | .double) [ expression (, expression)* ]
-bool AsmParser::parseDirectiveRealValue(const fltSemantics &Semantics) {
-  if (getLexer().isNot(AsmToken::EndOfStatement)) {
-    checkForValidSection();
-
-    while (true) {
-      // We don't truly support arithmetic on floating point expressions, so we
-      // have to manually parse unary prefixes.
-      bool IsNeg = false;
-      if (getLexer().is(AsmToken::Minus)) {
-        Lexer.Lex();
-        IsNeg = true;
-      } else if (getLexer().is(AsmToken::Plus))
-        Lexer.Lex();
+bool AsmParser::parseRealValue(const fltSemantics &Semantics, APInt &Res) {
 
-      if (Lexer.is(AsmToken::Error))
-        return TokError(Lexer.getErr());
-      if (Lexer.isNot(AsmToken::Integer) && Lexer.isNot(AsmToken::Real) &&
-          Lexer.isNot(AsmToken::Identifier))
-        return TokError("unexpected token in directive");
 
-      // Convert to an APFloat.
-      APFloat Value(Semantics);
-      StringRef IDVal = getTok().getString();
-      if (getLexer().is(AsmToken::Identifier)) {
-        if (!IDVal.compare_lower("infinity") || !IDVal.compare_lower("inf"))
-          Value = APFloat::getInf(Semantics);
-        else if (!IDVal.compare_lower("nan"))
-          Value = APFloat::getNaN(Semantics, false, ~0);
-        else
-          return TokError("invalid floating point literal");
-      } else if (Value.convertFromString(IDVal, APFloat::rmNearestTiesToEven) ==
-                 APFloat::opInvalidOp)
-        return TokError("invalid floating point literal");
-      if (IsNeg)
-        Value.changeSign();
 
-      // Consume the numeric token.
-      Lex();
 
-      // Emit the value as an integer.
-      APInt AsInt = Value.bitcastToAPInt();
+  Res = Value.bitcastToAPInt();
+
+  return false;
+}
+
+/// parseDirectiveRealValue
+///  ::= (.single | .double) [ expression (, expression)* ]
+bool AsmParser::parseDirectiveRealValue(const fltSemantics &Semantics) {
+  if (getLexer().isNot(AsmToken::EndOfStatement)) {
+    checkForValidSection();
+
+    while (true) {
+      APInt AsInt;
+      if (parseRealValue(Semantics, AsInt))
+        return true;
+
       getStreamer().EmitIntValue(AsInt.getLimitedValue(),
                                  AsInt.getBitWidth() / 8);
 
       if (Lexer.is(AsmToken::EndOfStatement))
         break;
 
       if (parseToken(AsmToken::Comma, "unexpected token in directive"))
         return true;
if (parseToken(AsmToken::EndOfStatement,
     return true;
 
   // FIXME: Sometimes the fill expr is 'nop' if it isn't supplied, instead of 0.
   getStreamer().emitFill(*NumBytes, FillExpr, NumBytesLoc);
 
   return false;
 }
 
+/// parseDirectiveDCB
+/// ::= .dcb.{b, l, w} expression, expression
+bool AsmParser::parseDirectiveDCB(StringRef IDVal, unsigned Size) {
+  checkForValidSection();
+
+  SMLoc NumValuesLoc = Lexer.getLoc();
+  int64_t NumValues;
+  if (parseAbsoluteExpression(NumValues))
+    return true;
+
+  if (NumValues < 0) {
+    Warning(NumValuesLoc, "'" + Twine(IDVal) + "' directive with negative repeat count has no effect");
+    return false;
+  }
+
+  if (parseToken(AsmToken::Comma,
+                 "unexpected token in '" + Twine(IDVal) + "' directive"))
+    return true;
+
+  const MCExpr *Value;
+  SMLoc ExprLoc = getLexer().getLoc();
+  if (parseExpression(Value))
+    return true;
+
+  // Special case constant expressions to match code generator.
+  if (const MCConstantExpr *MCE = dyn_cast<MCConstantExpr>(Value)) {
+    assert(Size <= 8 && "Invalid size");
+    uint64_t IntValue = MCE->getValue();
+    if (!isUIntN(8 * Size, IntValue) && !isIntN(8 * Size, IntValue))
+      return Error(ExprLoc, "literal value out of range for directive");
+    for (uint64_t i = 0, e = NumValues; i != e; ++i)
+      getStreamer().EmitIntValue(IntValue, Size);
+  } else {
+    for (uint64_t i = 0, e = NumValues; i != e; ++i)
+      getStreamer().EmitValue(Value, Size, ExprLoc);
+  }
+
+  if (parseToken(AsmToken::EndOfStatement,
+                 "unexpected token in '" + Twine(IDVal) + "' directive"))
+    return true;
+
+  return false;
+}
+
+/// parseDirectiveRealDCB
+/// ::= .dcb.{d, s} expression, expression
+bool AsmParser::parseDirectiveRealDCB(StringRef IDVal, const fltSemantics &Semantics) {
+  checkForValidSection();
+
+  SMLoc NumValuesLoc = Lexer.getLoc();
+  int64_t NumValues;
+  if (parseAbsoluteExpression(NumValues))
+    return true;
+
+  if (NumValues < 0) {
+    Warning(NumValuesLoc, "'" + Twine(IDVal) + "' directive with negative repeat count has no effect");
+    return false;
+  }
+
+  if (parseToken(AsmToken::Comma,
+                 "unexpected token in '" + Twine(IDVal) + "' directive"))
+    return true;
+
+  APInt AsInt;
+  if (parseRealValue(Semantics, AsInt))
+    return true;
+
+  if (parseToken(AsmToken::EndOfStatement,
+                 "unexpected token in '" + Twine(IDVal) + "' directive"))
+    return true;
+
+  for (uint64_t i = 0, e = NumValues; i != e; ++i)
+    getStreamer().EmitIntValue(AsInt.getLimitedValue(),
+                               AsInt.getBitWidth() / 8);
+
+  return false;
+}
+
 /// parseDirectiveLEB128
 /// ::= (.sleb128 | .uleb128) [ expression (, expression)* ]
 bool AsmParser::parseDirectiveLEB128(bool Signed) {
   checkForValidSection();
   const MCExpr *Value;
 
   while (true) {
     if (parseExpression(Value))
void AsmParser::initializeDirectiveKindMap() {
   DirectiveKindMap[".dc"] = DK_DC;
   DirectiveKindMap[".dc.a"] = DK_DC_A;
   DirectiveKindMap[".dc.b"] = DK_DC_B;
   DirectiveKindMap[".dc.d"] = DK_DC_D;
   DirectiveKindMap[".dc.l"] = DK_DC_L;
   DirectiveKindMap[".dc.s"] = DK_DC_S;
   DirectiveKindMap[".dc.w"] = DK_DC_W;
   DirectiveKindMap[".dc.x"] = DK_DC_X;
+  DirectiveKindMap[".dcb"] = DK_DCB;
+  DirectiveKindMap[".dcb.b"] = DK_DCB_B;
+  DirectiveKindMap[".dcb.d"] = DK_DCB_D;
+  DirectiveKindMap[".dcb.l"] = DK_DCB_L;
+  DirectiveKindMap[".dcb.s"] = DK_DCB_S;
+  DirectiveKindMap[".dcb.w"] = DK_DCB_W;
+  DirectiveKindMap[".dcb.x"] = DK_DCB_X;
 }
 
 MCAsmMacro *AsmParser::parseMacroLikeBody(SMLoc DirectiveLoc) {
   AsmToken EndToken, StartToken = getTok();
 
   unsigned NestLevel = 0;
   while (true) {
     // Check whether we have reached the end of the file.

llvm/trunk/test/MC/AsmParser/directive_dcb.s

+# RUN: not llvm-mc -triple i386-unknown-unknown %s | FileCheck %s
+# RUN: not llvm-mc -triple i386-unknown-unknown %s 2>&1 > /dev/null| FileCheck %s --check-prefix=CHECK-ERROR
+
+# CHECK: TEST0:
+# CHECK: .byte 1
+# CHECK: .byte 1
+TEST0:
+        .dcb.b 2, 1
+
+# CHECK: TEST1:
+# CHECK: .short 3
+TEST1:
+        .dcb 1, 3
+
+# CHECK: TEST2:
+# CHECK: .short 3
+# CHECK: .short 3
+TEST2:
+        .dcb.w 2, 3
+
+# CHECK: TEST3:
+# CHECK: .long 8
+# CHECK: .long 8
+# CHECK: .long 8
+TEST3:
+        .dcb.l 3, 8
+
+# CHECK: TEST5
+# CHECK: .long  1067412619
+# CHECK: .long  1067412619
+# CHECK: .long  1067412619
+# CHECK: .long  1067412619
+TEST5:
+        .dcb.s 4, 1.2455
+
+# CHECK: TEST6
+# CHECK: .quad  4597526701198935065
+# CHECK: .quad  4597526701198935065
+# CHECK: .quad  4597526701198935065
+# CHECK: .quad  4597526701198935065
+# CHECK: .quad  4597526701198935065
+TEST6:
+        .dcb.d 5, .232
+
+# CHECK-ERROR: error: .dcb.x not currently supported for this target
+TEST7:
+        .dcb.x 3, 1.2e3
+
+# CHECK-ERROR: warning: '.dcb' directive with negative repeat count has no effect
+TEST8:
+       .dcb -1, 2
+
+# CHECK-ERROR: error: unexpected token in '.dcb' directive
+TEST9:
+       .dcb 1 2
+
+# CHECK-ERROR: error: unexpected token in '.dcb' directive
+TEST10:
+       .dcb 1, 2 3