[X86] merge "={eax}" and "~{eax}" into "=&eax" for MSInlineASM

[FreddyYe]

Summary

This patch is to fix an end-to-end bug (https://gcc.godbolt.org/z/GG44f6) on recent rewrite on back-end's RegAllocFast.cpp. I created a fix patch in back-end before (https://reviews.llvm.org/D93983). Both solutions are reasonable to me. The proof of this patch is that GNU inline ASM doesn't support setting one register as both "output" and "clobber"(Both clang and gcc enable this feature https://gcc.godbolt.org/z/4n6W56). Moreover, since MS inline ASM didn't define semantics of "output" or "clobber". So I think LLVM IR should follow this rule with GNU inline ASM, too.

[pengfei]

clang/lib/CodeGen/CGStmt.cpp

2488

={eax} is set in X86_32TargetCodeGenInfo::addReturnRegisterOutputs, which also handles EAX:EDX case by =A. I think it's better handle it in the function to cover the other case as well.

[FreddyYe]

clang/lib/CodeGen/CGStmt.cpp

2488

THX for review. Agree. I'll update.

[FreddyYe]

consider "=A" into the patch

[pengfei]

clang/lib/CodeGen/CGStmt.cpp

2493

Should consider the Clobber == "edx"?

[FreddyYe]

consider Clobber == "edx"

[FreddyYe]

Fix Lint: Pre-merge checks

[FreddyYe]

Ping?

[pengfei]

clang/lib/CodeGen/CGStmt.cpp

2490

If Clobber is edx only, we shouldn't change "={eax}" to "=&{eax}".

[FreddyYe]

clang/lib/CodeGen/CGStmt.cpp

2490

Yes! I'll update and add a test. THX for review!

[FreddyYe]

If Clobber is edx only, don't change "={eax}" to "=&{eax}".

[FreddyYe]

refine clang-format

[pengfei]

LGTM.

clang/lib/CodeGen/CGStmt.cpp

2491

I'm not sure if there's real case that there's "={edx}" for MS inline asm, but there's no problem we handle it here.

clang/lib/CodeGen/CGStmt.cpp

 //===--- CGStmt.cpp - Emit LLVM Code from Statements ----------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
else if (Clobber != "cc") {
       Clobber = getTarget().getNormalizedGCCRegisterName(Clobber);
       if (CGM.getCodeGenOpts().StackClashProtector &&
           getTarget().isSPRegName(Clobber)) {
         CGM.getDiags().Report(S.getAsmLoc(),
                               diag::warn_stack_clash_protection_inline_asm);
       }
     }
 
+    if (isa<MSAsmStmt>(&S)) {
+      if (Clobber == "eax" || Clobber == "edx" ) {
+        if (Constraints.find("=&A") != std::string::npos)
+          continue;
+        std::string::size_type position1 = Constraints.find("={eax}");
+        if (position1 != std::string::npos) {
+          Constraints.insert(position1 + 1, "&");
+          continue;
+        }
+        std::string::size_type position2 = Constraints.find("=A");
+        if (position2 != std::string::npos) {
+          Constraints.insert(position2 + 1, "&");
+          continue;
+        }
+      }
+    }
     if (!Constraints.empty())
       Constraints += ',';
 
     Constraints += "~{";
     Constraints += Clobber;
     Constraints += '}';
   }
 

clang/test/CodeGen/ms-inline-asm.c

 // CHECK: t10
 // CHECK: [[r:%[a-zA-Z0-9]+]] = alloca i32, align 4
 // CHECK: [[I:%[a-zA-Z0-9]+]] = alloca i32, align 4
 // CHECK: [[J:%[a-zA-Z0-9]+]] = alloca i32, align 4
 // CHECK: store i32 1, i32* [[I]], align 4
 // CHECK: call i32 asm sideeffect inteldialect
 // CHECK-SAME: mov eax, $2
 // CHECK-SAME: mov $0, eax
