-
Notifications
You must be signed in to change notification settings - Fork 11.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[X86_64] fix arg pass error in struct. #86902
Conversation
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Longsheng Mou (CoTinker) Changes
When classify:
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/86902.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp
index 2291c991fb1107..de0dfe32a54d3a 100644
--- a/clang/lib/CodeGen/Targets/X86.cpp
+++ b/clang/lib/CodeGen/Targets/X86.cpp
@@ -2100,8 +2100,12 @@ void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo,
postMerge(Size, Lo, Hi);
return;
}
+
+ bool InMemory = Offset % getContext().getTypeAlign(i->getType()) ||
+ (i->getType()->getAs<BuiltinType>() &&
+ Offset % getContext().getTypeSize(i->getType()));
// Note, skip this test for bit-fields, see below.
- if (!BitField && Offset % getContext().getTypeAlign(i->getType())) {
+ if (!BitField && InMemory) {
Lo = Memory;
postMerge(Size, Lo, Hi);
return;
diff --git a/clang/test/CodeGen/X86/x86_64-arguments.c b/clang/test/CodeGen/X86/x86_64-arguments.c
index cf5636cfd518b6..82845f0a2b31fd 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__((aligned (4)));
+struct s67 {
+ int a;
+ t67 b;
+};
+// CHECK-LABEL: define{{.*}} void @f67(ptr noundef byval(%struct.s67) align 8 %x)
+void f67(struct s67 x) {
+}
+
+typedef double t68 __attribute__((aligned (4)));
+struct s68 {
+ int a;
+ t68 b;
+};
+// CHECK-LABEL: define{{.*}} void @f68(ptr noundef byval(%struct.s68) align 8 %x)
+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:
|
@DavidSpickett why my buildkite always fail, while check-all success in local environment. Could you please take a look at it? |
Try rebasing your patch? If you're unlucky, you might have based your patch on a bad revision of main (and the buildbot just uses your branch as-is). |
Thank you very much, I'll try it. |
In some struct like s67, 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, using memory like gcc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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.