[M68k] Add support for Motorola literal syntax to AsmParser

[ricky26]

Summary

This adds support for parsing Motorola-style integers, which are used by default by the existing compilers for the M68k.

These look like $00A0cf for hex and %001010101 for binary. They are used in Motorola assembly syntax.

There's a possible question about how the binary literals will interact with inline assembly in clang.

[ricky26]

Run clang-format

[anirudhp]

llvm/include/llvm/MC/MCAsmInfo.h

453

My apologies for intruding into a draft patch :)

Firstly, I don't believe there is anything wrong with introducing a field in MCAsmInfo.h and setting it in the appropriate target inherited version of it.

However, there seems to be previous precedent within MCAsmLexer.h to support a different class of Integers (LexMasmIntegers). So maybe the field could go directly into MCAsmLexer and could be set in AsmLexer/AsmParser as appropriate?

[ricky26]

llvm/include/llvm/MC/MCAsmInfo.h

453

Hi, there's no problem there. I mostly uploaded this early since it's a dependency for one of my other patches. Feedback is always welcome. :)

I think I might've seen LexMasmIntegers when I was initially implementing this but having just followed it through the code again, I can't see how to make it the default (for example, I'd expect clang -cc1as/llvm-mc/etc to use Motorola-style integers by default for M68k but not anything else).

I'm lament to add specific logic to determine that default in all of the front-ends but it looks like that's what is done with LexMasmIntegers.

I'll open this up for review proper now anyway.

[epastor]

Can you add tests for this?

llvm/lib/MC/MCParser/AsmLexer.cpp

426

0-9a-fA-F? (Currently reads 0-0)

811

I think this only recognizes hex numbers that start with 0-9. Is $ff a valid integer literal in this context? If so, this should maybe use isHexDigit.

[ricky26]

Apply review feedback, add some tests and a flag for llvm-mc.

[anirudhp]

llvm/lib/MC/MCParser/AsmLexer.cpp

36

If you're setting this in the constructor, I'm hoping you're aware that for any other invocations of the AsmParser instance, except from llvm-mc, the lexing of motorola integers will not work as "expected", until the UseMotorolaIntegers attribute is explicitly set in the Motorola's inherited version of MCAsmInfo

If the only intended invocation is from llvm-mc for now, then maybe it makes sense to remove the MCAsmInfo attribute completely, since you already declared LexMotorolaIntegers as LexMotorolaIntegers = false in MCAsmLexer.h.

[ricky26]

llvm/lib/MC/MCParser/AsmLexer.cpp

36

That's about the size of it. I kept it this way so that I can happily build tests for this piece of functionality (which affects more than just the experimental platform) without building for M68k. Then, when the M68k AsmParser implementation lands, it'll include the change to M68kMCAsmInfo.

I wanted to keep the MCAsmInfo change here to avoid bundling changes into the already quite large bootstrapping of the M68k assembler.

It feels a bit off to populate this in the constructor as I've done, so I'm very much open to alternatives.

[anirudhp]

llvm/lib/MC/MCParser/AsmLexer.cpp

36

I wanted to keep the MCAsmInfo change here to avoid bundling changes into the already quite large bootstrapping of the M68k assembler.

Hmm I see. I guess the only other suggestion I have in this case, is to have a small patch that just deals with setting the MCAsminfo changes, and the setting of the variable in the MCAsmLexer constructor, and have your big asm parser patch depend on that.

But at the same time that might be just increasing your work unnecessarily :), and if the particular attribute is going to be set soon in a forthcoming patch, then it should be fine to have your changes in this patch the way they are currently.

[ricky26]

Pulled the use of UseMotorolaIntegers back into this patch.

It ocurred to me that it can't actually have any negative effects since there's
no means in tree to instantiate an AsmParser for the M68k yet.

[myhsu]

llvm/lib/MC/MCParser/AsmLexer.cpp

36

I ca definitely foresee a more modular changes if we split the MCAsmInfo attribute and the populating LexMotorolaIntegers in constructor into a separate patch. But to be honest I feel like this is a really minor software engineering issue.
It will only concern me if reverting any of these patches causes large scale disaster. But either we split this patch or not, M68k will be the only one get affected. So I'm fine with the current arrangement.

llvm/include/llvm/MC/MCAsmInfo.h

protected:
   // If true, emit GOTPCRELX/REX_GOTPCRELX instead of GOTPCREL, on
   // X86_64 ELF.
   bool RelaxELFRelocations = true;
 
   // If true, then the lexer and expression parser will support %neg(),
   // %hi(), and similar unary operators.
   bool HasMipsExpressions = false;
 
