[HLSL] Enable InitList code to handle zero sized structs

[spall]

Enable InitList code to handle zero sized structs; this includes structs with no fields and those with only unnamed bitfields.
Closes #157920

[llvmbot]

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Sarah Spall (spall)

Changes

Enable InitList code to handle zero sized structs; this includes structs with no fields and those with only unnamed bitfields.
Closes #157920

---

Full diff: https://github.com/llvm/llvm-project/pull/160355.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaHLSL.cpp (+2)
• (modified) clang/test/CodeGenHLSL/BasicFeatures/InitLists.hlsl (+42)

diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 7a61474e2a25c..7f85886f72366 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -4284,6 +4284,8 @@ bool SemaHLSL::transformInitList(const InitializedEntity &Entity,
   }
   size_t ExpectedSize = ILT.DestTypes.size();
   size_t ActualSize = ILT.ArgExprs.size();
+  if (ExpectedSize == 0 && ActualSize == 0)
+    return true;
   // For incomplete arrays it is completely arbitrary to choose whether we think
   // the user intended fewer or more elements. This implementation assumes that
   // the user intended more, and errors that there are too few initializers to
diff --git a/clang/test/CodeGenHLSL/BasicFeatures/InitLists.hlsl b/clang/test/CodeGenHLSL/BasicFeatures/InitLists.hlsl
index 5a5c45f686956..5fe69806b61eb 100644
--- a/clang/test/CodeGenHLSL/BasicFeatures/InitLists.hlsl
+++ b/clang/test/CodeGenHLSL/BasicFeatures/InitLists.hlsl
@@ -50,6 +50,16 @@ struct Unnamed {
   int : 8;
 };
 
+struct Empty {
+};
+
+struct UnnamedOnly {
+  int : 8;
+};
+
+// CHECK-DAG: [[ConstE:@.*]] = private unnamed_addr constant %struct.Empty undef, align 1
+// CHECK-DAG: [[ConstUO:@.*]] = private unnamed_addr constant %struct.UnnamedOnly undef, align 1
+
 // Case 1: Extraneous braces get ignored in literal instantiation.
 // CHECK-LABEL: define hidden void @_Z5case1v(
 // CHECK-SAME: ptr dead_on_unwind noalias writable sret([[STRUCT_TWOFLOATS:%.*]]) align 1 [[AGG_RESULT:%.*]]) #[[ATTR0:[0-9]+]] {
@@ -985,3 +995,35 @@ void case18() {
 void case19(Unnamed U) {
   TwoInts TI = {U, 1};
 }
+
+// InitList with Empty Struct on LHS
+// CHECK-LABEL: case20
+// CHECK: [[E:%.*]] = alloca %struct.Empty, align 1
+// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 1 [[E]], ptr align 1 [[ConstE]], i32 1, i1 false)
+void case20() {
+  Empty E = {};
+}
+
+// InitList with Empty Struct on RHS
+// CHECK-LABEL: case21
+// CHECK: [[TI:%.*]] = alloca %struct.TwoInts, align 1
+// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 1 %TI, ptr align 1 {{.*}}, i32 8, i1 false)
+void case21(Empty E) {
+  TwoInts TI = {E, 1, 2};
+}
+
+// InitList with Struct with only unnamed bitfield on LHS
+// CHECK-LABEL: case22
+// CHECK: [[UO:%.*]] = alloca %struct.UnnamedOnly, align 1
+// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 1 [[UO]], ptr align 1 [[ConstUO]], i32 1, i1 false)
+void case22() {
+ UnnamedOnly UO = {}; 
+}
+
+// InitList with Struct with only unnamed bitfield on RHS
+// CHECK-LABEL: case23
+// CHECK: [[TI:%.*]] = alloca %struct.TwoInts, align 1
+// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 1 [[TI]], ptr align 1 {{.*}}, i32 8, i1 false)
+void case23(UnnamedOnly UO) {
+  TwoInts TI = {UO, 1, 2};
+}

[Icohedron]

Why is the length of the memcpy for an empty struct 1 byte instead of 0 bytes?

[spall]

from googling it seems that is the size of an empty struct in C++.

[Icohedron]

Is this detail important enough to have a mention in the hlsl spec too?

[spall]

I'm not sure our specification currently mentions implementation details such as this, so I'm not sure this, which I believe is an implementation detail, would be appropriate to document there.

[Icohedron]

If it's just an implementation detail and doesn't have any user-observable behavior then it's fine to leave it out of the spec. I was just asking to make sure it is ok.

[llvm-beanz]

The C++ object model defines that all objects of a "most derived type" have a non-zero size:

If a complete object, a data member ([class.mem]), or an array element is of class type, its type is considered the most derived class, to distinguish it from the class type of any base class subobject; an object of a most derived class type or of a non-class type is called a most derived object.

Unless it is a bit-field ([class.bit]), a most derived object shall have a non-zero size and shall occupy one or more bytes of storage. Base class subobjects may have zero size. An object of trivially copyable or standard-layout type ([basic.types]) shall occupy contiguous bytes of storage.

Source: https://timsong-cpp.github.io/cppwp/n3337/intro.object#5

This means that any instance of an object (even an "empty" object), must have a size > 0.

HLSL in DXC doesn't follow this behavior, and we should figure out what we need to do about it, but my gut is that we should align closer with C++. We almost certainly need to prevent storing empty objects in resources or group shared memory, otherwise we would need to make the size of an empty structure 4 rather than 1.

This would be a good topic to discuss in the language design meeting.

[llvm-beanz]

Looks good, one suggestion for additional tests.

[llvm-beanz]

A few other potential cases to consider:

struct EmptyDerived : Empty {};
struct UnnamedDerived : UnnamedOnly {};