Partial revert of d16fba3 to fix #4719

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

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;
 

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();
   }
 

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;

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;
+    };
+}