-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[HLSL] Enable InitList code to handle zero sized structs #160355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,6 +50,22 @@ struct Unnamed { | |
int : 8; | ||
}; | ||
|
||
struct Empty { | ||
}; | ||
|
||
struct UnnamedOnly { | ||
int : 8; | ||
}; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A few other potential cases to consider:
|
||
struct EmptyDerived : Empty {}; | ||
|
||
struct UnnamedDerived : UnnamedOnly {}; | ||
|
||
// CHECK-DAG: [[ConstE:@.*]] = private unnamed_addr constant %struct.Empty undef, align 1 | ||
// CHECK-DAG: [[ConstUO:@.*]] = private unnamed_addr constant %struct.UnnamedOnly undef, align 1 | ||
// CHECK-DAG: [[ConstED:@.*]] = private unnamed_addr constant %struct.EmptyDerived undef, align 1 | ||
// CHECK-DAG: [[ConstUD:@.*]] = private unnamed_addr constant %struct.UnnamedDerived 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 +1001,57 @@ 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the length of the memcpy for an empty struct 1 byte instead of 0 bytes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. from googling it seems that is the size of an empty struct in C++. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this detail important enough to have a mention in the hlsl spec too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The C++ object model defines that all objects of a "most derived type" have a non-zero size:
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. |
||
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}; | ||
} | ||
|
||
// InitList with Derived empty struct on LHS | ||
// InitList with Derived unnamed bitfield on LHS | ||
// CHECK-LABEL: case24 | ||
// CHECK: [[ED:%.*]] = alloca %struct.EmptyDerived, align 1 | ||
// CHECK-NEXT: [[UD:%.*]] = alloca %struct.UnnamedDerived, align 1 | ||
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 1 %ED, ptr align 1 [[ConstED]], i32 1, i1 false) | ||
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 1 %UD, ptr align 1 [[ConstUD]], i32 1, i1 false) | ||
void case24() { | ||
EmptyDerived ED = {}; | ||
UnnamedDerived UD = {}; | ||
} | ||
|
||
// CHECK-LABEL: case25 | ||
// CHECK: [[TI1:%.*]] = alloca %struct.TwoInts, align 1 | ||
// CHECK-NEXT: [[TI2:%.*]] = alloca %struct.TwoInts, align 1 | ||
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 1 %TI1, ptr align 1 {{.*}}, i32 8, i1 false) | ||
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 1 %TI2, ptr align 1 {{.*}}, i32 8, i1 false) | ||
void case25(EmptyDerived ED, UnnamedDerived UD) { | ||
TwoInts TI1 = {ED, 1, 2}; | ||
TwoInts TI2 = {UD, 1, 2}; | ||
} |
Uh oh!
There was an error while loading. Please reload this page.