-// CHECK-SAME: "=*m,={eax},*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, i32* %{{.*}})
+// CHECK-SAME: "=*m,=&{eax},*m,~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, i32* %{{.*}})
 // CHECK: [[RET:%[a-zA-Z0-9]+]] = load i32, i32* [[J]], align 4
 // CHECK: ret i32 [[RET]]
 }
 
 void t11(void) {
   __asm mov eax, 1
 // CHECK: t11
 // CHECK: call void asm sideeffect inteldialect "mov eax, $$1", "~{eax},~{dirflag},~{fpsr},~{flags}"()
unsigned t12(void) {
   }
   return j + m;
 // CHECK: t12
 // CHECK: call i32 asm sideeffect inteldialect
 // CHECK-SAME: mov eax, $3
 // CHECK-SAME: mov $0, eax
 // CHECK-SAME: mov eax, $4
 // CHECK-SAME: mov $1, eax
-// CHECK-SAME: "=*m,=*m,={eax},*m,*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, i32* %{{.*}}, i32* %{{.*}}, i32* %{{.*}})
+// CHECK-SAME: "=*m,=*m,=&{eax},*m,*m,~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, i32* %{{.*}}, i32* %{{.*}}, i32* %{{.*}})
 }
 
 void t13() {
   char i = 1;
   short j = 2;
   __asm movzx eax, i
   __asm movzx eax, j
 // CHECK-LABEL: define{{.*}} void @t13()

clang/test/CodeGenCXX/ms-inline-asm-return.cpp

 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 %s -triple i686-pc-windows-msvc -emit-llvm -o - -fasm-blocks | FileCheck %s
 
 // Check that we take EAX or EAX:EDX and return it from these functions for MSVC
 // compatibility.
 
 extern "C" {
 
 long long f_i64() {
   __asm {
     mov eax, 1
     mov edx, 1
   }
 }
 // CHECK-LABEL: define dso_local i64 @f_i64()
-// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=A,~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=&A,{{.*}}"
+// CHECK: ret i64 %[[r]]
+
+long long f_i64_reverse() {
+  __asm {
+    mov edx, 1
+    mov eax, 1
+  }
+}
+// CHECK-LABEL: define dso_local i64 @f_i64_reverse()
+// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov edx, $$1\0A\09mov eax, $$1", "=&A,{{.*}}"
 // CHECK: ret i64 %[[r]]
 
 int f_i32() {
   __asm {
     mov eax, 1
     mov edx, 1
   }
 }
 // CHECK-LABEL: define dso_local i32 @f_i32()
-// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=&{eax},~{edx},{{.*}}"
+// CHECK: ret i32 %[[r]]
+
+int f_i32_reverse() {
+  __asm {
+    mov edx, 1
+    mov eax, 1
+  }
+}
+// CHECK-LABEL: define dso_local i32 @f_i32_reverse()
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov edx, $$1\0A\09mov eax, $$1", "=&{eax},~{edx},{{.*}}"
 // CHECK: ret i32 %[[r]]
 
 short f_i16() {
   __asm {
     mov eax, 1
     mov edx, 1
   }
 }
 // CHECK-LABEL: define dso_local signext i16 @f_i16()
-// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=&{eax},~{edx},{{.*}}"
 // CHECK: %[[r_i16:[^ ]*]] = trunc i32 %[[r]] to i16
 // CHECK: ret i16 %[[r_i16]]
 
 char f_i8() {
   __asm {
     mov eax, 1
     mov edx, 1
   }
 }
 // CHECK-LABEL: define dso_local signext i8 @f_i8()
-// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=&{eax},~{edx},{{.*}}"
 // CHECK: %[[r_i8:[^ ]*]] = trunc i32 %[[r]] to i8
 // CHECK: ret i8 %[[r_i8]]
 
 bool f_i1() {
   __asm {
     mov eax, 1L
     mov edx, 1U
   }
 }
 // CHECK-LABEL: define dso_local zeroext i1 @f_i1()
-// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=&{eax},~{edx},{{.*}}"
 // CHECK: %[[r_i8:[^ ]*]] = trunc i32 %[[r]] to i8
 // CHECK: store i8 %[[r_i8]], i8* %{{.*}}
 // CHECK: %[[r_i1:[^ ]*]] = load i1, i1* %{{.*}}
 // CHECK: ret i1 %[[r_i1]]
 
 struct FourChars {
   char a, b, c, d;
 };
 FourChars f_s4() {
   __asm {
     mov eax, 0x01010101
   }
 }
 // CHECK-LABEL: define dso_local i32 @f_s4()
-// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$16843009", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$16843009", "=&{eax},{{.*}}"
 // CHECK: store i32 %[[r]], i32* %{{.*}}
 // CHECK: %[[r_i32:[^ ]*]] = load i32, i32* %{{.*}}
 // CHECK: ret i32 %[[r_i32]]
 
 struct EightChars {
   char a, b, c, d, e, f, g, h;
 };
 EightChars f_s8() {
   __asm {
     mov eax, 01010101h
     mov edx, 01010101b
   }
 }
 // CHECK-LABEL: define dso_local i64 @f_s8()
-// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov eax, $$16843009\0A\09mov edx, $$85", "=A,~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov eax, $$16843009\0A\09mov edx, $$85", "=&A,{{.*}}"
 // CHECK: store i64 %[[r]], i64* %{{.*}}
 // CHECK: %[[r_i64:[^ ]*]] = load i64, i64* %{{.*}}
 // CHECK: ret i64 %[[r_i64]]
 
 } // extern "C"
 
 int main() {
   __asm xor eax, eax
 }
 // CHECK-LABEL: define dso_local i32 @main()
-// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "xor eax, eax", "={eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "xor eax, eax", "=&{eax},{{.*}}"
 // CHECK: ret i32 %[[r]]