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

[LiuChen via Phabricator via llvm-commits]

LiuChen3 added inline comments.


================
Comment at: clang/test/CodeGen/X86/ms-inline-asm-prefix.c:1
+// RUN:%clang_cc1 %s -ferror-limit 0 -triple=x86_64-pc-widows-msvc -target-feature +avx512f -target-feature +avx2 -target-feature +avx512vl -fasm-blocks -mllvm -x86-asm-syntax=intel -S -o -  | FileCheck %s -check-prefix CHECK
+
----------------
LiuChen3 wrote:
> epastor wrote:
> > epastor wrote:
> > > pengfei wrote:
> > > > pengfei wrote:
> > > > > pengfei wrote:
> > > > > > Maybe need `// REQUIRES: x86-registered-target`
> > > > > You may need add att check too since you modified the att code.
> > > > Should it be avalible only when `-fms-compatibility`
> > > The triple is misspelled; it should be `x86_64-pc-windows-msvc` (the "n" in windows is missing)
> > A broader question: As written, this applies to anything in Intel syntax. Is this an Intel syntax feature, or a MASM feature?
> Thanks for your review. After checking with the people of MSVC, I found that prefix without braces is not intel syntax. Actually, we don't know if there any document says what the prefix should be. At least, gcc does have the "{}" in intel syntax, so does clang. We currently decide to only support parsing the prefix without MSVC.
I am not sure. But I think -fasm-blocks, -fms-extensions  also support MASM.


================
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()) {
----------------
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. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90441