+  // If true, use Motorola-style integers in Assembly (ex. $0ac).
+  bool UseMotorolaIntegers = false;
+
   // If true, emit function descriptor symbol on AIX.
   bool NeedsFunctionDescriptors = false;
 
 public:
   explicit MCAsmInfo();
   virtual ~MCAsmInfo();
 
   /// Get the code pointer size in bytes.
public:
   }
 
   bool shouldUseLogicalShr() const { return UseLogicalShr; }
 
   bool canRelaxRelocations() const { return RelaxELFRelocations; }
   void setRelaxELFRelocations(bool V) { RelaxELFRelocations = V; }
   bool hasMipsExpressions() const { return HasMipsExpressions; }
   bool needsFunctionDescriptors() const { return NeedsFunctionDescriptors; }
+  bool shouldUseMotorolaIntegers() const { return UseMotorolaIntegers; }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCASMINFO_H

llvm/include/llvm/MC/MCParser/MCAsmLexer.h

protected: // Can only create subclasses.
   const char *TokStart = nullptr;
   bool SkipSpace = true;
   bool AllowAtInIdentifier;
   bool AllowHashInIdentifier = false;
   bool IsAtStartOfStatement = true;
   bool LexMasmHexFloats = false;
   bool LexMasmIntegers = false;
   bool LexMasmStrings = false;
+  bool LexMotorolaIntegers = false;
   bool UseMasmDefaultRadix = false;
   unsigned DefaultRadix = 10;
   AsmCommentConsumer *CommentConsumer = nullptr;
 
   MCAsmLexer();
 
   virtual AsmToken LexToken() = 0;
 
public:
   void setMasmDefaultRadix(unsigned Radix) { DefaultRadix = Radix; }
 
   /// Set whether to lex masm-style hex float literals, such as 3f800000r.
   void setLexMasmHexFloats(bool V) { LexMasmHexFloats = V; }
 
   /// Set whether to lex masm-style string literals, such as 'Can''t find file'
   /// and "This ""value"" not found".
   void setLexMasmStrings(bool V) { LexMasmStrings = V; }
+
+  /// Set whether to lex Motorola-style integer literals, such as $deadbeef or
+  /// %01010110.
+  void setLexMotorolaIntegers(bool V) { LexMotorolaIntegers = V; }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCPARSER_MCASMLEXER_H

