llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-powerpc
Author: Matt Turner (mattst88)
<details>
<summary>Changes</summary>
The 64-bit PowerPC ELFv1 ABI (section 3.2.3) passes "one member aggregates
passed by value containing a floating point value" in FPRs, and an aggregate
containing one vector value in a VR. Section 3.1.6 defines aggregates as
structures and arrays and lists unions separately, so a union is never
unwrapped this way.
Clang used isSingleElementStruct, which also accepts unions, so it passed
`union { float a; }`, `struct { union { float a; } u; }` and
`union { struct { float a; } s; }` in f1 where GCC passes them in r3. The same
happened for double, long double and 128-bit vector members.
Add an AllowUnions parameter to isSingleElementStruct and refuse unions on
ELFv1. ELFv2 is unchanged: it defines homogeneous aggregates to include unions,
and clang already agrees with GCC there.
This is an ABI break for existing clang-compiled ELFv1 code that passes such
unions; -fclang-abi-compat=23 restores the previous behavior. rustc has the
same bug (rust-lang/rust#<!-- -->162011).
Assisted-by: Claude Code
---
Full diff: https://github.com/llvm/llvm-project/pull/223481.diff
5 Files Affected:
- (modified) clang/docs/ReleaseNotes.md (+6)
- (modified) clang/lib/CodeGen/ABIInfoImpl.cpp (+5-4)
- (modified) clang/lib/CodeGen/ABIInfoImpl.h (+4-2)
- (modified) clang/lib/CodeGen/Targets/PPC.cpp (+23-17)
- (added) clang/test/CodeGen/PowerPC/ppc64-union-onefloat.c (+63)
``````````diff
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index 043a0ddae2a6cd..783ad520685db2 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -97,6 +97,12 @@ features cannot lower the translation-unit ABI level;
- On MIPS N32/N64, an `__int128` now correctly start in an even-numbered
register
or 16-byte aligned stack slot, matching GCC.
+- On 64-bit PowerPC ELFv1, a union argument that wraps a single floating-point
+ or vector value, or a struct that wraps such a union, is now passed in
+ general-purpose registers rather than in a floating-point or vector register,
+ matching GCC. ELFv2 is unchanged. `-fclang-abi-compat=23` restores the
+ previous behavior.
+
- Except on PlayStation, on x86-64 System V a non-zero-width unnamed bit-field
now classifies the eightbytes it occupies as INTEGER, like a named bit-field,
matching GCC. Aggregates where this changes the classification may be passed
diff --git a/clang/lib/CodeGen/ABIInfoImpl.cpp
b/clang/lib/CodeGen/ABIInfoImpl.cpp
index 887a645a4783a7..c4c4caf8312827 100644
--- a/clang/lib/CodeGen/ABIInfoImpl.cpp
+++ b/clang/lib/CodeGen/ABIInfoImpl.cpp
@@ -335,9 +335,10 @@ bool CodeGen::isEmptyRecordForLayout(const ASTContext
&Context, QualType T) {
return true;
}
-const Type *CodeGen::isSingleElementStruct(QualType T, ASTContext &Context) {
+const Type *CodeGen::isSingleElementStruct(QualType T, ASTContext &Context,
+ bool AllowUnions) {
const auto *RD = T->getAsRecordDecl();
- if (!RD)
+ if (!RD || (!AllowUnions && RD->isUnion()))
return nullptr;
if (RD->hasFlexibleArrayMember())
@@ -358,7 +359,7 @@ const Type *CodeGen::isSingleElementStruct(QualType T,
ASTContext &Context) {
// If this is non-empty and not a single element struct, the composite
// cannot be a single element struct.
- Found = isSingleElementStruct(I.getType(), Context);
+ Found = isSingleElementStruct(I.getType(), Context, AllowUnions);
if (!Found)
return nullptr;
}
@@ -387,7 +388,7 @@ const Type *CodeGen::isSingleElementStruct(QualType T,
ASTContext &Context) {
if (!isAggregateTypeForABI(FT)) {
Found = FT.getTypePtr();
} else {
- Found = isSingleElementStruct(FT, Context);
+ Found = isSingleElementStruct(FT, Context, AllowUnions);
if (!Found)
return nullptr;
}
diff --git a/clang/lib/CodeGen/ABIInfoImpl.h b/clang/lib/CodeGen/ABIInfoImpl.h
index d9d79c6a55ddb1..2014b6016bc5a4 100644
--- a/clang/lib/CodeGen/ABIInfoImpl.h
+++ b/clang/lib/CodeGen/ABIInfoImpl.h
@@ -134,11 +134,13 @@ bool isEmptyRecordForLayout(const ASTContext &Context,
QualType T);
/// element struct", i.e. it has exactly one non-empty field or
/// exactly one field which is itself a single element
/// struct. Structures with flexible array members are never
-/// considered single element structs.
+/// considered single element structs. If \p AllowUnions is false, neither T
+/// nor any record it contains may be a union.
///
/// \return The field declaration for the single non-empty field, if
/// it exists.
-const Type *isSingleElementStruct(QualType T, ASTContext &Context);
+const Type *isSingleElementStruct(QualType T, ASTContext &Context,
+ bool AllowUnions = true);
Address EmitVAArgInstr(CodeGenFunction &CGF, Address VAListAddr, QualType Ty,
const ABIArgInfo &AI);
diff --git a/clang/lib/CodeGen/Targets/PPC.cpp
b/clang/lib/CodeGen/Targets/PPC.cpp
index a87f8410636500..dec907602186eb 100644
--- a/clang/lib/CodeGen/Targets/PPC.cpp
+++ b/clang/lib/CodeGen/Targets/PPC.cpp
@@ -654,6 +654,7 @@ class PPC64_SVR4_ABIInfo : public ABIInfo {
bool isPromotableTypeForABI(QualType Ty) const;
CharUnits getParamTypeAlignment(QualType Ty) const;
+ const Type *getSingleElementFPOrVectorType(QualType Ty) const;
ABIArgInfo classifyReturnType(QualType RetTy) const;
ABIArgInfo classifyArgumentType(QualType Ty) const;
@@ -675,15 +676,9 @@ class PPC64_SVR4_ABIInfo : public ABIInfo {
// We rely on the default argument classification for the most part.
// One exception: An aggregate containing a single floating-point
// or vector item must be passed in a register if one is available.
- const Type *T = isSingleElementStruct(I.type, getContext());
- if (T) {
- const BuiltinType *BT = T->getAs<BuiltinType>();
- if ((T->isVectorType() && getContext().getTypeSize(T) == 128) ||
- (BT && BT->isFloatingPoint())) {
- QualType QT(T, 0);
- I.info = ABIArgInfo::getDirectInReg(CGT.ConvertType(QT));
- continue;
- }
+ if (const Type *T = getSingleElementFPOrVectorType(I.type)) {
+ I.info = ABIArgInfo::getDirectInReg(CGT.ConvertType(QualType(T, 0)));
+ continue;
}
I.info = classifyArgumentType(I.type);
}
@@ -783,14 +778,7 @@ CharUnits
PPC64_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
// For single-element float/vector structs, we consider the whole type
// to have the same alignment requirements as its single element.
- const Type *AlignAsType = nullptr;
- const Type *EltType = isSingleElementStruct(Ty, getContext());
- if (EltType) {
- const BuiltinType *BT = EltType->getAs<BuiltinType>();
- if ((EltType->isVectorType() && getContext().getTypeSize(EltType) == 128)
||
- (BT && BT->isFloatingPoint()))
- AlignAsType = EltType;
- }
+ const Type *AlignAsType = getSingleElementFPOrVectorType(Ty);
// Likewise for ELFv2 homogeneous aggregates.
const Type *Base = nullptr;
@@ -815,6 +803,24 @@ CharUnits
PPC64_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
return CharUnits::fromQuantity(8);
}
+/// If Ty is a one-member aggregate wrapping a floating-point or 128-bit vector
+/// type, return that type. ELFv1 only unwraps structs and arrays; GCC passes
+/// unions in GPRs.
+const Type *
+PPC64_SVR4_ABIInfo::getSingleElementFPOrVectorType(QualType Ty) const {
+ bool AllowUnions =
+ Kind != PPC64_SVR4_ABIKind::ELFv1 ||
+
getContext().getLangOpts().isCompatibleWith(LangOptions::ClangABI::Ver23);
+ const Type *EltType = isSingleElementStruct(Ty, getContext(), AllowUnions);
+ if (!EltType)
+ return nullptr;
+ const BuiltinType *BT = EltType->getAs<BuiltinType>();
+ if ((EltType->isVectorType() && getContext().getTypeSize(EltType) == 128) ||
+ (BT && BT->isFloatingPoint()))
+ return EltType;
+ return nullptr;
+}
+
bool PPC64_SVR4_ABIInfo::isHomogeneousAggregateBaseType(QualType Ty) const {
// Homogeneous aggregates for ELFv2 must have base types of float,
// double, long double, or 128-bit vectors.
diff --git a/clang/test/CodeGen/PowerPC/ppc64-union-onefloat.c
b/clang/test/CodeGen/PowerPC/ppc64-union-onefloat.c
new file mode 100644
index 00000000000000..a6893f365ced8d
--- /dev/null
+++ b/clang/test/CodeGen/PowerPC/ppc64-union-onefloat.c
@@ -0,0 +1,63 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=ALL,GPR
+// RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -fclang-abi-compat=23 \
+// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefixes=ALL,UNWRAP
+// RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -target-abi elfv2 \
+// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefixes=ALL,UNWRAP
+// RUN: %clang_cc1 -triple powerpc64le-unknown-linux-gnu -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=ALL,UNWRAP
+
+// ELFv1 does not unwrap unions for FPR/VR passing; ELFv2 does.
+
+typedef float v4sf __attribute__((vector_size(16)));
+
+typedef struct { float a; } S1;
+typedef union { float a; } U1;
+typedef union { double a; } UD;
+typedef union { long double a; } ULD;
+typedef union { v4sf v; } UV;
+typedef struct { U1 u; } SU1;
+typedef union { S1 s; } US1;
+typedef struct { U1 a[1]; } SAU1;
+
+// ALL-LABEL: define{{.*}} void @s1(float inreg %
+void s1(S1 x) {}
+
+// GPR-LABEL: define{{.*}} void @u1(i32 %
+// UNWRAP-LABEL: define{{.*}} void @u1(float inreg %
+void u1(U1 x) {}
+
+// GPR-LABEL: define{{.*}} void @ud(i64 %
+// UNWRAP-LABEL: define{{.*}} void @ud(double inreg %
+void ud(UD x) {}
+
+// GPR-LABEL: define{{.*}} void @uld([1 x i128] %
+// UNWRAP-LABEL: define{{.*}} void @uld(ppc_fp128 inreg %
+void uld(ULD x) {}
+
+// GPR-LABEL: define{{.*}} void @uv([1 x i128] %
+// UNWRAP-LABEL: define{{.*}} void @uv(<4 x float> inreg %
+void uv(UV x) {}
+
+// GPR-LABEL: define{{.*}} void @su1(i32 %
+// UNWRAP-LABEL: define{{.*}} void @su1(float inreg %
+void su1(SU1 x) {}
+
+// GPR-LABEL: define{{.*}} void @us1(i32 %
+// UNWRAP-LABEL: define{{.*}} void @us1(float inreg %
+void us1(US1 x) {}
+
+// GPR-LABEL: define{{.*}} void @sau1(i32 %
+// UNWRAP-LABEL: define{{.*}} void @sau1(float inreg %
+void sau1(SAU1 x) {}
+
+// A union of a vector is still quadword-aligned in the parameter save area.
+// ALL-LABEL: define{{.*}} void @uva(
+// ALL: call ptr @llvm.ptrmask.p0.i64(ptr %{{.*}}, i64 -16)
+void uva(int n, ...) {
+ __builtin_va_list ap;
+ __builtin_va_start(ap, n);
+ UV x = __builtin_va_arg(ap, UV);
+ __builtin_va_end(ap);
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/223481
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
