diff --git a/ir/irtypeaggr.cpp b/ir/irtypeaggr.cpp index 3f49cc7c65..f3d4da4585 100644 --- a/ir/irtypeaggr.cpp +++ b/ir/irtypeaggr.cpp @@ -272,6 +272,41 @@ IrTypeAggr::IrTypeAggr(AggregateDeclaration *ad) LLStructType::create(gIR->context(), ad->toPrettyChars())), aggr(ad) {} +bool IrTypeAggr::isPacked(AggregateDeclaration *ad) { + // If the aggregate's size is unknown, any field with type alignment > 1 will + // make it packed. + unsigned aggregateSize = ~0u; + unsigned aggregateAlignment = 1; + if (ad->sizeok == Sizeok::done) { + aggregateSize = ad->structsize; + aggregateAlignment = ad->alignsize; + + if (auto sd = ad->isStructDeclaration()) { + if (!sd->alignment.isDefault() && !sd->alignment.isPack()) + aggregateAlignment = sd->alignment.get(); + } + } + + // Classes apparently aren't padded; their size may not match the alignment. + assert((ad->isClassDeclaration() || + (aggregateSize & (aggregateAlignment - 1)) == 0) && + "Size not a multiple of alignment?"); + + // For unions, only a subset of the fields are actually used for the IR type - + // don't care (about a few potentially needlessly packed IR structs). + for (const auto field : ad->fields) { + // The aggregate size, aggregate alignment and the field offset need to be + // multiples of the field type's alignment, otherwise the aggregate type is + // unnaturally aligned, and LLVM would insert padding. + const unsigned fieldTypeAlignment = DtoAlignment(field->type); + const auto mask = fieldTypeAlignment - 1; + if ((aggregateSize | aggregateAlignment | field->offset) & mask) + return true; + } + + return false; +} + unsigned IrTypeAggr::getMemberLocation(VarDeclaration *var, bool& isFieldIdx) const { // Note: The interface is a bit more general than what we actually return. // Specifically, the frontend offset information we use for overlapping diff --git a/ir/irtypeaggr.h b/ir/irtypeaggr.h index 780aab8000..4f4e24aa01 100644 --- a/ir/irtypeaggr.h +++ b/ir/irtypeaggr.h @@ -78,6 +78,10 @@ class IrTypeAggr : public IrType { /// explicit IrTypeAggr(AggregateDeclaration *ad); + /// Returns true, if the LLVM struct type for the aggregate must be declared + /// as packed. + static bool isPacked(AggregateDeclaration *ad); + /// AggregateDeclaration this type represents. AggregateDeclaration *aggr = nullptr; diff --git a/ir/irtypestruct.cpp b/ir/irtypestruct.cpp index ccbb5b9514..31805c7671 100644 --- a/ir/irtypestruct.cpp +++ b/ir/irtypestruct.cpp @@ -89,7 +89,8 @@ IrTypeStruct *IrTypeStruct::get(StructDeclaration *sd) { AggrTypeBuilder builder; builder.addAggregate(sd); builder.addTailPadding(sd->structsize); - isaStruct(t->type)->setBody(builder.defaultTypes(), builder.isPacked()); + bool packed = builder.isPacked() || IrTypeAggr::isPacked(sd); + isaStruct(t->type)->setBody(builder.defaultTypes(), packed); t->varGEPIndices = builder.varGEPIndices(); } diff --git a/tests/codegen/gh2346.d b/tests/codegen/gh2346.d index 61a6034ec7..60000f450f 100644 --- a/tests/codegen/gh2346.d +++ b/tests/codegen/gh2346.d @@ -1,14 +1,15 @@ // RUN: %ldc -output-ll -of=%t.ll %s && FileCheck %s < %t.ll -// Make sure the LL struct isn't packed. -// CHECK-DAG: %gh2346.UnalignedUInt = type { i32 } +// Make sure the LL struct is packed due to an unnatural overall alignment of 1. +// CHECK-DAG: %gh2346.UnalignedUInt = type <{ i32 }> struct UnalignedUInt { align(1) uint a; } static assert(UnalignedUInt.alignof == 1); static assert(UnalignedUInt.sizeof == 4); -// CHECK-DAG: %gh2346.Container = type <{ i8, %gh2346.UnalignedUInt }> +// Then there's no need to pack naturally aligned containers. +// CHECK-DAG: %gh2346.Container = type { i8, %gh2346.UnalignedUInt } struct Container { ubyte one; UnalignedUInt two; @@ -17,14 +18,14 @@ static assert(Container.alignof == 1); static assert(Container.sizeof == 5); static assert(Container.two.offsetof == 1); -// CHECK-DAG: %gh2346.UnalignedUInt2 = type { i32 } +// CHECK-DAG: %gh2346.UnalignedUInt2 = type <{ i32 }> struct UnalignedUInt2 { align(2) uint a; } static assert(UnalignedUInt2.alignof == 2); static assert(UnalignedUInt2.sizeof == 4); -// CHECK-DAG: %gh2346.Container2 = type <{ i8, [1 x i8], %gh2346.UnalignedUInt2 }> +// CHECK-DAG: %gh2346.Container2 = type { i8, [1 x i8], %gh2346.UnalignedUInt2 } struct Container2 { ubyte one; UnalignedUInt2 two; @@ -42,7 +43,7 @@ static assert(PackedContainer2.alignof == 1); static assert(PackedContainer2.sizeof == 5); static assert(PackedContainer2.two.offsetof == 1); -// CHECK-DAG: %gh2346.WeirdContainer = type <{ i8, [1 x i8], %gh2346.UnalignedUInt, [2 x i8] }> +// CHECK-DAG: %gh2346.WeirdContainer = type { i8, [1 x i8], %gh2346.UnalignedUInt, [2 x i8] } align(4) struct WeirdContainer { ubyte one; align(2) UnalignedUInt two; @@ -51,14 +52,14 @@ static assert(WeirdContainer.alignof == 4); static assert(WeirdContainer.sizeof == 8); static assert(WeirdContainer.two.offsetof == 2); -// CHECK-DAG: %gh2346.ExplicitlyUnalignedUInt2 = type { i32 } +// CHECK-DAG: %gh2346.ExplicitlyUnalignedUInt2 = type <{ i32 }> align(2) struct ExplicitlyUnalignedUInt2 { uint a; } static assert(ExplicitlyUnalignedUInt2.alignof == 2); static assert(ExplicitlyUnalignedUInt2.sizeof == 4); -// CHECK-DAG: %gh2346.AnotherContainer = type <{ i8, [1 x i8], %gh2346.ExplicitlyUnalignedUInt2 }> +// CHECK-DAG: %gh2346.AnotherContainer = type { i8, [1 x i8], %gh2346.ExplicitlyUnalignedUInt2 } struct AnotherContainer { ubyte one; ExplicitlyUnalignedUInt2 two; diff --git a/tests/codegen/gh4719.d b/tests/codegen/gh4719.d new file mode 100644 index 0000000000..46b6e3ed1b --- /dev/null +++ b/tests/codegen/gh4719.d @@ -0,0 +1,32 @@ +// RUN: %ldc -c -output-ll -of=%t.ll %s + +struct TraceBuf { + align(1) uint args; +} + +// Test correct compilation (no error) of the context pointer type for the delegate of `foo`. +void foo() { + byte[2] fixDescs; + TraceBuf fixLog; + + auto dlg = delegate() { + fixDescs[0] = 1; + fixLog.args = 1; + }; +} + +class TraceClass { + align(1) + uint args; +} + +// Test correct compilation (no error) of the context pointer type for the delegate of `foo2`. +void foo2() { + byte[2] fixDescs; + scope TraceClass fixLog; + + auto dlg = delegate() { + fixDescs[0] = 1; + fixLog.args = 1; + }; +}