Skip to content

Conversation

DanilaZhebryakov
Copy link
Contributor

soft float ABI selection was not taking effect on little-endian powerPC with embedded vectors (e.g. e500v2) leading to errors.
(embedded vectors use "extended" GPRs to store floating-point values, and this caused issues with variadic arguments assuming dedicated floating-point registers with hard-float ABI)

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Aug 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 21, 2025

@llvm/pr-subscribers-clang-codegen

Author: None (DanilaZhebryakov)

Changes

soft float ABI selection was not taking effect on little-endian powerPC with embedded vectors (e.g. e500v2) leading to errors.
(embedded vectors use "extended" GPRs to store floating-point values, and this caused issues with variadic arguments assuming dedicated floating-point registers with hard-float ABI)


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

1 Files Affected:

  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+2-1)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 677d8bc82cb0a..8b85b93b5d568 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -188,7 +188,8 @@ createTargetCodeGenInfo(CodeGenModule &CGM) {
     return createPPC32TargetCodeGenInfo(CGM, IsSoftFloat);
   }
   case llvm::Triple::ppcle: {
-    bool IsSoftFloat = CodeGenOpts.FloatABI == "soft";
+    bool IsSoftFloat =
+        CodeGenOpts.FloatABI == "soft" || Target.hasFeature("spe");
     return createPPC32TargetCodeGenInfo(CGM, IsSoftFloat);
   }
   case llvm::Triple::ppc64:

@llvmbot
Copy link
Member

llvmbot commented Aug 21, 2025

@llvm/pr-subscribers-clang

Author: None (DanilaZhebryakov)

Changes

soft float ABI selection was not taking effect on little-endian powerPC with embedded vectors (e.g. e500v2) leading to errors.
(embedded vectors use "extended" GPRs to store floating-point values, and this caused issues with variadic arguments assuming dedicated floating-point registers with hard-float ABI)


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

1 Files Affected:

  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+2-1)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 677d8bc82cb0a..8b85b93b5d568 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -188,7 +188,8 @@ createTargetCodeGenInfo(CodeGenModule &CGM) {
     return createPPC32TargetCodeGenInfo(CGM, IsSoftFloat);
   }
   case llvm::Triple::ppcle: {
-    bool IsSoftFloat = CodeGenOpts.FloatABI == "soft";
+    bool IsSoftFloat =
+        CodeGenOpts.FloatABI == "soft" || Target.hasFeature("spe");
     return createPPC32TargetCodeGenInfo(CGM, IsSoftFloat);
   }
   case llvm::Triple::ppc64:

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

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

Please add a regression test.

I'm not sure the Target.hasFeature("spe") is really what we want to be doing here. It looks like spe is actually a mutually incompatible ABI, not just a CPU feature, and we should model it as such. But it matches big-endian ppc, so I won't block this patch on fixing that.

@DanilaZhebryakov
Copy link
Contributor Author

Added test.

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

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

Can you unify the test with clang/test/CodeGen/PowerPC/ppc-sfvarargs.c, since it's testing basically the same thing?

@DanilaZhebryakov
Copy link
Contributor Author

Done. It appears that existing checks in clang/test/CodeGen/PowerPC/ppc-sfvarargs.c are sufficient to check that correct ABI is used.

@DanilaZhebryakov
Copy link
Contributor Author

also oops, some weird changes are shown due to accidental merge rather than rebase.

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

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

LGTM

@efriedma-quic
Copy link
Collaborator

We have automation that's supposed to trigger, but since it isn't:

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Developer Policy and LLVM Discourse for more information.

@DanilaZhebryakov
Copy link
Contributor Author

Please turn off Keep my email addresses private setting in your account.

Done.

If everything is ok, please merge it.
Оtherwise, is there anything more I have to do for the email setting to take effect?

@efriedma-quic efriedma-quic merged commit 324511b into llvm:main Sep 18, 2025
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:PowerPC 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.

4 participants