[ARM] Fix how size-0 bitfields affect homogeneous aggregates.

[simon_tatham]

Summary

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.

[simon_tatham]

Added tests with extern "C", at @lenary's (offline) suggestion.

[lenary]

LGTM.

I tested this patch in an end-to-end fashion to look at the assembly in affected cases, and it is doing the right thing.

clang/lib/CodeGen/ABIInfo.h

public:
     virtual CodeGen::Address EmitMSVAArg(CodeGen::CodeGenFunction &CGF,
                                          CodeGen::Address VAListAddr,
                                          QualType Ty) const;
 
     virtual bool isHomogeneousAggregateBaseType(QualType Ty) const;
 
     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;
 
     // Implement the Type::IsPromotableIntegerType for ABI specific needs. The
     // only difference is that this considers bit-precise integer types as well.
     bool isPromotableIntegerTypeForABI(QualType Ty) const;
 

clang/lib/CodeGen/TargetInfo.cpp

bool ABIInfo::isHomogeneousAggregateBaseType(QualType Ty) const {
   return false;
 }
 
 bool ABIInfo::isHomogeneousAggregateSmallEnough(const Type *Base,
                                                 uint64_t Members) const {
   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=";
   switch (TheKind) {
   case Direct:
     OS << "Direct Type=";
     if (llvm::Type *Ty = getCoerceToType())
       Ty->print(OS);
for (const auto *FD : RD->fields()) {
              getContext().getAsConstantArrayType(FT)) {
         if (AT->getSize().getZExtValue() == 0)
           return false;
         FT = AT->getElementType();
       }
       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;
 
       uint64_t FldMembers;
       if (!isHomogeneousAggregate(FD->getType(), Base, FldMembers))
         return false;
 
       Members = (RD->isUnion() ?
private:
 
   ABIArgInfo classifyReturnType(QualType RetTy, bool IsVariadic) const;
   ABIArgInfo classifyArgumentType(QualType RetTy, bool IsVariadic,
                                   unsigned CallingConvention) const;
   ABIArgInfo coerceIllegalVector(QualType Ty) const;
   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;
 
   void computeInfo(CGFunctionInfo &FI) const override {
     if (!::classifyReturnType(getCXXABI(), FI, *this))
       FI.getReturnInfo() =
           classifyReturnType(FI.getReturnType(), FI.isVariadic());
 
bool AArch64ABIInfo::isHomogeneousAggregateBaseType(QualType Ty) const {
   return false;
 }
 
 bool AArch64ABIInfo::isHomogeneousAggregateSmallEnough(const Type *Base,
                                                        uint64_t Members) const {
   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,
                                        CGF.CurFnInfo->getCallingConvention());
   bool IsIndirect = AI.isIndirect();
 
   llvm::Type *BaseTy = CGF.ConvertType(Ty);
   if (IsIndirect)
ABIArgInfo classifyHomogeneousAggregate(QualType Ty, const Type *Base,
                                           uint64_t Members) const;
   ABIArgInfo coerceIllegalVector(QualType Ty) const;
   bool isIllegalVectorType(QualType Ty) const;
   bool containsAnyFP16Vectors(QualType Ty) const;
 
   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;
 
   void computeInfo(CGFunctionInfo &FI) const override;
 
   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
                     QualType Ty) const override;
 
bool ARMABIInfo::isHomogeneousAggregateBaseType(QualType Ty) const {
   return false;
 }
 
 bool ARMABIInfo::isHomogeneousAggregateSmallEnough(const Type *Base,
                                                    uint64_t Members) const {
   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.
   if (callConvention != llvm::CallingConv::C)
     return (callConvention == llvm::CallingConv::ARM_AAPCS_VFP);
   else
     return (getABIKind() == AAPCS_VFP) ||
            (acceptHalf && (getABIKind() == AAPCS16_VFP));

clang/test/CodeGen/homogeneous-aggregates.c

+// 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 difference.
+#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; }