simon_tatham created this revision.
simon_tatham added reviewers: asl, rsmith, lenary, john.brawn.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
simon_tatham requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127197

Files:
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/homogeneous-aggregates.c

Index: clang/test/CodeGen/homogeneous-aggregates.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/homogeneous-aggregates.c
@@ -0,0 +1,84 @@
+// 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 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 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
+
+// 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;
+};
+
+// 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.*}}
+struct Floats HandleFloats(struct Floats x) { return x; }
+
+// AAPCS: define{{.*}} %struct.Doubles @{{.*HandleDoubles.*}}
+// PPC: define{{.*}} [2 x double] @{{.*HandleDoubles.*}}
+struct Doubles HandleDoubles(struct Doubles x) { return x; }
+
+// AAPCS-NOT: define{{.*}} %struct.Mixed @{{.*HandleMixed.*}}
+// PPC: define{{.*}} { i64, i64 } @{{.*HandleMixed.*}}
+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.*}}
+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.*}}
+struct DoublesBF HandleDoublesBF(struct DoublesBF x) { return x; }
Index: clang/lib/CodeGen/TargetInfo.cpp
===================================================================
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -240,6 +240,11 @@
   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 @@
       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 @@
   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 @@
   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 @@
   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 @@
   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.
Index: clang/lib/CodeGen/ABIInfo.h
===================================================================
--- clang/lib/CodeGen/ABIInfo.h
+++ clang/lib/CodeGen/ABIInfo.h
@@ -100,6 +100,7 @@
 
     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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to