瀏覽代碼

[spirv] Fix issues related to struct inheritance memory layout (#3273)

* [spirv] Fix reconstructValue for struct inheritance.

* [spirv] Fix memory layout for a derived empty struct.
Ehsan 4 年之前
父節點
當前提交
e41ac97e5e

+ 4 - 1
tools/clang/lib/SPIRV/AlignmentSizeCalculator.cpp

@@ -244,9 +244,12 @@ std::pair<uint32_t, uint32_t> AlignmentSizeCalculator::getAlignmentAndSize(
 
   // Rule 9
   if (const auto *structType = type->getAs<RecordType>()) {
+    bool hasBaseStructs = type->getAsCXXRecordDecl() &&
+                          type->getAsCXXRecordDecl()->getNumBases() > 0;
+
     // Special case for handling empty structs, whose size is 0 and has no
     // requirement over alignment (thus 1).
-    if (structType->getDecl()->field_empty())
+    if (structType->getDecl()->field_empty() && !hasBaseStructs)
       return {1, 0};
 
     uint32_t maxAlignment = 0;

+ 13 - 0
tools/clang/lib/SPIRV/SpirvEmitter.cpp

@@ -5536,6 +5536,19 @@ SpirvInstruction *SpirvEmitter::reconstructValue(SpirvInstruction *srcVal,
   if (const auto *recordType = valType->getAs<RecordType>()) {
     uint32_t index = 0;
     llvm::SmallVector<SpirvInstruction *, 4> elements;
+
+    // If the struct inherits from other structs, visit the bases.
+    const auto *decl = valType->getAsCXXRecordDecl();
+    for (auto baseIt = decl->bases_begin(), baseIe = decl->bases_end();
+         baseIt != baseIe; ++baseIt, ++index) {
+      SpirvInstruction *subSrcVal = spvBuilder.createCompositeExtract(
+          baseIt->getType(), srcVal, {index}, loc);
+      subSrcVal->setLayoutRule(srcVal->getLayoutRule());
+      elements.push_back(
+          reconstructValue(subSrcVal, baseIt->getType(), dstLR, loc));
+    }
+
+    // Go over struct fields.
     for (const auto *field : recordType->getDecl()->fields()) {
       SpirvInstruction *subSrcVal = spvBuilder.createCompositeExtract(
           field->getType(), srcVal, {index}, loc);

+ 39 - 0
tools/clang/test/CodeGenSPIRV/oo.inheritance.layout-differences.hlsl

@@ -0,0 +1,39 @@
+// Run: %dxc -T cs_6_0 -E main
+
+struct Base {
+  static const uint s_coefficientCount = 4;
+  float4 m_coefficients[s_coefficientCount];
+};
+
+struct Derived : Base {
+  float4 a;
+};
+
+RWStructuredBuffer<Derived> g_probes : register(u0);
+
+[numthreads(64u, 1u, 1u)]
+void main(uint3 dispatchThreadId : SV_DispatchThreadID) {
+  Derived p;
+  p.a = float4(0,0,0,0);
+
+// Due to layout differences, 'p' has to be reconstructed to be stored in a RWStructuredBuffer.
+// Layout differences mean that one type has decorations, and the other doesn't.
+// Note that the m_coefficients is an array and has ArrayStride decoration when in RWStructuredBuffer,
+// but does not have an ArrayStride decoration when used in the function scope (as part of p).
+// Therefore the array needs to be reconstructed.
+// The Derived.a member, however, is a vector, and has no decorations either way.
+//
+// CHECK:        [[p:%\d+]] = OpLoad %Derived_0 %p
+// CHECK:   [[p_base:%\d+]] = OpCompositeExtract %Base_0 [[p]] 0
+// CHECK:      [[arr:%\d+]] = OpCompositeExtract %_arr_v4float_uint_4_0 [[p_base]] 0
+// CHECK:     [[arr0:%\d+]] = OpCompositeExtract %v4float [[arr]] 0
+// CHECK:     [[arr1:%\d+]] = OpCompositeExtract %v4float [[arr]] 1
+// CHECK:     [[arr2:%\d+]] = OpCompositeExtract %v4float [[arr]] 2
+// CHECK:     [[arr3:%\d+]] = OpCompositeExtract %v4float [[arr]] 3
+// CHECK:  [[new_arr:%\d+]] = OpCompositeConstruct %_arr_v4float_uint_4 [[arr0]] [[arr1]] [[arr2]] [[arr3]]
+// CHECK: [[new_base:%\d+]] = OpCompositeConstruct %Base [[new_arr]]
+// CHECK:        [[a:%\d+]] = OpCompositeExtract %v4float [[p]] 1
+// CHECK:    [[new_p:%\d+]] = OpCompositeConstruct %Derived [[new_base]] [[a]]
+// CHECK:                     OpStore {{%\d+}} [[new_p]]
+	g_probes[0] = p;
+}

+ 25 - 0
tools/clang/test/CodeGenSPIRV/oo.inheritance.layout.empty-struct.hlsl

@@ -0,0 +1,25 @@
+// Run: %dxc -T cs_6_0 -E main
+
+// Note that since 'derived_count' is static,
+// Derived structures are in fact empty (have no members other than their Base)
+// The layout calculations should *not* consider Derived as size 0.
+
+// CHECK: OpDecorate %_arr_v4float_uint_4 ArrayStride 16
+// CHECK: OpMemberDecorate %Base 0 Offset 0
+struct Base {
+	static const uint count = 4;
+	float4 arr[count];
+};
+
+// CHECK: OpMemberDecorate %Derived 0 Offset 0
+struct Derived : Base {
+  static const uint derived_count = Base::count;
+};
+
+// CHECK: OpDecorate %_runtimearr_Derived ArrayStride 64
+// CHECK: OpMemberDecorate %type_RWStructuredBuffer_Derived 0 Offset 0
+RWStructuredBuffer<Derived> rwsb : register(u0);
+
+[numthreads(64u, 1u, 1u)]
+void main(uint3 dispatchThreadId : SV_DispatchThreadID) {
+}

+ 6 - 0
tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp

@@ -639,6 +639,12 @@ TEST_F(FileTest, InheritanceStageIOVS) {
 TEST_F(FileTest, InheritanceStageIOGS) {
   runFileTest("oo.inheritance.stage-io.gs.hlsl");
 }
+TEST_F(FileTest, InheritanceLayoutDifferences) {
+  runFileTest("oo.inheritance.layout-differences.hlsl");
+}
+TEST_F(FileTest, InheritanceLayoutEmptyStruct) {
+  runFileTest("oo.inheritance.layout.empty-struct.hlsl");
+}
 
 // For semantics
 // SV_Position, SV_ClipDistance, and SV_CullDistance are covered in