Skip to content

[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

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

DavidTruby
Copy link
Member

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

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
@DavidTruby DavidTruby added this to the LLVM 20.X Release milestone Feb 19, 2025
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:codegen labels Feb 19, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 19, 2025

@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-flang-codegen

Author: David Truby (DavidTruby)

Changes

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


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

2 Files Affected:

  • (modified) flang/lib/Optimizer/CodeGen/Target.cpp (+7)
  • (modified) flang/test/Fir/struct-passing-aarch64-byval.fir (+14)
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}>)

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@tstellar tstellar merged commit 449f84f into llvm:main Feb 21, 2025
12 checks passed
@pawosm-arm
Copy link
Contributor

This really fixes the issue with CP2K, how can we make sure it is going to be cherry-picked to the release 20.x?

@DavidTruby
Copy link
Member Author

DavidTruby commented Feb 24, 2025

/cherry-pick 449f84f

@llvmbot
Copy link
Member

llvmbot commented Feb 24, 2025

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

@DavidTruby
Copy link
Member Author

I guess I need to rebase this manually and create a PR.

@llvmbot
Copy link
Member

llvmbot commented Feb 24, 2025

/pull-request #128518

swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 25, 2025
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:codegen flang:fir-hlfir flang Flang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

[flang] CP2K fails to compile due to assertion 'result && "Fell off the end of a type-switch"'
5 participants