llvm/lib/MC/MCParser/AsmLexer.cpp

 #include <string>
 #include <tuple>
 #include <utility>
 
 using namespace llvm;
 
 AsmLexer::AsmLexer(const MCAsmInfo &MAI) : MAI(MAI) {
   AllowAtInIdentifier = !StringRef(MAI.getCommentString()).startswith("@");
+  LexMotorolaIntegers = MAI.shouldUseMotorolaIntegers();
 }
 
 AsmLexer::~AsmLexer() = default;
 
 void AsmLexer::setBuffer(StringRef Buf, const char *ptr,
                          bool EndStatementAtEOF) {
   CurBuf = Buf;
 
if (LexMasmIntegers && UseMasmDefaultRadix) {
     if (Result.getAsInteger(DefaultRadix, Value)) {
       return ReturnError(TokStart,
                          "invalid " + radixName(DefaultRadix) + " number");
     }
 
     return intToken(Result, Value);
   }
 
+  // Motorola hex integers: $[0-9a-fA-F]+
+  if (LexMotorolaIntegers && CurPtr[-1] == '$') {
+    const char *NumStart = CurPtr;
+    while (isHexDigit(CurPtr[0]))
+      ++CurPtr;
+
+    APInt Result(128, 0);
+    if (StringRef(NumStart, CurPtr - NumStart).getAsInteger(16, Result))
+      return ReturnError(TokStart, "invalid hexadecimal number");
+
+    return intToken(StringRef(TokStart, CurPtr - TokStart), Result);
+  }
+
+  // Motorola binary integers: %[01]+
+  if (LexMotorolaIntegers && CurPtr[-1] == '%') {
+    const char *NumStart = CurPtr;
+    while (*CurPtr == '0' || *CurPtr == '1')
+      ++CurPtr;
+
+    APInt Result(128, 0);
+    if (StringRef(NumStart, CurPtr - NumStart).getAsInteger(2, Result))
+      return ReturnError(TokStart, "invalid binary number");
+
+    return intToken(StringRef(TokStart, CurPtr - TokStart), Result);
+  }
+
   // Decimal integer: [1-9][0-9]*
   if (CurPtr[-1] != '0' || CurPtr[0] == '.') {
     unsigned Radix = doHexLookAhead(CurPtr, 10, LexMasmIntegers);
     bool isHex = Radix == 16;
     // Check for floating point literals.
     if (!isHex && (*CurPtr == '.' || *CurPtr == 'e' || *CurPtr == 'E')) {
       if (*CurPtr == '.')
         ++CurPtr;
AsmToken AsmLexer::LexToken() {
   case '(': return AsmToken(AsmToken::LParen, StringRef(TokStart, 1));
   case ')': return AsmToken(AsmToken::RParen, StringRef(TokStart, 1));
   case '[': return AsmToken(AsmToken::LBrac, StringRef(TokStart, 1));
   case ']': return AsmToken(AsmToken::RBrac, StringRef(TokStart, 1));
   case '{': return AsmToken(AsmToken::LCurly, StringRef(TokStart, 1));
   case '}': return AsmToken(AsmToken::RCurly, StringRef(TokStart, 1));
   case '*': return AsmToken(AsmToken::Star, StringRef(TokStart, 1));
   case ',': return AsmToken(AsmToken::Comma, StringRef(TokStart, 1));
-  case '$': return AsmToken(AsmToken::Dollar, StringRef(TokStart, 1));
+  case '$':
+    if (LexMotorolaIntegers && isHexDigit(*CurPtr)) {
+      return LexDigit();
+    }
+
+    return AsmToken(AsmToken::Dollar, StringRef(TokStart, 1));
   case '@': return AsmToken(AsmToken::At, StringRef(TokStart, 1));
   case '\\': return AsmToken(AsmToken::BackSlash, StringRef(TokStart, 1));
   case '=':
     if (*CurPtr == '=') {
       ++CurPtr;
       return AsmToken(AsmToken::EqualEqual, StringRef(TokStart, 2));
     }
     return AsmToken(AsmToken::Equal, StringRef(TokStart, 1));
case '&':
     return AsmToken(AsmToken::Amp, StringRef(TokStart, 1));
   case '!':
     if (*CurPtr == '=') {
       ++CurPtr;
       return AsmToken(AsmToken::ExclaimEqual, StringRef(TokStart, 2));
     }
     return AsmToken(AsmToken::Exclaim, StringRef(TokStart, 1));
   case '%':
+    if (LexMotorolaIntegers && (*CurPtr == '0' || *CurPtr == '1')) {
+      return LexDigit();
+    }
+
     if (MAI.hasMipsExpressions()) {
       AsmToken::TokenKind Operator;
       unsigned OperatorLength;
 
       std::tie(Operator, OperatorLength) =
           StringSwitch<std::pair<AsmToken::TokenKind, unsigned>>(
               StringRef(CurPtr))
               .StartsWith("call16", {AsmToken::PercentCall16, 7})

llvm/lib/Target/M68k/MCTargetDesc/M68kMCAsmInfo.cpp

M68kELFMCAsmInfo::M68kELFMCAsmInfo(const Triple &T) {
   IsLittleEndian = false;
 
   // Debug Information
   SupportsDebugInformation = true;
 
   // Exceptions handling
   ExceptionsType = ExceptionHandling::DwarfCFI;
 
+  UseMotorolaIntegers = true;
   CommentString = ";";
 }

llvm/test/MC/AsmParser/motorola_integers.s

+# RUN: llvm-mc -triple i386-unknown-unknown -motorola-integers %s | FileCheck %s
+
+# CHECK: .set a, 2882400009
+.set a, $aBcDeF09
+# CHECK: .set b, 256
+.set b, $0100
+# CHECK: .set c, 10
+.set c, %01010
+# CHECK: .set d, 1
+.set d, %1

llvm/tools/llvm-mc/llvm-mc.cpp

 static cl::opt<bool> LexMasmIntegers(
     "masm-integers",
     cl::desc("Enable binary and hex masm integers (0b110 and 0ABCh)"));
 
 static cl::opt<bool> LexMasmHexFloats(
     "masm-hexfloats",
     cl::desc("Enable MASM-style hex float initializers (3F800000r)"));
 
+static cl::opt<bool> LexMotorolaIntegers(
+    "motorola-integers",
+    cl::desc("Enable binary and hex Motorola integers (%110 and $ABC)"));
+
 static cl::opt<bool> NoExecStack("no-exec-stack",
                                  cl::desc("File doesn't need an exec stack"));
 
 enum ActionType {
   AC_AsLex,
   AC_Assemble,
   AC_Disassemble,
   AC_MDisassemble,
static int AssembleInput(const char *ProgName, const Target *TheTarget,
 
   int SymbolResult = fillCommandLineSymbols(*Parser);
   if(SymbolResult)
     return SymbolResult;
   Parser->setShowParsedOperands(ShowInstOperands);
   Parser->setTargetParser(*TAP);
   Parser->getLexer().setLexMasmIntegers(LexMasmIntegers);
   Parser->getLexer().setLexMasmHexFloats(LexMasmHexFloats);
+  Parser->getLexer().setLexMotorolaIntegers(LexMotorolaIntegers);
 
   int Res = Parser->Run(NoInitialTextSection);
 
   return Res;
 }
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);