-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang][codegen] Fix ABI for HVA/HFA returns on x86_64 MSVC #113104
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
base: main
Are you sure you want to change the base?
Conversation
MSVC normally has a bunch of restrictions on returning values directly which don't apply to passing values directly. (This roughly corresponds to the definition of a C++14 aggregate.) However, these restrictions don't apply to HVAs and HFAs; make sure we check for that. Fixes llvm#63417
|
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 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. |
|
@llvm/pr-subscribers-clang-codegen Author: Daniil (kin4stat) ChangesMSVC normally has a bunch of restrictions on returning values directly which don't apply to passing values directly. (This roughly corresponds to the definition of a C++14 aggregate.) However, these restrictions don't apply to HVAs and HFAs; make sure we check for that. Fixes llvm/llvm-project#63417 Full diff: https://github.com/llvm/llvm-project/pull/113104.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index 0b0b45ffead92f..fa5d83243f60fe 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1110,6 +1110,10 @@ static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
isa<VectorType>(Base)) {
return true;
}
+ if (CGM.getTarget().getTriple().isX86() &&
+ CGM.getABIInfo().isHomogeneousAggregate(Ty, Base, NumElts)) {
+ return true;
+ }
// We use the C++14 definition of an aggregate, so we also
// check for:
diff --git a/clang/test/CodeGenCXX/homogeneous-aggregates.cpp b/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
index 63ffc6b5bfac84..cf8019d2f18964 100644
--- a/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
+++ b/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
@@ -302,3 +302,22 @@ struct test2 : base2 { test2(double); protected: double v2;};
test2 f(test2 *x) { return *x; }
// WOA64: define dso_local void @"?f@pr62223@@YA?AUtest2@1@PEAU21@@Z"(ptr dead_on_unwind inreg noalias writable sret(%"struct.pr62223::test2") align 8 %{{.*}}, ptr noundef %{{.*}})
}
+
+namespace pr113104 {
+struct HFA {
+ float a;
+ float b;
+};
+
+using HVA = float __attribute__((__vector_size__(16), __aligned__(16)));
+
+struct base_hfa { HFA v1; };
+struct test_hfa : base_hfa { test_hfa(double); protected: HFA v2;};
+test_hfa CC f(test_hfa *x) { return *x; }
+// X64: define dso_local x86_vectorcallcc %"struct.pr113104::test_hfa" @"\01_ZN8pr1131041fEPNS_8test_hfaE@@8"(ptr noundef %x)
+
+struct base_hva { HVA v1; };
+struct test_hva : base_hva { test_hva(double); protected: HVA v2;};
+test_hva CC f(test_hva *x) { return *x; }
+// X64: define dso_local x86_vectorcallcc %"struct.pr113104::test_hva" @"\01_ZN8pr1131041fEPNS_8test_hvaE@@8"(ptr noundef %x)
+}
\ No newline at end of file
|
|
@llvm/pr-subscribers-clang Author: Daniil (kin4stat) ChangesMSVC normally has a bunch of restrictions on returning values directly which don't apply to passing values directly. (This roughly corresponds to the definition of a C++14 aggregate.) However, these restrictions don't apply to HVAs and HFAs; make sure we check for that. Fixes llvm/llvm-project#63417 Full diff: https://github.com/llvm/llvm-project/pull/113104.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index 0b0b45ffead92f..fa5d83243f60fe 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1110,6 +1110,10 @@ static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
isa<VectorType>(Base)) {
return true;
}
+ if (CGM.getTarget().getTriple().isX86() &&
+ CGM.getABIInfo().isHomogeneousAggregate(Ty, Base, NumElts)) {
+ return true;
+ }
// We use the C++14 definition of an aggregate, so we also
// check for:
diff --git a/clang/test/CodeGenCXX/homogeneous-aggregates.cpp b/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
index 63ffc6b5bfac84..cf8019d2f18964 100644
--- a/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
+++ b/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
@@ -302,3 +302,22 @@ struct test2 : base2 { test2(double); protected: double v2;};
test2 f(test2 *x) { return *x; }
// WOA64: define dso_local void @"?f@pr62223@@YA?AUtest2@1@PEAU21@@Z"(ptr dead_on_unwind inreg noalias writable sret(%"struct.pr62223::test2") align 8 %{{.*}}, ptr noundef %{{.*}})
}
+
+namespace pr113104 {
+struct HFA {
+ float a;
+ float b;
+};
+
+using HVA = float __attribute__((__vector_size__(16), __aligned__(16)));
+
+struct base_hfa { HFA v1; };
+struct test_hfa : base_hfa { test_hfa(double); protected: HFA v2;};
+test_hfa CC f(test_hfa *x) { return *x; }
+// X64: define dso_local x86_vectorcallcc %"struct.pr113104::test_hfa" @"\01_ZN8pr1131041fEPNS_8test_hfaE@@8"(ptr noundef %x)
+
+struct base_hva { HVA v1; };
+struct test_hva : base_hva { test_hva(double); protected: HVA v2;};
+test_hva CC f(test_hva *x) { return *x; }
+// X64: define dso_local x86_vectorcallcc %"struct.pr113104::test_hva" @"\01_ZN8pr1131041fEPNS_8test_hvaE@@8"(ptr noundef %x)
+}
\ No newline at end of file
|
f2c5998 to
8612770
Compare
| return true; | ||
| } | ||
| if (CGM.getTarget().getTriple().isX86() && | ||
| CGM.getABIInfo().isHomogeneousAggregate(Ty, Base, NumElts)) { |
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.
This looks like it might affect non-vectorcall calls; can you check (and add a testcase)?
Please also add test coverage for 32-bit x86.
MSVC normally has a bunch of restrictions on returning values directly which don't apply to passing values directly. (This roughly corresponds to the definition of a C++14 aggregate.) However, these restrictions don't apply to HVAs and HFAs; make sure we check for that.
Code from tests on godbolt:
https://godbolt.org/z/3hnGxYaah
Fixes #63417