Skip to content
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

SwiftCallingConv: Fix the splitVectorEntry function #69953

Merged

Conversation

aschwaighofer
Copy link
Collaborator

When splitting an entry into multiple entries, the indices of the split entries are a combination of the original split entry's and the number of elements we split that entry to.

Failure to do so resulted in non-sensical entries leading e.g to assertion failures in getCoerceAndExpandTypes and runtime failures in Swift programs.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen labels Oct 23, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 23, 2023

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Arnold Schwaighofer (aschwaighofer)

Changes

When splitting an entry into multiple entries, the indices of the split entries are a combination of the original split entry's and the number of elements we split that entry to.

Failure to do so resulted in non-sensical entries leading e.g to assertion failures in getCoerceAndExpandTypes and runtime failures in Swift programs.


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/SwiftCallingConv.cpp (+4-3)
  • (modified) clang/test/CodeGen/64bit-swiftcall.c (+19-2)
diff --git a/clang/lib/CodeGen/SwiftCallingConv.cpp b/clang/lib/CodeGen/SwiftCallingConv.cpp
index 055dd3704386673..16fbf52a517db48 100644
--- a/clang/lib/CodeGen/SwiftCallingConv.cpp
+++ b/clang/lib/CodeGen/SwiftCallingConv.cpp
@@ -409,9 +409,10 @@ void SwiftAggLowering::splitVectorEntry(unsigned index) {
 
   CharUnits begin = Entries[index].Begin;
   for (unsigned i = 0; i != numElts; ++i) {
-    Entries[index].Type = eltTy;
-    Entries[index].Begin = begin;
-    Entries[index].End = begin + eltSize;
+    unsigned idx = index + i;
+    Entries[idx].Type = eltTy;
+    Entries[idx].Begin = begin;
+    Entries[idx].End = begin + eltSize;
     begin += eltSize;
   }
 }
diff --git a/clang/test/CodeGen/64bit-swiftcall.c b/clang/test/CodeGen/64bit-swiftcall.c
index 5290de2471e8e55..da6f18248c2af29 100644
--- a/clang/test/CodeGen/64bit-swiftcall.c
+++ b/clang/test/CodeGen/64bit-swiftcall.c
@@ -985,8 +985,8 @@ struct {
 } s;
 } union_het_vecint;
 TEST(union_het_vecint)
-// CHECK: define{{.*}} swiftcc void @return_union_het_vecint(ptr noalias sret([[UNION:.+]])
-// CHECK: define{{.*}} swiftcc void @take_union_het_vecint(ptr
+// CHECK: define{{.*}} swiftcc { i64, i64, i64, i64 } @return_union_het_vecint()
+// CHECK: define{{.*}} swiftcc void @take_union_het_vecint(i64 %0, i64 %1, i64 %2, i64 %3)
 
 typedef struct {
   float3 f3;
@@ -1044,3 +1044,20 @@ typedef struct {
 
 // CHECK-LABEL: use_atomic_padded(i64 %0, i64 %1)
 SWIFTCALL void use_atomic_padded(atomic_padded a) {}
+
+
+typedef union {
+  float4 v;
+  float3 v2;
+  struct {
+    float a;
+    float b;
+    float c;
+    float d;
+  };
+} vector_union;
+
+TEST(vector_union)
+
+// CHECK-LABEL: define swiftcc { float, float, float, float } @return_vector_union()
+// CHECK-LABEL: define swiftcc void @take_vector_union(float %0, float %1, float %2, float %3)

@aschwaighofer
Copy link
Collaborator Author

The failures are not related to this PR / pre-existing.

clang/docs/ReleaseNotes.rst: is not touched by this PR.

+ echo '*** Checking for trailing whitespace left in Clang source files ***'
--
  | *** Checking for trailing whitespace left in Clang source files ***
  | + grep -rnI '[[:blank:]] clang/lib clang/include clang/docs
  | clang/docs/ReleaseNotes.rst:108:  when targetting MSVC to match the behavior of MSVC.
  | + echo '*** Trailing whitespace has been found in Clang source files as described above ***'
  | *** Trailing whitespace has been found in Clang source files as described above ***

And the CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp failure also occurs in the baseline.

Copy link
Contributor

@rjmccall rjmccall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. LGTM.

When splitting an entry into multiple entries, the indices of the split entries
are a combination of the original split entry's and the number of elements we
split that entry to.
@aschwaighofer aschwaighofer force-pushed the fix_swiftagglowering_vector_unions branch from 82b9642 to 3dcc80d Compare October 27, 2023 14:50
@aschwaighofer aschwaighofer merged commit 08e9c46 into llvm:main Oct 27, 2023
3 checks passed
aschwaighofer added a commit to aschwaighofer/llvm-project that referenced this pull request Oct 30, 2023
…llvm#69953)

When splitting an entry into multiple entries, the indices of the split
entries are a combination of the original split entry's and the number
of elements we split that entry to.

Failure to do so resulted in non-sensical entries leading e.g to
assertion failures in `getCoerceAndExpandTypes` and runtime failures in
Swift programs.

(cherry picked from commit 08e9c46)

rdar://116852924
aschwaighofer added a commit to apple/llvm-project that referenced this pull request Nov 1, 2023
…lowering_stable_20230725

[stable/20230725] SwiftCallingConv: Fix the splitVectorEntry function (llvm#69953)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants