https://github.com/ostannard updated https://github.com/llvm/llvm-project/pull/126774
>From 223eb6f3153bce087306f54c398034ff7a64c1d1 Mon Sep 17 00:00:00 2001 From: Oliver Stannard <oliver.stann...@arm.com> Date: Tue, 11 Feb 2025 17:40:55 +0000 Subject: [PATCH 1/4] Add tests showing bugs --- clang/test/CodeGen/aapcs-align.cpp | 43 +++++++++++++++++++ clang/test/CodeGen/aapcs64-align.cpp | 64 ++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/clang/test/CodeGen/aapcs-align.cpp b/clang/test/CodeGen/aapcs-align.cpp index 4f393d9e6b7f3..01b5ea5a69fdc 100644 --- a/clang/test/CodeGen/aapcs-align.cpp +++ b/clang/test/CodeGen/aapcs-align.cpp @@ -6,6 +6,11 @@ extern "C" { +// CHECK: @sizeof_OverSizedBitfield ={{.*}} global i32 8 +// CHECK: @alignof_OverSizedBitfield ={{.*}} global i32 8 +// CHECK: @sizeof_VeryOverSizedBitfield ={{.*}} global i32 16 +// CHECK: @alignof_VeryOverSizedBitfield ={{.*}} global i32 8 + // Base case, nothing interesting. struct S { int x, y; @@ -138,4 +143,42 @@ void g6() { // CHECK: call void @f6m(i32 noundef 1, i32 noundef 2, i32 noundef 3, i32 noundef 4, i32 noundef 5, [4 x i32] [i32 6, i32 7, i32 0, i32 undef]) // CHECK: declare void @f6(i32 noundef, [4 x i32]) // CHECK: declare void @f6m(i32 noundef, i32 noundef, i32 noundef, i32 noundef, i32 noundef, [4 x i32]) + +// Over-sized bitfield, which results in a 64-bit container type, so 64-bit +// alignment. +struct OverSizedBitfield { + int x : 64; +}; + +unsigned sizeof_OverSizedBitfield = sizeof(OverSizedBitfield); +unsigned alignof_OverSizedBitfield = alignof(OverSizedBitfield); + +// CHECK: define{{.*}} void @g7 +// CHECK: call void @f7(i32 noundef 1, [2 x i32] [i32 42, i32 0]) +// CHECK: declare void @f7(i32 noundef, [2 x i32]) +void f7(int a, OverSizedBitfield b); +void g7() { + OverSizedBitfield s = {42}; + f7(1, s); +} + +// There are no 128-bit fundamental data types defined by AAPCS32, so this gets +// a 64-bit container plus 64 bits of padding, giving it a size of 16 bytes and +// alignment of 8 bytes. +struct VeryOverSizedBitfield { + int x : 128; +}; + +unsigned sizeof_VeryOverSizedBitfield = sizeof(VeryOverSizedBitfield); +unsigned alignof_VeryOverSizedBitfield = alignof(VeryOverSizedBitfield); + +// CHECK: define{{.*}} void @g8 +// CHECK: call void @f8(i32 noundef 1, [4 x i32] [i32 42, i32 0, i32 0, i32 0]) +// CHECK: declare void @f8(i32 noundef, [4 x i32]) +void f8(int a, VeryOverSizedBitfield b); +void g8() { + VeryOverSizedBitfield s = {42}; + f8(1, s); +} + } diff --git a/clang/test/CodeGen/aapcs64-align.cpp b/clang/test/CodeGen/aapcs64-align.cpp index 7a8151022852e..e9d22135836e4 100644 --- a/clang/test/CodeGen/aapcs64-align.cpp +++ b/clang/test/CodeGen/aapcs64-align.cpp @@ -5,6 +5,13 @@ extern "C" { +// CHECK: @sizeof_OverSizedBitfield ={{.*}} global i32 8 +// CHECK: @alignof_OverSizedBitfield ={{.*}} global i32 8 +// CHECK: @sizeof_VeryOverSizedBitfield ={{.*}} global i32 16 +// CHECK: @alignof_VeryOverSizedBitfield ={{.*}} global i32 8 +// CHECK: @sizeof_RidiculouslyOverSizedBitfield ={{.*}} global i32 32 +// CHECK: @alignof_RidiculouslyOverSizedBitfield ={{.*}} global i32 8 + // Base case, nothing interesting. struct S { long x, y; @@ -161,5 +168,62 @@ int test_bitint8(){ } // CHECK: ret i32 1 +// Over-sized bitfield, which results in a 64-bit container type, so 64-bit +// alignment. +struct OverSizedBitfield { + int x : 64; +}; + +unsigned sizeof_OverSizedBitfield = sizeof(OverSizedBitfield); +unsigned alignof_OverSizedBitfield = alignof(OverSizedBitfield); + +// CHECK: define{{.*}} void @g7 +// CHECK: call void @f7(i32 noundef 1, i64 42) +// CHECK: declare void @f7(i32 noundef, i64) +void f7(int a, OverSizedBitfield b); +void g7() { + OverSizedBitfield s = {42}; + f7(1, s); +} + +// There are no 128-bit fundamental data types defined by AAPCS32, so this gets +// a 64-bit container plus 64 bits of padding, giving it a size of 16 bytes and +// alignment of 8 bytes. +struct VeryOverSizedBitfield { + int x : 128; +}; + +unsigned sizeof_VeryOverSizedBitfield = sizeof(VeryOverSizedBitfield); +unsigned alignof_VeryOverSizedBitfield = alignof(VeryOverSizedBitfield); + +// CHECK: define{{.*}} void @g8 +// CHECK: call void @f8(i32 noundef 1, [2 x i64] [i64 42, i64 0]) +// CHECK: declare void @f8(i32 noundef, [2 x i64]) +void f8(int a, VeryOverSizedBitfield b); +void g8() { + VeryOverSizedBitfield s = {42}; + f8(1, s); +} + +// There are no bigger fundamental data types, so this gets a 128-bit container +// and 128 bits of padding, giving the struct a size of 32 bytes, and an +// alignment of 16 bytes. This is over the PCS size limit of 16 bytes, so it +// will be passed indirectly. +struct RidiculouslyOverSizedBitfield { + int x : 256; +}; + +unsigned sizeof_RidiculouslyOverSizedBitfield = sizeof(RidiculouslyOverSizedBitfield); +unsigned alignof_RidiculouslyOverSizedBitfield = alignof(RidiculouslyOverSizedBitfield); + +// CHECK: define{{.*}} void @g9 +// CHECK: call void @f9(i32 noundef 1, ptr noundef nonnull %agg.tmp) +// CHECK: declare void @f9(i32 noundef, ptr noundef) +void f9(int a, RidiculouslyOverSizedBitfield b); +void g9() { + RidiculouslyOverSizedBitfield s = {42}; + f9(1, s); +} + } >From 75e6e376b72b8bd47c45539d584b03e4d1163d16 Mon Sep 17 00:00:00 2001 From: Oliver Stannard <oliver.stann...@arm.com> Date: Tue, 11 Feb 2025 17:37:57 +0000 Subject: [PATCH 2/4] [AArch64] Allow 128-bit containers for over-sized bitfields AAPCS64 defines the bitfield layout algorithm for over-sized bitfields as picking a container which is the fundamental integer data type with the largest size less than or equal to the bit-field width. Since AAPCS64 has a 128-bit integer fundamental data type, we need to consider Int128 as a container type for AArch64. --- clang/include/clang/Basic/TargetInfo.h | 8 ++++++++ clang/lib/AST/RecordLayoutBuilder.cpp | 9 ++++++--- clang/lib/Basic/TargetInfo.cpp | 1 + clang/lib/Basic/Targets/AArch64.cpp | 4 ++++ clang/test/CodeGen/aapcs64-align.cpp | 4 ++-- .../debug-info-structured-binding-bitfield.cpp | 4 ++-- 6 files changed, 23 insertions(+), 7 deletions(-) diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 070cc792ca7db..db23afa6d6f0b 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -199,6 +199,10 @@ struct TransferrableTargetInfo { /// zero length bitfield, regardless of the zero length bitfield type. unsigned ZeroLengthBitfieldBoundary; + /// The largest container size which should be used for an over-sized + /// bitfield, in bits. + unsigned LargestOverSizedBitfieldContainer; + /// If non-zero, specifies a maximum alignment to truncate alignment /// specified in the aligned attribute of a static variable to this value. unsigned MaxAlignedAttribute; @@ -954,6 +958,10 @@ class TargetInfo : public TransferrableTargetInfo, return ZeroLengthBitfieldBoundary; } + unsigned getLargestOverSizedBitfieldContainer() const { + return LargestOverSizedBitfieldContainer; + } + /// Get the maximum alignment in bits for a static variable with /// aligned attribute. unsigned getMaxAlignedAttribute() const { return MaxAlignedAttribute; } diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index ae6d299024c6d..c461c3a12c8fd 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -1469,15 +1469,18 @@ void ItaniumRecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize, // sizeof(T')*8 <= n. QualType IntegralPODTypes[] = { - Context.UnsignedCharTy, Context.UnsignedShortTy, Context.UnsignedIntTy, - Context.UnsignedLongTy, Context.UnsignedLongLongTy + Context.UnsignedCharTy, Context.UnsignedShortTy, + Context.UnsignedIntTy, Context.UnsignedLongTy, + Context.UnsignedLongLongTy, Context.UnsignedInt128Ty, }; QualType Type; + uint64_t MaxSize = + Context.getTargetInfo().getLargestOverSizedBitfieldContainer(); for (const QualType &QT : IntegralPODTypes) { uint64_t Size = Context.getTypeSize(QT); - if (Size > FieldSize) + if (Size > FieldSize || Size > MaxSize) break; Type = QT; diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp index c0bf4e686cf03..0699ec686e4e6 100644 --- a/clang/lib/Basic/TargetInfo.cpp +++ b/clang/lib/Basic/TargetInfo.cpp @@ -141,6 +141,7 @@ TargetInfo::TargetInfo(const llvm::Triple &T) : Triple(T) { UseLeadingZeroLengthBitfield = true; UseExplicitBitFieldAlignment = true; ZeroLengthBitfieldBoundary = 0; + LargestOverSizedBitfieldContainer = 64; MaxAlignedAttribute = 0; HalfFormat = &llvm::APFloat::IEEEhalf(); FloatFormat = &llvm::APFloat::IEEEsingle(); diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp index fad8d773bfc52..3633bab6e0df9 100644 --- a/clang/lib/Basic/Targets/AArch64.cpp +++ b/clang/lib/Basic/Targets/AArch64.cpp @@ -261,6 +261,10 @@ AArch64TargetInfo::AArch64TargetInfo(const llvm::Triple &Triple, assert(UseBitFieldTypeAlignment && "bitfields affect type alignment"); UseZeroLengthBitfieldAlignment = true; + // AAPCS64 allows any "fundamental integer data type" to be used for + // over-sized bitfields, which includes 128-bit integers. + LargestOverSizedBitfieldContainer = 128; + HasUnalignedAccess = true; // AArch64 targets default to using the ARM C++ ABI. diff --git a/clang/test/CodeGen/aapcs64-align.cpp b/clang/test/CodeGen/aapcs64-align.cpp index e9d22135836e4..740b7e1cb7a69 100644 --- a/clang/test/CodeGen/aapcs64-align.cpp +++ b/clang/test/CodeGen/aapcs64-align.cpp @@ -8,9 +8,9 @@ extern "C" { // CHECK: @sizeof_OverSizedBitfield ={{.*}} global i32 8 // CHECK: @alignof_OverSizedBitfield ={{.*}} global i32 8 // CHECK: @sizeof_VeryOverSizedBitfield ={{.*}} global i32 16 -// CHECK: @alignof_VeryOverSizedBitfield ={{.*}} global i32 8 +// CHECK: @alignof_VeryOverSizedBitfield ={{.*}} global i32 16 // CHECK: @sizeof_RidiculouslyOverSizedBitfield ={{.*}} global i32 32 -// CHECK: @alignof_RidiculouslyOverSizedBitfield ={{.*}} global i32 8 +// CHECK: @alignof_RidiculouslyOverSizedBitfield ={{.*}} global i32 16 // Base case, nothing interesting. struct S { diff --git a/clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp b/clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp index e475f032f5ce3..b7aad6a5bcd21 100644 --- a/clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp +++ b/clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp @@ -248,8 +248,8 @@ struct S15 { }; // CHECK-LABEL: define dso_local void @_Z4fS15v -// CHECK: alloca %struct.S15, align 8 -// CHECK-NEXT: [[TMP0:%.*]] = alloca %struct.S15, align 8 +// CHECK: alloca %struct.S15, align 16 +// CHECK-NEXT: [[TMP0:%.*]] = alloca %struct.S15, align 16 // CHECK: #dbg_declare(ptr [[TMP0]], [[S15_A:![0-9]+]], !DIExpression(DW_OP_LLVM_extract_bits_sext, 0, 32), // CHECK-NEXT: #dbg_declare(ptr [[TMP0]], [[S15_B:![0-9]+]], !DIExpression(DW_OP_plus_uconst, 16, DW_OP_LLVM_extract_bits_zext, 0, 32), // >From 01b4a28226fbbc842a67720c96e02fadb96844b8 Mon Sep 17 00:00:00 2001 From: Oliver Stannard <oliver.stann...@arm.com> Date: Tue, 11 Feb 2025 17:43:49 +0000 Subject: [PATCH 3/4] [ARM,AArch64] Over-sized bitfields affect unadjusted alignment The container type picked for an over-sized bitfield already contributes to the alignment of the structure, but it should also contribute to the "unadjusted alignment" which is used by the ARM and AArch64 PCS. --- clang/lib/AST/RecordLayoutBuilder.cpp | 1 + clang/test/CodeGen/aapcs-align.cpp | 8 ++++---- clang/test/CodeGen/aapcs64-align.cpp | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index c461c3a12c8fd..d9380c4b6ae96 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -1523,6 +1523,7 @@ void ItaniumRecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize, setSize(std::max(getSizeInBits(), getDataSizeInBits())); // Remember max struct/class alignment. + UnadjustedAlignment = std::max(UnadjustedAlignment, TypeAlign); UpdateAlignment(TypeAlign); } diff --git a/clang/test/CodeGen/aapcs-align.cpp b/clang/test/CodeGen/aapcs-align.cpp index 01b5ea5a69fdc..c7bc5ba0bbfef 100644 --- a/clang/test/CodeGen/aapcs-align.cpp +++ b/clang/test/CodeGen/aapcs-align.cpp @@ -154,8 +154,8 @@ unsigned sizeof_OverSizedBitfield = sizeof(OverSizedBitfield); unsigned alignof_OverSizedBitfield = alignof(OverSizedBitfield); // CHECK: define{{.*}} void @g7 -// CHECK: call void @f7(i32 noundef 1, [2 x i32] [i32 42, i32 0]) -// CHECK: declare void @f7(i32 noundef, [2 x i32]) +// CHECK: call void @f7(i32 noundef 1, [1 x i64] [i64 42]) +// CHECK: declare void @f7(i32 noundef, [1 x i64]) void f7(int a, OverSizedBitfield b); void g7() { OverSizedBitfield s = {42}; @@ -173,8 +173,8 @@ unsigned sizeof_VeryOverSizedBitfield = sizeof(VeryOverSizedBitfield); unsigned alignof_VeryOverSizedBitfield = alignof(VeryOverSizedBitfield); // CHECK: define{{.*}} void @g8 -// CHECK: call void @f8(i32 noundef 1, [4 x i32] [i32 42, i32 0, i32 0, i32 0]) -// CHECK: declare void @f8(i32 noundef, [4 x i32]) +// CHECK: call void @f8(i32 noundef 1, [2 x i64] [i64 42, i64 0]) +// CHECK: declare void @f8(i32 noundef, [2 x i64]) void f8(int a, VeryOverSizedBitfield b); void g8() { VeryOverSizedBitfield s = {42}; diff --git a/clang/test/CodeGen/aapcs64-align.cpp b/clang/test/CodeGen/aapcs64-align.cpp index 740b7e1cb7a69..9578772dace75 100644 --- a/clang/test/CodeGen/aapcs64-align.cpp +++ b/clang/test/CodeGen/aapcs64-align.cpp @@ -197,8 +197,8 @@ unsigned sizeof_VeryOverSizedBitfield = sizeof(VeryOverSizedBitfield); unsigned alignof_VeryOverSizedBitfield = alignof(VeryOverSizedBitfield); // CHECK: define{{.*}} void @g8 -// CHECK: call void @f8(i32 noundef 1, [2 x i64] [i64 42, i64 0]) -// CHECK: declare void @f8(i32 noundef, [2 x i64]) +// CHECK: call void @f8(i32 noundef 1, i128 42) +// CHECK: declare void @f8(i32 noundef, i128) void f8(int a, VeryOverSizedBitfield b); void g8() { VeryOverSizedBitfield s = {42}; >From 8ce77cc5498093f60b27358fc6cc199824a10cc1 Mon Sep 17 00:00:00 2001 From: Oliver Stannard <oliver.stann...@arm.com> Date: Thu, 20 Feb 2025 13:54:19 +0000 Subject: [PATCH 4/4] Update comment in test --- clang/test/CodeGen/aapcs64-align.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/test/CodeGen/aapcs64-align.cpp b/clang/test/CodeGen/aapcs64-align.cpp index 9578772dace75..e69faf231936c 100644 --- a/clang/test/CodeGen/aapcs64-align.cpp +++ b/clang/test/CodeGen/aapcs64-align.cpp @@ -186,9 +186,9 @@ void g7() { f7(1, s); } -// There are no 128-bit fundamental data types defined by AAPCS32, so this gets -// a 64-bit container plus 64 bits of padding, giving it a size of 16 bytes and -// alignment of 8 bytes. +// AAPCS64 does have a 128-bit integer fundamental data type, so this gets a +// 128-bit container with 128-bit alignment. This is just within the limit of +// what can be passed directly. struct VeryOverSizedBitfield { int x : 128; }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits