[clang] ceb21fa - [ARM] Fix how size-0 bitfields affect homogeneous aggregates.
Simon Tatham via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 10 03:27:05 PDT 2022
Author: Simon Tatham
Date: 2022-06-10T11:27:24+01:00
New Revision: ceb21fa4e49ddc8478371b41250f206082c5c67e
URL: https://github.com/llvm/llvm-project/commit/ceb21fa4e49ddc8478371b41250f206082c5c67e
DIFF: https://github.com/llvm/llvm-project/commit/ceb21fa4e49ddc8478371b41250f206082c5c67e.diff
LOG: [ARM] Fix how size-0 bitfields affect homogeneous aggregates.
By both AAPCS32 and AAPCS64, the test for whether an aggregate
qualifies as homogeneous (either HFA or HVA) is based on the data
layout alone. So any logical member of the structure that does not
affect the data layout also should not affect homogeneity. In
particular, an empty bitfield ('int : 0') should make no difference.
In fact, clang considered it to make a difference in C but not in C++,
and justified that policy as compatible with gcc. But that's
considered a bug in gcc as well (at least for Arm targets), and it's
fixed in gcc 12.1.
This fix mimics gcc's: zero-sized bitfields are now ignored in all
languages for the Arm (32- and 64-bit) ABIs. But I've left the
previous behaviour unchanged in other ABIs, by means of adding an
ABIInfo::isZeroLengthBitfieldPermittedInHomogeneousAggregate query
method which the Arm subclasses override.
Reviewed By: lenary
Differential Revision: https://reviews.llvm.org/D127197
Added:
clang/test/CodeGen/homogeneous-aggregates.c
Modified:
clang/lib/CodeGen/ABIInfo.h
clang/lib/CodeGen/TargetInfo.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/ABIInfo.h b/clang/lib/CodeGen/ABIInfo.h
index 0d12183055e18..6214148adab93 100644
--- a/clang/lib/CodeGen/ABIInfo.h
+++ b/clang/lib/CodeGen/ABIInfo.h
@@ -100,6 +100,7 @@ namespace swiftcall {
virtual bool isHomogeneousAggregateSmallEnough(const Type *Base,
uint64_t Members) const;
+ virtual bool isZeroLengthBitfieldPermittedInHomogeneousAggregate() const;
bool isHomogeneousAggregate(QualType Ty, const Type *&Base,
uint64_t &Members) const;
diff --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp
index 5e97a946782ca..d481d1c2857bc 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -240,6 +240,11 @@ bool ABIInfo::isHomogeneousAggregateSmallEnough(const Type *Base,
return false;
}
+bool ABIInfo::isZeroLengthBitfieldPermittedInHomogeneousAggregate() const {
+ // For compatibility with GCC, ignore empty bitfields in C++ mode.
+ return getContext().getLangOpts().CPlusPlus;
+}
+
LLVM_DUMP_METHOD void ABIArgInfo::dump() const {
raw_ostream &OS = llvm::errs();
OS << "(ABIArgInfo Kind=";
@@ -5213,8 +5218,7 @@ bool ABIInfo::isHomogeneousAggregate(QualType Ty, const Type *&Base,
if (isEmptyRecord(getContext(), FT, true))
continue;
- // For compatibility with GCC, ignore empty bitfields in C++ mode.
- if (getContext().getLangOpts().CPlusPlus &&
+ if (isZeroLengthBitfieldPermittedInHomogeneousAggregate() &&
FD->isZeroLengthBitField(getContext()))
continue;
@@ -5511,6 +5515,7 @@ class AArch64ABIInfo : public SwiftABIInfo {
bool isHomogeneousAggregateBaseType(QualType Ty) const override;
bool isHomogeneousAggregateSmallEnough(const Type *Ty,
uint64_t Members) const override;
+ bool isZeroLengthBitfieldPermittedInHomogeneousAggregate() const override;
bool isIllegalVectorType(QualType Ty) const;
@@ -5970,6 +5975,16 @@ bool AArch64ABIInfo::isHomogeneousAggregateSmallEnough(const Type *Base,
return Members <= 4;
}
+bool AArch64ABIInfo::isZeroLengthBitfieldPermittedInHomogeneousAggregate()
+ const {
+ // AAPCS64 says that the rule for whether something is a homogeneous
+ // aggregate is applied to the output of the data layout decision. So
+ // anything that doesn't affect the data layout also does not affect
+ // homogeneity. In particular, zero-length bitfields don't stop a struct
+ // being homogeneous.
+ return true;
+}
+
Address AArch64ABIInfo::EmitAAPCSVAArg(Address VAListAddr, QualType Ty,
CodeGenFunction &CGF) const {
ABIArgInfo AI = classifyArgumentType(Ty, /*IsVariadic=*/true,
@@ -6339,6 +6354,7 @@ class ARMABIInfo : public SwiftABIInfo {
bool isHomogeneousAggregateBaseType(QualType Ty) const override;
bool isHomogeneousAggregateSmallEnough(const Type *Ty,
uint64_t Members) const override;
+ bool isZeroLengthBitfieldPermittedInHomogeneousAggregate() const override;
bool isEffectivelyAAPCS_VFP(unsigned callConvention, bool acceptHalf) const;
@@ -7002,6 +7018,15 @@ bool ARMABIInfo::isHomogeneousAggregateSmallEnough(const Type *Base,
return Members <= 4;
}
+bool ARMABIInfo::isZeroLengthBitfieldPermittedInHomogeneousAggregate() const {
+ // AAPCS32 says that the rule for whether something is a homogeneous
+ // aggregate is applied to the output of the data layout decision. So
+ // anything that doesn't affect the data layout also does not affect
+ // homogeneity. In particular, zero-length bitfields don't stop a struct
+ // being homogeneous.
+ return true;
+}
+
bool ARMABIInfo::isEffectivelyAAPCS_VFP(unsigned callConvention,
bool acceptHalf) const {
// Give precedence to user-specified calling conventions.
diff --git a/clang/test/CodeGen/homogeneous-aggregates.c b/clang/test/CodeGen/homogeneous-aggregates.c
new file mode 100644
index 0000000000000..bf3751b10ba65
--- /dev/null
+++ b/clang/test/CodeGen/homogeneous-aggregates.c
@@ -0,0 +1,95 @@
+// REQUIRES: arm-registered-target,aarch64-registered-target,powerpc-registered-target
+// RUN: %clang_cc1 -triple thumbv7-none-none -mfloat-abi hard -x c -emit-llvm -o - %s | FileCheck %s --check-prefix=AAPCS
+// RUN: %clang_cc1 -triple thumbv7-none-none -mfloat-abi hard -x c++ -emit-llvm -o - %s | FileCheck %s --check-prefix=AAPCS
+// RUN: %clang_cc1 -triple thumbv7-none-none -mfloat-abi hard -x c++ -DEXTERN_C -emit-llvm -o - %s | FileCheck %s --check-prefix=AAPCS
+// RUN: %clang_cc1 -triple aarch64-none-none -mfloat-abi hard -x c -emit-llvm -o - %s | FileCheck %s --check-prefix=AAPCS
+// RUN: %clang_cc1 -triple aarch64-none-none -mfloat-abi hard -x c++ -emit-llvm -o - %s | FileCheck %s --check-prefix=AAPCS
+// RUN: %clang_cc1 -triple aarch64-none-none -mfloat-abi hard -x c++ -DEXTERN_C -emit-llvm -o - %s | FileCheck %s --check-prefix=AAPCS
+// RUN: %clang_cc1 -triple powerpc64le-none-none -mfloat-abi hard -x c -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC --check-prefix=PPC-C
+// RUN: %clang_cc1 -triple powerpc64le-none-none -mfloat-abi hard -x c++ -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC --check-prefix=PPC-CXX
+// RUN: %clang_cc1 -triple powerpc64le-none-none -mfloat-abi hard -x c++ -DEXTERN_C -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC --check-prefix=PPC-CXX
+
+// The aim here is to test whether each of these structure types is
+// regarded as a homogeneous aggregate of a single kind of
+// floating-point item, because in all of these ABIs, that changes the
+// calling convention.
+//
+// We expect that 'Floats' and 'Doubles' are homogeneous, and 'Mixed'
+// is not. But the next two structures, with separating zero-size
+// bitfields, are more interesting.
+//
+// For the Arm architecture, AAPCS says that the homogeneity rule is
+// applied _after_ data layout is completed, so that it's unaffected
+// by anything that was completely discarded during data layout. So we
+// expect that FloatsBF and DoublesBF still count as homogeneous.
+//
+// But on PowerPC, it depends on whether the source language is C or
+// C++, because that's consistent with the decisions gcc makes.
+
+struct Floats {
+ float a;
+ float b;
+};
+
+struct Doubles {
+ double a;
+ double b;
+};
+
+struct Mixed {
+ double a;
+ float b;
+};
+
+struct FloatsBF {
+ float a;
+ int : 0;
+ float b;
+};
+
+struct DoublesBF {
+ double a;
+ int : 0;
+ double b;
+};
+
+// In C++ mode, we test both with and without extern "C", to ensure
+// that doesn't make a
diff erence.
+#ifdef EXTERN_C
+#define LINKAGE extern "C"
+#else
+#define LINKAGE
+#endif
+
+// For Arm backends, the IR emitted for the homogeneous-aggregate
+// return convention uses the actual structure type, so that
+// HandleFloats returns a %struct.Floats, and so on. To check that
+// 'Mixed' is not treated as homogeneous, it's enough to check that
+// its return type is _not_ %struct.Mixed. (The fallback handling
+// varies between AArch32 and AArch64.)
+//
+// For PowerPC, homogeneous structure types are lowered to an IR array
+// types like [2 x float], and the non-homogeneous Mixed is lowered to
+// a pair of i64.
+
+// AAPCS: define{{.*}} %struct.Floats @{{.*HandleFloats.*}}
+// PPC: define{{.*}} [2 x float] @{{.*HandleFloats.*}}
+LINKAGE struct Floats HandleFloats(struct Floats x) { return x; }
+
+// AAPCS: define{{.*}} %struct.Doubles @{{.*HandleDoubles.*}}
+// PPC: define{{.*}} [2 x double] @{{.*HandleDoubles.*}}
+LINKAGE struct Doubles HandleDoubles(struct Doubles x) { return x; }
+
+// AAPCS-NOT: define{{.*}} %struct.Mixed @{{.*HandleMixed.*}}
+// PPC: define{{.*}} { i64, i64 } @{{.*HandleMixed.*}}
+LINKAGE struct Mixed HandleMixed(struct Mixed x) { return x; }
+
+// AAPCS: define{{.*}} %struct.FloatsBF @{{.*HandleFloatsBF.*}}
+// PPC-C-NOT: define{{.*}} [2 x float] @{{.*HandleFloatsBF.*}}
+// PPC-CXX: define{{.*}} [2 x float] @{{.*HandleFloatsBF.*}}
+LINKAGE struct FloatsBF HandleFloatsBF(struct FloatsBF x) { return x; }
+
+// AAPCS: define{{.*}} %struct.DoublesBF @{{.*HandleDoublesBF.*}}
+// PPC-C-NOT: define{{.*}} [2 x double] @{{.*HandleDoublesBF.*}}
+// PPC-CXX: define{{.*}} [2 x double] @{{.*HandleDoublesBF.*}}
+LINKAGE struct DoublesBF HandleDoublesBF(struct DoublesBF x) { return x; }
More information about the cfe-commits
mailing list