[PATCH] D90441: [X86] Add support for vex, vex2, vex3, and evex for MASM

Wed Nov 4 08:34:32 PST 2020

craig.topper added inline comments.

================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:2851
+    // Parse MASM style pseudo prefixes.
+    // FIXME: This prefix should only be used for MASM, not for intel-syntax.
+    if (isParsingIntelSyntax()) {
----------------
epastor wrote:
> LiuChen3 wrote:
> > I tried to limit to MASM. But I found that the  'isParsingMSInlineAsm()' is not accurate.  And then I tried to transmit 'ParsingMSInlineAsm' information correctly in AsmPrinterInlineAsm.cpp (according to the '-fasm-blocks' option). But I was surprised to find that isParsingMSInlineAsm() is actually used as the argument of 'MatchingInlineAsm' in 'MatchAndEmitInstruction()'. This makes me confused. Should that 'MatchingInlineAsm' be 'MatchingMSInlineAsm' ?Is this MatchingInlineAsm only used by llvm-ml.
> > It difficult to limit this to MASM at the moment. 
> llvm-ml attempts not to touch **anything** involving inline assembly so far. The signal that MasmParser.cpp is involved is `Parser.isParsingMasm()`. However... while I can't answer the majority of this without more research, I suspect you're correct that `MatchingInlineAsm` is misnamed. We need to check this, and if so, we should rename it to avoid confusion.
MS inline assembly is parsed twice. Once by  clang to find names of C/C++ variables. And again in the backend. GNU inline assembly is only parsed in the backend since variable names are bound explicitly and not referenced in the assembly text.

IsParsingInlineAsm is set during the clang parsing.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90441