[clang][SPARC] Pass 16-aligned 16-byte structs as i128 in CC #155829
+45
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This coerces 16-byte C structs that are 16-byte aligned as an i128 in LLVM IR. Additionally, since i128 values will be lowered as split i64 pairs in the backend, correctly set the alignment of such arguments as 16 bytes. This should make it compliant with the ABI specification and fix llvm#144709.
llvmbot
added
clang
|
@beetrees does this help? |
|
@llvm/pr-subscribers-backend-sparc @llvm/pr-subscribers-clang-codegen Author: Koakuma (koachan) ChangesThis coerces 9 to 16-byte C structs that are 16-byte aligned as an i128 in LLVM IR. Additionally, since i128 values will be lowered as split i64 pairs in the backend, correctly set the alignment of such arguments as 16 bytes. This should make clang compliant with the ABI specification and fix #144709. Full diff: https://github.com/llvm/llvm-project/pull/155829.diff 4 Files Affected:
diff --git a/clang/lib/CodeGen/Targets/Sparc.cpp b/clang/lib/CodeGen/Targets/Sparc.cpp
index 5f3c15d106eb6..b6c8fdcfe29b6 100644
--- a/clang/lib/CodeGen/Targets/Sparc.cpp
+++ b/clang/lib/CodeGen/Targets/Sparc.cpp
@@ -228,6 +228,7 @@ SparcV9ABIInfo::classifyType(QualType Ty, unsigned SizeLimit) const {
return ABIArgInfo::getIgnore();
uint64_t Size = getContext().getTypeSize(Ty);
+ unsigned Alignment = getContext().getTypeAlign(Ty);
// Anything too big to fit in registers is passed with an explicit indirect
// pointer / sret pointer.
@@ -275,10 +276,14 @@ SparcV9ABIInfo::classifyType(QualType Ty, unsigned SizeLimit) const {
// Try to use the original type for coercion.
llvm::Type *CoerceTy = CB.isUsableType(StrTy) ? StrTy : CB.getType();
+ // We use a pair of i64 for 9-16 byte aggregate with 8 byte alignment.
+ // For 9-16 byte aggregates with 16 byte alignment, we use i128.
+ llvm::Type *WideTy = llvm::Type::getIntNTy(getVMContext(), 128);
+ bool UseI128 = (Size > 64) && (Size <= 128) && (Alignment == 128);
+
if (CB.InReg)
- return ABIArgInfo::getDirectInReg(CoerceTy);
- else
- return ABIArgInfo::getDirect(CoerceTy);
+ return ABIArgInfo::getDirectInReg(UseI128 ? WideTy : CoerceTy);
+ return ABIArgInfo::getDirect(UseI128 ? WideTy : CoerceTy);
}
RValue SparcV9ABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
diff --git a/clang/test/CodeGen/sparcv9-abi.c b/clang/test/CodeGen/sparcv9-abi.c
index 5a3d64fd37889..9819d07425274 100644
--- a/clang/test/CodeGen/sparcv9-abi.c
+++ b/clang/test/CodeGen/sparcv9-abi.c
@@ -25,12 +25,21 @@ long double f_ld(long double x) { return x; }
struct empty {};
struct emptyarr { struct empty a[10]; };
+// 16-byte structs with 16-byte alignment gets passed as if i128.
+struct align16 { _Alignas(16) int x; };
+
// CHECK-LABEL: define{{.*}} i64 @f_empty(i64 %x.coerce)
struct empty f_empty(struct empty x) { return x; }
// CHECK-LABEL: define{{.*}} i64 @f_emptyarr(i64 %x.coerce)
struct empty f_emptyarr(struct emptyarr x) { return x.a[0]; }
+// CHECK-LABEL: define{{.*}} void @f_aligncaller(i128 %a.coerce)
+void f_aligncallee(int pad, struct align16 a);
+void f_aligncaller(struct align16 a) {
+ f_aligncallee(0, a);
+}
+
// CHECK-LABEL: define{{.*}} i64 @f_emptyvar(i32 noundef zeroext %count, ...)
long f_emptyvar(unsigned count, ...) {
long ret;
diff --git a/llvm/lib/Target/Sparc/SparcISelLowering.cpp b/llvm/lib/Target/Sparc/SparcISelLowering.cpp
index d01218f573dc2..e5ed9d267afed 100644
--- a/llvm/lib/Target/Sparc/SparcISelLowering.cpp
+++ b/llvm/lib/Target/Sparc/SparcISelLowering.cpp
@@ -115,7 +115,8 @@ static bool Analyze_CC_Sparc64_Full(bool IsReturn, unsigned &ValNo, MVT &ValVT,
// Stack space is allocated for all arguments starting from [%fp+BIAS+128].
unsigned size = (LocVT == MVT::f128) ? 16 : 8;
- Align alignment = (LocVT == MVT::f128) ? Align(16) : Align(8);
+ Align alignment =
+ (LocVT == MVT::f128 || ArgFlags.isSplit()) ? Align(16) : Align(8);
unsigned Offset = State.AllocateStack(size, alignment);
unsigned Reg = 0;
diff --git a/llvm/test/CodeGen/SPARC/64abi.ll b/llvm/test/CodeGen/SPARC/64abi.ll
index 6485a7f13e8d5..dc8c9af4a5185 100644
--- a/llvm/test/CodeGen/SPARC/64abi.ll
+++ b/llvm/test/CodeGen/SPARC/64abi.ll
@@ -473,8 +473,8 @@ declare i64 @receive_fp128(i64 %a, ...)
; HARD-DAG: ldx [%sp+[[Offset0]]], %o2
; HARD-DAG: ldx [%sp+[[Offset1]]], %o3
; SOFT-DAG: mov %i0, %o0
-; SOFT-DAG: mov %i1, %o1
; SOFT-DAG: mov %i2, %o2
+; SOFT-DAG: mov %i3, %o3
; CHECK: call receive_fp128
define i64 @test_fp128_variable_args(i64 %a, fp128 %b) {
entry:
@@ -482,6 +482,19 @@ entry:
ret i64 %0
}
+declare i64 @receive_i128(i64 %a, i128 %b)
+
+; CHECK-LABEL: test_i128_args:
+; CHECK: mov %i3, %o3
+; CHECK: mov %i2, %o2
+; CHECK: mov %i0, %o0
+; CHECK: call receive_i128
+define i64 @test_i128_args(i64 %a, i128 %b) {
+entry:
+ %0 = call i64 @receive_i128(i64 %a, i128 %b)
+ ret i64 %0
+}
+
; CHECK-LABEL: test_call_libfunc:
; HARD: st %f1, [%fp+[[Offset0:[0-9]+]]]
; HARD: fmovs %f3, %f1
|
|
@llvm/pr-subscribers-clang Author: Koakuma (koachan) ChangesThis coerces 9 to 16-byte C structs that are 16-byte aligned as an i128 in LLVM IR. Additionally, since i128 values will be lowered as split i64 pairs in the backend, correctly set the alignment of such arguments as 16 bytes. This should make clang compliant with the ABI specification and fix #144709. Full diff: https://github.com/llvm/llvm-project/pull/155829.diff 4 Files Affected:
diff --git a/clang/lib/CodeGen/Targets/Sparc.cpp b/clang/lib/CodeGen/Targets/Sparc.cpp
index 5f3c15d106eb6..b6c8fdcfe29b6 100644
--- a/clang/lib/CodeGen/Targets/Sparc.cpp
+++ b/clang/lib/CodeGen/Targets/Sparc.cpp
@@ -228,6 +228,7 @@ SparcV9ABIInfo::classifyType(QualType Ty, unsigned SizeLimit) const {
return ABIArgInfo::getIgnore();
uint64_t Size = getContext().getTypeSize(Ty);
+ unsigned Alignment = getContext().getTypeAlign(Ty);
// Anything too big to fit in registers is passed with an explicit indirect
// pointer / sret pointer.
@@ -275,10 +276,14 @@ SparcV9ABIInfo::classifyType(QualType Ty, unsigned SizeLimit) const {
// Try to use the original type for coercion.
llvm::Type *CoerceTy = CB.isUsableType(StrTy) ? StrTy : CB.getType();
+ // We use a pair of i64 for 9-16 byte aggregate with 8 byte alignment.
+ // For 9-16 byte aggregates with 16 byte alignment, we use i128.
+ llvm::Type *WideTy = llvm::Type::getIntNTy(getVMContext(), 128);
+ bool UseI128 = (Size > 64) && (Size <= 128) && (Alignment == 128);
+
if (CB.InReg)
- return ABIArgInfo::getDirectInReg(CoerceTy);
- else
- return ABIArgInfo::getDirect(CoerceTy);
+ return ABIArgInfo::getDirectInReg(UseI128 ? WideTy : CoerceTy);
+ return ABIArgInfo::getDirect(UseI128 ? WideTy : CoerceTy);
}
RValue SparcV9ABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
diff --git a/clang/test/CodeGen/sparcv9-abi.c b/clang/test/CodeGen/sparcv9-abi.c
index 5a3d64fd37889..9819d07425274 100644
--- a/clang/test/CodeGen/sparcv9-abi.c
+++ b/clang/test/CodeGen/sparcv9-abi.c
@@ -25,12 +25,21 @@ long double f_ld(long double x) { return x; }
struct empty {};
struct emptyarr { struct empty a[10]; };
+// 16-byte structs with 16-byte alignment gets passed as if i128.
+struct align16 { _Alignas(16) int x; };
+
// CHECK-LABEL: define{{.*}} i64 @f_empty(i64 %x.coerce)
struct empty f_empty(struct empty x) { return x; }
// CHECK-LABEL: define{{.*}} i64 @f_emptyarr(i64 %x.coerce)
struct empty f_emptyarr(struct emptyarr x) { return x.a[0]; }
+// CHECK-LABEL: define{{.*}} void @f_aligncaller(i128 %a.coerce)
+void f_aligncallee(int pad, struct align16 a);
+void f_aligncaller(struct align16 a) {
+ f_aligncallee(0, a);
+}
+
// CHECK-LABEL: define{{.*}} i64 @f_emptyvar(i32 noundef zeroext %count, ...)
long f_emptyvar(unsigned count, ...) {
long ret;
diff --git a/llvm/lib/Target/Sparc/SparcISelLowering.cpp b/llvm/lib/Target/Sparc/SparcISelLowering.cpp
index d01218f573dc2..e5ed9d267afed 100644
--- a/llvm/lib/Target/Sparc/SparcISelLowering.cpp
+++ b/llvm/lib/Target/Sparc/SparcISelLowering.cpp
@@ -115,7 +115,8 @@ static bool Analyze_CC_Sparc64_Full(bool IsReturn, unsigned &ValNo, MVT &ValVT,
// Stack space is allocated for all arguments starting from [%fp+BIAS+128].
unsigned size = (LocVT == MVT::f128) ? 16 : 8;
- Align alignment = (LocVT == MVT::f128) ? Align(16) : Align(8);
+ Align alignment =
+ (LocVT == MVT::f128 || ArgFlags.isSplit()) ? Align(16) : Align(8);
unsigned Offset = State.AllocateStack(size, alignment);
unsigned Reg = 0;
diff --git a/llvm/test/CodeGen/SPARC/64abi.ll b/llvm/test/CodeGen/SPARC/64abi.ll
index 6485a7f13e8d5..dc8c9af4a5185 100644
--- a/llvm/test/CodeGen/SPARC/64abi.ll
+++ b/llvm/test/CodeGen/SPARC/64abi.ll
@@ -473,8 +473,8 @@ declare i64 @receive_fp128(i64 %a, ...)
; HARD-DAG: ldx [%sp+[[Offset0]]], %o2
; HARD-DAG: ldx [%sp+[[Offset1]]], %o3
; SOFT-DAG: mov %i0, %o0
-; SOFT-DAG: mov %i1, %o1
; SOFT-DAG: mov %i2, %o2
+; SOFT-DAG: mov %i3, %o3
; CHECK: call receive_fp128
define i64 @test_fp128_variable_args(i64 %a, fp128 %b) {
entry:
@@ -482,6 +482,19 @@ entry:
ret i64 %0
}
+declare i64 @receive_i128(i64 %a, i128 %b)
+
+; CHECK-LABEL: test_i128_args:
+; CHECK: mov %i3, %o3
+; CHECK: mov %i2, %o2
+; CHECK: mov %i0, %o0
+; CHECK: call receive_i128
+define i64 @test_i128_args(i64 %a, i128 %b) {
+entry:
+ %0 = call i64 @receive_i128(i64 %a, i128 %b)
+ ret i64 %0
+}
+
; CHECK-LABEL: test_call_libfunc:
; HARD: st %f1, [%fp+[[Offset0:[0-9]+]]]
; HARD: fmovs %f3, %f1
|
| // We use a pair of i64 for 9-16 byte aggregate with 8 byte alignment. | ||
| // For 9-16 byte aggregates with 16 byte alignment, we use i128. | ||
| llvm::Type *WideTy = llvm::Type::getIntNTy(getVMContext(), 128); | ||
| bool UseI128 = (Size > 64) && (Size <= 128) && (Alignment == 128); |
There was a problem hiding this comment.
What happens if the size is bigger than 16? Not sure how much it matters, since SizeLimit is 128 for arguments, but we should have some test coverage for return types.
There was a problem hiding this comment.
Structs 17-32 bytes are passed indirectly, but returned in registers; structs over 32 bytes are also returned indirectly.
Structures or unions larger than sixteen bytes are copied by the caller and passed indirectly; the caller will pass the address of a correctly aligned structure value.
Structure and union return types up to thirty-two bytes in size are returned in registers. [...] For types with a larger size the caller allocates an area large enough and aligned properly to hold the return value, and passes a pointer to that area as an implicit first argument (of type pointer-to-data) to the callee.
I think for that case it's already tested in sparcv9-abi.c as the struct medium and struct large cases?
There was a problem hiding this comment.
Please also add coverage for a "medium" struct with 128-bit alignment.
There was a problem hiding this comment.
Done, as medium_aligned~
12 tasks
|
Thanks for working on this. This PR correctly fixes the example given in #144709. However, this PR currently fails to correctly pass overaligned structs containing floating point fields. Consider the following example: struct Struct {
_Alignas(16) long x;
double y;
};
double f(long a, struct Struct b) {
return b.y;
}GCC will correctly pass This PR also breaks the ABI of struct Struct {
long double x;
};
long double f(long a, struct Struct b) {
return b.x;
}GCC and current Clang will pass |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This coerces 9 to 16-byte C structs that are 16-byte aligned as an i128 in LLVM IR. Additionally, since i128 values will be lowered as split i64 pairs in the backend, correctly set the alignment of such arguments as 16 bytes.
This should make clang compliant with the ABI specification and fix #144709.