[PATCH] D72680: [ms] [llvm-ml] Add a draft MASM parser

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

This is a big diff. But AsmParser is a big interface, so I'm not sure it can be all that much smaller.

Some parts of MasmParser looks like they're from the other class and not needed. Is the idea to copy most things over for now and then delete dead code later, to keep the git diff clearer?

Maybe the tryParseRegister bits could be in a separate change?



================
Comment at: llvm/include/llvm/MC/MCParser/MCAsmParser.h:311
 
+MCAsmParser *createMCMasmParser(SourceMgr &, MCContext &, MCStreamer &,
+                                const MCAsmInfo &, unsigned CB = 0);
----------------
This looks very similar to the line above; maybe add a comment that draws attention to the extra "M".


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:680
+    report_fatal_error(
+        "Need to implement createELFMasmParser for ELF format.");
     break;
----------------
Maybe say "llvm-ml currently only supports COFF output" and use one branch for all the other file types.


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:1280
 const MCExpr *
-AsmParser::applyModifierToExpr(const MCExpr *E,
-                               MCSymbolRefExpr::VariantKind Variant) {
+MasmParser::applyModifierToExpr(const MCExpr *E,
+                                MCSymbolRefExpr::VariantKind Variant) {
----------------
If we need this, should this be in the super class?


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:1487
                                          MCBinaryExpr::Opcode &Kind,
                                          bool ShouldUseLogicalShr) {
   switch (K) {
----------------
Likely don't need this one?


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:1563
 
 static unsigned getGNUBinOpPrecedence(AsmToken::TokenKind K,
                                       MCBinaryExpr::Opcode &Kind,
----------------
ditto


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72680/new/

https://reviews.llvm.org/D72680