[AsmPrinter,X86] Hard code AT&T syntax input for module-level inline assembly for MSVC triples

[MaskRay]

clang-cl is conflating input assembly syntax with output assembly
syntax. It expects AT&T syntax input but Intel syntax output. This
conflicts with clang -c -masm=intel users that do expect -masm=intel to
control both input and output.

After #85367, we use AT&T dialect for module-level inline assembly
parsing, breaking assumptions by Chromium. Hard code MSVC to use
AT&T for now.

% cat a.c
asm("movq %rax, %rax");
int main() { asm("movq %rax, %rax"); }
% clang-cl -c a.c   # succeeded

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-x86

Author: Fangrui Song (MaskRay)

Changes

clang-cl is conflating input assembly syntax with output assembly
syntax. It expects AT&T syntax input but Intel syntax output. This
conflicts with clang -c -masm=intel users that do expect -masm=intel to
control both input and output.

After #85367, we use AT&T dialect for module-level inline assembly
parsing, breaking assumptions by Chromium. Hard code MSVC to use
AT&T for now.

---

Full diff: https://github.com/llvm/llvm-project/pull/85668.diff

2 Files Affected:

• (added) clang/test/CodeGen/X86/inline-asm-cl.c (+19)
• (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+3-1)

diff --git a/clang/test/CodeGen/X86/inline-asm-cl.c b/clang/test/CodeGen/X86/inline-asm-cl.c
new file mode 100644
index 00000000000000..15be500521ff9f
--- /dev/null
+++ b/clang/test/CodeGen/X86/inline-asm-cl.c
@@ -0,0 +1,19 @@
+// REQUIRES: x86-registered-target
+/// Some clang-cl users expect AT&T syntax input even if -x86-asm-syntax=intel is set.
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -S -fms-extensions -mllvm -x86-asm-syntax=intel %s -o - | FileCheck %s
+
+// CHECK:         .intel_syntax noprefix
+// CHECK:         mov     rax, rax
+// CHECK-LABEL: foo:
+// CHECK:         mov     rdx, rdx
+// CHECK:         mov     rdx, rdx
+
+asm("movq %rax, %rax");
+
+void foo() {
+  asm("movq %rdx, %rdx");
+
+  __asm {
+    mov rdx, rdx
+  }
+}
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index a15538755d73b3..20b3967af057f1 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -541,7 +541,9 @@ bool AsmPrinter::doInitialization(Module &M) {
     emitInlineAsm(
         M.getModuleInlineAsm() + "\n", *TM.getMCSubtargetInfo(),
         TM.Options.MCOptions, nullptr,
-        InlineAsm::AsmDialect(TM.getMCAsmInfo()->getAssemblerDialect()));
+        TM.getTargetTriple().isWindowsMSVCEnvironment()
+            ? InlineAsm::AD_ATT
+            : InlineAsm::AsmDialect(TM.getMCAsmInfo()->getAssemblerDialect()));
     OutStreamer->AddComment("End of file scope inline assembly");
     OutStreamer->addBlankLine();
   }

[zmodem]

https://g-issues.chromium.org/issues/330163686#comment5 suggested maybe the fix should be on the Chromium side

[MaskRay]

Thanks. Fixing it on the Chromium feels nicer to me as well. This should work around the code if you run into more code needing this :) Personally I suspect not, since module-level inline asm with instructions are very rarely used (non-instruction directives are uses more).

[nico]

Looks like we were able to fix code on our end and this here isn't needed. But maybe the current behavior isn't 100% ideal.

Here's what I think the situation currently is (please correct me if I'm wrong):

• With clang-cl, you get intel assembly output by default with /FA. That's good.
• With clang, you get AT&T assembly output by default with -S, and intel assembly output with -masm=intel. That's good.
• With clang, the syntax for asm() is AT&T by default, and intel with -masm=intel. That's good.
• With clang-cl, __asm { ... } blocks always use Intel syntax. That's good.
• With clang-cl, the syntax for asm() is intel by default, and there's no way to change this. (This is now consistent between module-level assembly and function-local assembly. It's good that they're consistent, that's better than before.)

The last point seems suboptimal to me. cl.exe doesn't support asm(), only __asm {}. So users that use asm() with clang-cl probably want to share that code with other platforms where they use asm() too.

Ideally, clang-cl could use intel asm output by default, intel asm input for __asm {} blocks, but at&t style asm input for asm(). And ideally it could also offer a flag to opt in to intel style asm input for asm(). (And the pragma mentioned in https://reviews.llvm.org/D113707 might be nice too.)

Does that sound right?

(I'm not saying you have to do any of this! Just want to check if we agree on this assessment of the state of the world.)

[MaskRay]

Looks like we were able to fix code on our end and this here isn't needed. But maybe the current behavior isn't 100% ideal.

Thanks. I am abandoning this workaround.

Here's what I think the situation currently is (please correct me if I'm wrong):

• With clang-cl, you get intel assembly output by default with /FA. That's good.
• With clang, you get AT&T assembly output by default with -S, and intel assembly output with -masm=intel. That's good.
• With clang, the syntax for asm() is AT&T by default, and intel with -masm=intel. That's good.
• With clang-cl, __asm { ... } blocks always use Intel syntax. That's good.
• With clang-cl, the syntax for asm() is intel by default, and there's no way to change this. (This is now consistent between module-level assembly and function-local assembly. It's good that they're consistent, that's better than before.)

The last point seems suboptimal to me. cl.exe doesn't support asm(), only __asm {}. So users that use asm() with clang-cl probably want to share that code with other platforms where they use asm() too.

Ideally, clang-cl could use intel asm output by default, intel asm input for __asm {} blocks, but at&t style asm input for asm(). And ideally it could also offer a flag to opt in to intel style asm input for asm(). (And the pragma mentioned in reviews.llvm.org/D113707 might be nice too.)

Does that sound right?

(I'm not saying you have to do any of this! Just want to check if we agree on this assessment of the state of the world.)

I agree with the analysis. The original bug for the improvement appears to be #34830 (comment)

I agree that AT&T input asm() and Intel output provides most convenience, but I do feel it slightly strange and wonder whether users need an explicit command line option to indicate this preference. @uweigand's https://reviews.llvm.org/D95444 (which hasn't landed) seems appealing as well, but I haven't read closely these patches.