Remove padding in empty structs (#2124)

Clang implicitly adds an i8 to empty structs. Subsequent empty struct fields in a larger structure get packed differently by clang (all in one dword) than by the llvm DataLayout (one per dword), causing inconsistent debug metadata. With this change, we stop adding implicit i8s, such that empty structs are zero-sized and we don't have to ignore their dummy field, which fixes debug info inconsistencies.

tools/clang/lib/AST/RecordLayoutBuilder.cpp

@@ -2358,8 +2358,14 @@ void MicrosoftRecordLayoutBuilder::layout(const RecordDecl *RD) {
 }
 
 void MicrosoftRecordLayoutBuilder::cxxLayout(const CXXRecordDecl *RD) {
-  // The C++ standard says that empty structs have size 1.
-  MinEmptyStructSize = CharUnits::One();
+  // HLSL Change Begins
+  if (Context.getLangOpts().HLSL) {
+    MinEmptyStructSize = CharUnits::fromQuantity(0);
+  }
+  else { // HLSL Change Ends
+    // The C++ standard says that empty structs have size 1.
+    MinEmptyStructSize = CharUnits::One();
+  } // HLSL Change
   initializeLayout(RD);
   initializeCXXLayout(RD);
   layoutNonVirtualBases(RD);
@@ -2741,12 +2747,14 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) {
   if (Size.isZero()) {
     EndsWithZeroSizedObject = true;
     LeadsWithZeroSizedBase = true;
+    if (!Context.getLangOpts().HLSL) { // HLSL Change - allow empty structs to be zero sized
     // Zero-sized structures have size equal to their alignment if a
     // __declspec(align) came into play.
     if (RequiredAlignment >= MinEmptyStructSize)
       Size = Alignment;
     else
       Size = MinEmptyStructSize;
+    } // HLSL Change
   }
 
   if (UseExternalLayout) {

tools/clang/lib/CodeGen/CGHLSLMS.cpp

@@ -4005,6 +4005,7 @@ static void SimplifyBitCast(BitCastOperator *BC, SmallInstSet &deadInsts) {
     Value *zeroIdx = Builder.getInt32(0);
     unsigned nestLevel = 1;
     while (llvm::StructType *ST = dyn_cast<llvm::StructType>(FromTy)) {
+      if (ST->getNumElements() == 0) break;
       FromTy = ST->getElementType(0);
       nestLevel++;
     }
@@ -7126,18 +7127,6 @@ void CGMSHLSLRuntime::EmitHLSLOutParamConversionInit(
                         CGF.getContext().getTrivialTypeSourceInfo(ParamTy),
                         StorageClass::SC_Auto);
 
-    bool isEmptyAggregate = false;
-    if (isAggregateType) {
-      DXASSERT(argAddr, "should be RV or simple LV");
-      llvm::Type *ElTy = argAddr->getType()->getPointerElementType();
-      while (ElTy->isArrayTy())
-        ElTy = ElTy->getArrayElementType();
-      if (llvm::StructType *ST = dyn_cast<StructType>(ElTy)) {
-        DxilStructAnnotation *SA = m_pHLModule->GetTypeSystem().GetStructAnnotation(ST);
-        isEmptyAggregate = SA && SA->IsEmptyStruct();
-      }
-    }
-
     // Aggregate type will be indirect param convert to pointer type.
     // So don't update to ReferenceType, use RValue for it.
     const DeclRefExpr *tmpRef = DeclRefExpr::Create(
@@ -7160,11 +7149,6 @@ void CGMSHLSLRuntime::EmitHLSLOutParamConversionInit(
     // add it to local decl map
     TmpArgMap(tmpArg, tmpArgAddr);
 
-    // If param is empty, copy in/out will just create problems.
-    // No copy will result in undef, which is fine.
-    if (isEmptyAggregate)
-      continue;
-
     LValue tmpLV = LValue::MakeAddr(tmpArgAddr, ParamTy, argAlignment,
                                     CGF.getContext());
 

tools/clang/test/CodeGenHLSL/batch/misc/shader-compat-suite/lib_arg_flatten/lib_empty_struct_arg.hlsl

@@ -1,7 +1,10 @@
 // RUN: %dxc -T lib_6_3 -auto-binding-space 11 -default-linkage external %s | FileCheck %s
 
-// Make sure empty struct arg is replaced with undef.
-// CHECK: call float @"\01?test@@YAMUT@@@Z"(%struct.T* undef)
+// Make sure calls with empty struct params are well-behaved
+
+// CHECK: %[[alloca:.*]] = alloca %struct.T
+// Copy from t is a no-op, no code should be generated
+// CHECK-NEXT: call float @"\01?test@@YAMUT@@@Z"(%struct.T* nonnull %[[alloca]])
 
 struct T {
 };

tools/clang/test/CodeGenHLSL/debug/empty_global_struct.hlsl

@@ -1,7 +1,10 @@
 // RUN: %dxc -E main -T ps_6_0 %s -Zi -Od | FileCheck %s
 
-// CHECK-DAG: !DIGlobalVariable(name: "foo.0", linkageName: "foo.0", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ty:[0-9]+]]
-// CHECK-DAG: ![[ty]] = !DIDerivedType(tag: DW_TAG_member, name: "S.0", file: !1, line: 6, baseType: !{{[0-9]+}}, size: 8
+// CHECK-DAG: ![[S:.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "S", {{.*}}, align: 8,
+// CHECK-DAG: !DIGlobalVariable(name: "foo", {{.*}}, type: ![[S]], isLocal: true, isDefinition: true)
+
+// Exclude quoted source file (see readme)
+// CHECK-LABEL: {{!"[^"]*\\0A[^"]*"}}
 
 struct S {
   float f() {

tools/clang/test/CodeGenHLSL/debug/empty_global_struct_const.hlsl

@@ -1,17 +0,0 @@
-// RUN: %dxc -E main -T ps_6_0 %s -Zi -Od | FileCheck %s
-
-// CHECK-DAG: !DIGlobalVariable(name: "foo.0", linkageName: "foo.0", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ty:[0-9]+]]
-// CHECK-DAG: ![[ty]] = !DIDerivedType(tag: DW_TAG_member, name: ".0", baseType: !{{[0-9]+}}, size: 8
-
-struct S {
-  float f() {
-    return 420;
-  }
-};
-
-const static S foo;
-
-float main() : SV_Target {
-  return foo.f();
-}
-

tools/clang/test/CodeGenHLSL/debug/types/empty_struct_layout.hlsl

@@ -0,0 +1,27 @@
+// RUN: %dxc -E main -T vs_6_2 -enable-16bit-types -Zi %s | FileCheck %s
+
+// Check that the clang field offsets (as seen from debug type definitions)
+// and the LLVM field offsets (as seen from SROA'd offsets) are consistent
+// when empty structs are involved.
+
+// Exclude quoted source file (see readme)
+// CHECK-LABEL: {{!"[^"]*\\0A[^"]*"}}
+
+// CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, {{.*}}, size: 64, align: 32
+// CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "x", {{.*}}, align: 32)
+// CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "empty1", {{.*}}, align: 8, offset: 32)
+// CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "empty2", {{.*}}, align: 8, offset: 32)
+// CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "y", {{.*}}, size: 32, align: 32, offset: 32)
+// CHECK-DAG: !DIExpression(DW_OP_bit_piece, 32, 32)
+
+float main(float val : IN) : OUT
+{
+  struct
+  {
+    float x; // HLSL [0,32), C++ [0, 32)
+    struct {} empty1; // HLSL [32,32), C++ [32, 40)
+    struct {} empty2; // HLSL [32,32), C++ [40, 48)
+    float y; // HLSL [32,64), C++ [64, 96)
+  } s = { 0, val };
+  return s.y;
+}