LiuChen3 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()) {
----------------
craig.topper wrote:
> 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.
Thanks. That's make sense. 
So 'MatchingInlineAsm' in MatchAndEmitInstruction() more like 
'MatchingMSInlineAsm' and can only be set when parsing the instructions first 
time. And the second time parser can not set 'setParsingMSInlineAsm(true)'. 
That's make difficult to limit the scope to MASM for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90441

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to