[PATCH] D73227: [ms] [llvm-ml] Use default RIP-relative addressing for x64 MASM.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/include/llvm/MC/MCParser/MCAsmParser.h:168
 
-  virtual void setParsingInlineAsm(bool V) = 0;
-  virtual bool isParsingInlineAsm() = 0;
+  virtual void setParsingInlineMSAsm(bool V) = 0;
+  virtual bool isParsingInlineMSAsm() = 0;
----------------
I guess I'd prefer `isParsingMSInlineAsm`. Do you mind uploading that and committing it separately?


================
Comment at: llvm/include/llvm/MC/MCParser/MCAsmParser.h:171
+
+  virtual bool isParsingMasm() const { return false; }
 
----------------
We already have ASMLexer::LexMasmIntegers. Can this be collapsed with that so that we don't have two indicators of "masm-ness"?


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:2062-2063
     return ErrorOperand(Start, ErrMsg);
-  if (isParsingInlineAsm())
+  if (isParsingInlineMSAsm())
     return CreateMemForInlineAsm(RegNo, Disp, BaseReg, IndexReg,
                                  Scale, Start, End, Size, SM.getSymName(),
----------------
Does this not make the check for `isParsingInlindMSAsm()` below redundant? If it's true, we always return here.


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:2070-2072
+    return X86Operand::CreateMem(getPointerWidth(), RegNo, Disp, BaseReg,
+                                 IndexReg, Scale, Start, End, Size,
+                                 /*DefaultBaseReg=*/X86::RIP);
----------------
What ends up going wrong if we set the basereg to RIP in these conditions when it is not already set? I'm wondering if it would be cleaner to do the defaulting here, and then adjust the downstream code to treat RIP-based memory operands the same as no-base memory operands.


================
Comment at: llvm/lib/Target/X86/AsmParser/X86Operand.h:63
     unsigned BaseReg;
+    unsigned DefaultBaseReg;
     unsigned IndexReg;
----------------
I'd really like to get by without adding more operand members that don't correspond to parts of the x86 addressing mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73227