[X86_64] fix arg pass error in struct.

[CoTinker]

typedef long long t67 __attribute__((aligned (4)));
struct s67 {
  int a;
  t67 b;
};
void f67(struct s67 x) {
}

When classify:
a: Lo = Integer, Hi = NoClass
b: Lo = Integer, Hi = NoClass
struct S: Lo = Integer, Hi = NoClass

define dso_local void @f67(i64 %x.coerce) {

In this case, only one i64 register is used when the structure parameter is transferred, which is obviously incorrect.So we need to treat the split case specially. fix #85387.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

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

@llvm/pr-subscribers-clang

Author: Longsheng Mou (CoTinker)

Changes

typedef long long Alignll attribute((align (4))); struct S {
int a;
Alignll b;
};

When classify:
a: Lo = Integer, Hi = NoClass
b: Lo = Integer, Hi = NoClass
struct S: Lo = Integer, Hi = NoClass

In this case, only one i64 register is used when the structure parameter is transferred, which is obviously incorrect.So we need to treat the split case specially. fix #85387.

---

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

2 Files Affected:

• (modified) clang/lib/CodeGen/Targets/X86.cpp (+5)
• (modified) clang/test/CodeGen/X86/x86_64-arguments.c (+18)

diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp
index 2291c991fb1107..1a02d94a8eb530 100644
--- a/clang/lib/CodeGen/Targets/X86.cpp
+++ b/clang/lib/CodeGen/Targets/X86.cpp
@@ -1787,6 +1787,7 @@ void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo,
   Lo = Hi = NoClass;
 
   Class &Current = OffsetBase < 64 ? Lo : Hi;
+  bool IsSplit = OffsetBase < 64 && (OffsetBase + getContext().getTypeSize(Ty)) > 64;
   Current = Memory;
 
   if (const BuiltinType *BT = Ty->getAs<BuiltinType>()) {
@@ -1799,9 +1800,13 @@ void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo,
       Hi = Integer;
     } else if (k >= BuiltinType::Bool && k <= BuiltinType::LongLong) {
       Current = Integer;
+      if (IsSplit)
+        Hi = Integer;
     } else if (k == BuiltinType::Float || k == BuiltinType::Double ||
                k == BuiltinType::Float16 || k == BuiltinType::BFloat16) {
       Current = SSE;
+      if (IsSplit)
+        Hi = SSE;
     } else if (k == BuiltinType::Float128) {
       Lo = SSE;
       Hi = SSEUp;
diff --git a/clang/test/CodeGen/X86/x86_64-arguments.c b/clang/test/CodeGen/X86/x86_64-arguments.c
index cf5636cfd518b6..c8a215989cf9fc 100644
--- a/clang/test/CodeGen/X86/x86_64-arguments.c
+++ b/clang/test/CodeGen/X86/x86_64-arguments.c
@@ -533,6 +533,24 @@ typedef float t66 __attribute__((__vector_size__(128), __aligned__(128)));
 void f66(t66 a0) {
 }
 
+typedef long long t67 __attribute__((align (4)));
+struct s67 {
+  int a;
+  t67 b;
+};
+// CHECK-LABEL: define{{.*}} void @f67(i64 %x.coerce0, i32 %x.coerce1)
+void f67(struct s67 x) {
+}
+
+typedef double t68 __attribute__((align (4)));
+struct s68 {
+  int a;
+  t68 b;
+};
+// CHECK-LABEL: define{{.*}} void @f68(i64 %x.coerce0, double %x.coerce1)
+void f68(struct s68 x) {
+}
+
 /// The synthesized __va_list_tag does not have file/line fields.
 // CHECK:      = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "__va_list_tag",
 // CHECK-NOT:  file:

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[efriedma-quic]

I think the logic the code is using is that t67 is actually properly aligned: it's a type with size 8 and alignment 4, so everything is fine. If that's not right, we should tweak the relevant logic (around line 2104).

Specifically checking whether a type crosses an 8-byte boundary seems wrong; the only relevant text I can find in the standard is just "if the size of an object is larger than eight eightbytes, or it contains unaligned fields, it has class MEMORY."

[CoTinker]

Thanks, I'll modify it according to this.

[CoTinker]

Which is more appropriate to pass this structure through, memory or register?

[efriedma-quic]

gcc uses memory, and the ABI standard doesn't explicitly contradict it, so let's just go with that.