[X86] Add ABI handling for __float128 to match with GCC

[phoebewang]

Fixes #74601

[llvmbot]

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-backend-x86

Author: Phoebe Wang (phoebewang)

Changes

Fixes #74601

---

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

2 Files Affected:

• (modified) clang/lib/CodeGen/Targets/X86.cpp (+2-1)
• (added) clang/test/CodeGen/X86/fp128-abi.c (+35)

diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp
index 2af24035043884..c4bf38056a673f 100644
--- a/clang/lib/CodeGen/Targets/X86.cpp
+++ b/clang/lib/CodeGen/Targets/X86.cpp
@@ -1795,7 +1795,8 @@ void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo,
     } else if (k >= BuiltinType::Bool && k <= BuiltinType::LongLong) {
       Current = Integer;
     } else if (k == BuiltinType::Float || k == BuiltinType::Double ||
-               k == BuiltinType::Float16 || k == BuiltinType::BFloat16) {
+               k == BuiltinType::Float16 || k == BuiltinType::BFloat16 ||
+               k == BuiltinType::Float128) {
       Current = SSE;
     } else if (k == BuiltinType::LongDouble) {
       const llvm::fltSemantics *LDF = &getTarget().getLongDoubleFormat();
diff --git a/clang/test/CodeGen/X86/fp128-abi.c b/clang/test/CodeGen/X86/fp128-abi.c
new file mode 100644
index 00000000000000..1c5d7cf1166ee1
--- /dev/null
+++ b/clang/test/CodeGen/X86/fp128-abi.c
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm  -target-feature +sse2 < %s | FileCheck %s --check-prefixes=CHECK
+
+struct st1 {
+  __float128 a;
+};
+
+struct st1 h1(__float128 a) {
+  // CHECK: define{{.*}}fp128 @h1(fp128
+  struct st1 x;
+  x.a = a;
+  return x;
+}
+
+__float128 h2(struct st1 x) {
+  // CHECK: define{{.*}}fp128 @h2(fp128
+  return x.a;
+}
+
+struct st2 {
+  __float128 a;
+  int b;
+};
+
+struct st2 h3(__float128 a, int b) {
+  // CHECK: define{{.*}}void @h3(ptr {{.*}}sret(%struct.st2)
+  struct st2 x;
+  x.a = a;
+  x.b = b;
+  return x;
+}
+
+__float128 h4(struct st2 x) {
+  // CHECK: define{{.*}}fp128 @h4(ptr {{.*}}byval(%struct.st2)
+  return x.a;
+}

[RKSimon]

Worth adding a non-SSE RUN?

[phoebewang]

This patch only changes for 64-bit ABI, non-SSE is not a valid case for 64-bit.
OTOH, -sse just generate identical output with +sse2, which doesn't match with GCC https://godbolt.org/z/4nxnhnovd
I think we may need to add a semacheck for it. Do you think we should add it with this patch or a follow up?

[RKSimon]

If you're dealing with it soon then handling it in a followup sounds good to me

[phoebewang]

I decided not to report error for -sse after investigating the current diagnosis machinism.
We didn't report the SSE error in semacheck but in backend due to backend crash.
__float128 doesn't have crash issue and can be passed on GPR registers without SSE enabled.
IIUC, we prefer to be compatible with early version rather than always to GCC. So I'd like to keep the convention as is.

[RKSimon]

OK - so we have test coverage for non-SSE cases already in the backend?

[phoebewang]

Yes, e.g, llvm/test/CodeGen/X86/{soft-fp,x87}.ll, though I doubt if they can work in reality since they are calling to the same lib functions.

[phoebewang]

Ping @RKSimon

[RKSimon]

Add a Release notes entry? I always forget exactly what we need to do for ABI fixes/tweaks

[phoebewang]

Add a Release notes entry? I always forget exactly what we need to do for ABI fixes/tweaks

Goot point! Done.

[RKSimon]

LGTM - please mention in the commit message + release notes that this matches GCC behaviour