-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[flang] fix AArch64 PCS for struct following pointer #127802
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
Conversation
Pointers are already handled as taking up a register in the ABI handling, but the handling for structs was not taking this into account. This patch changes the struct handling to acknowledge that pointer arguments take up an integer register. Fixes llvm#123075
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-codegen Author: David Truby (DavidTruby) ChangesPointers are already handled as taking up a register in the ABI handling, but the handling for structs was not taking this into account. This patch changes the struct handling to acknowledge that pointer arguments take up an integer register. Fixes #123075 Full diff: https://github.com/llvm/llvm-project/pull/127802.diff 2 Files Affected:
diff --git a/flang/lib/Optimizer/CodeGen/Target.cpp b/flang/lib/Optimizer/CodeGen/Target.cpp
index 1bc673bb34e32..2a1eb0bc33f5c 100644
--- a/flang/lib/Optimizer/CodeGen/Target.cpp
+++ b/flang/lib/Optimizer/CodeGen/Target.cpp
@@ -930,6 +930,13 @@ struct TargetAArch64 : public GenericTarget<TargetAArch64> {
.Case<fir::VectorType>([&](auto) {
TODO(loc, "passing vector argument to C by value is not supported");
return NRegs{};
+ })
+ .Default([&](auto ty) {
+ if (fir::conformsWithPassByRef(ty))
+ return NRegs{1, false}; // Pointers take 1 integer register
+ TODO(loc, "unsupported component type for BIND(C), VALUE derived "
+ "type argument");
+ return NRegs{};
});
}
diff --git a/flang/test/Fir/struct-passing-aarch64-byval.fir b/flang/test/Fir/struct-passing-aarch64-byval.fir
index 27143459dde2f..087efba393014 100644
--- a/flang/test/Fir/struct-passing-aarch64-byval.fir
+++ b/flang/test/Fir/struct-passing-aarch64-byval.fir
@@ -71,3 +71,17 @@ func.func private @too_many_hfa(!fir.type<hfa_max{i:f128,j:f128,k:f128,l:f128}>,
// CHECK-LABEL: func.func private @too_big(!fir.ref<!fir.type<too_big{i:!fir.array<5xi32>}>> {{{.*}}, llvm.byval = !fir.type<too_big{i:!fir.array<5xi32>}>})
func.func private @too_big(!fir.type<too_big{i:!fir.array<5xi32>}>)
+
+// CHECK-LABEL: func.func private @pointer_type(!fir.ref<i64>, !fir.array<1xi64>)
+func.func private @pointer_type(!fir.ref<i64>, !fir.type<pointer_type{i:i64}>)
+
+// CHECK-LABEL: func.func private @pointer_type_too_many_int(!fir.ref<i64>,
+// CHECK-SAME: !fir.array<2xi64>,
+// CHECK-SAME: !fir.array<2xi64>,
+// CHECK-SAME: !fir.array<2xi64>,
+// CHECK-SAME: !fir.ref<!fir.type<int_max{i:i64,j:i64}>> {{{.*}}, llvm.byval = !fir.type<int_max{i:i64,j:i64}>})
+func.func private @pointer_type_too_many_int(!fir.ref<i64>,
+ !fir.type<int_max{i:i64,j:i64}>,
+ !fir.type<int_max{i:i64,j:i64}>,
+ !fir.type<int_max{i:i64,j:i64}>,
+ !fir.type<int_max{i:i64,j:i64}>)
|
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, thanks!
This really fixes the issue with CP2K, how can we make sure it is going to be cherry-picked to the release 20.x? |
/cherry-pick 449f84f |
Failed to cherry-pick: 447f84f https://github.com/llvm/llvm-project/actions/runs/13500103851 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
I guess I need to rebase this manually and create a PR. |
/pull-request #128518 |
Pointers are already handled as taking up a register in the ABI handling, but the handling for structs was not taking this into account. This patch changes the struct handling to acknowledge that pointer arguments take up an integer register. Fixes llvm#123075 (cherry picked from commit 449f84f)
Pointers are already handled as taking up a register in the ABI handling, but the handling for structs was not taking this into account. This patch changes the struct handling to acknowledge that pointer arguments take up an integer register.
Fixes #123075