Skip to content

Conversation

@kin4stat
Copy link

@kin4stat kin4stat commented Oct 20, 2024

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

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
@github-actions
Copy link

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 llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Oct 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 20, 2024

@llvm/pr-subscribers-clang-codegen

Author: Daniil (kin4stat)

Changes

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/llvm-project#63417


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/MicrosoftCXXABI.cpp (+4)
  • (modified) clang/test/CodeGenCXX/homogeneous-aggregates.cpp (+19)
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

@llvmbot
Copy link
Member

llvmbot commented Oct 20, 2024

@llvm/pr-subscribers-clang

Author: Daniil (kin4stat)

Changes

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/llvm-project#63417


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/MicrosoftCXXABI.cpp (+4)
  • (modified) clang/test/CodeGenCXX/homogeneous-aggregates.cpp (+19)
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

@kin4stat kin4stat force-pushed the fix_x86_64_msvc_abi branch from f2c5998 to 8612770 Compare October 20, 2024 21:59
return true;
}
if (CGM.getTarget().getTriple().isX86() &&
CGM.getABIInfo().isHomogeneousAggregate(Ty, Base, NumElts)) {
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

msvc abi incompatibility: x86 __vectorcall changes rules for returning C++ HFA/HVA classes

3 participants