瀏覽代碼

Avoid validation failure when missing annotation (#2687)

When validating CBuffer ranges, the validator may find itself with a field
with an empty EltSize. This may be due to partial reflection stripping which
removes some of the annotations, but not all. This allows the validation to
begin, but runs into problems with the missing inner fields. Specifically, this
has been found to occur when using a library stage with a cbuffer containing
an array of structs. This is a bug with the compiler that this change works
around by bailing on validation late.

This can also occur when a cbuffer contains an array of empty structs. Not
a terribly useful case, but it should be handled correctly.

This also adds tests for each of the above described conditions.
Greg Roth 5 年之前
父節點
當前提交
e26d9e3c22

+ 1 - 1
lib/HLSL/DxilValidation.cpp

@@ -4053,7 +4053,7 @@ CollectCBufferRanges(DxilStructAnnotation *annotation,
 
 
         unsigned arrayBase = base + offset;
         unsigned arrayBase = base + offset;
         if (!EltAnnotation) {
         if (!EltAnnotation) {
-          if (nullptr != constAllocator.Insert(
+          if (EltSize > 0 && nullptr != constAllocator.Insert(
                 &fieldAnnotation, arrayBase, arrayBase + arraySize - 1)) {
                 &fieldAnnotation, arrayBase, arrayBase + arraySize - 1)) {
             ValCtx.EmitFormatError(
             ValCtx.EmitFormatError(
                 ValidationRule::SmCBufferOffsetOverlap,
                 ValidationRule::SmCBufferOffsetOverlap,

+ 50 - 0
tools/clang/test/HLSLFileCheck/d3dreflect/cbuffer_alignment.hlsl

@@ -0,0 +1,50 @@
+// RUN: %dxc -T lib_6_3 %s | FileCheck %s
+
+// CHECK: ; Buffer Definitions:
+// CHECK: ;
+// CHECK: ; cbuffer cbuf
+// CHECK: ; {
+// CHECK: ;
+// CHECK: ;   struct cbuf
+// CHECK: ;   {
+// CHECK: ;
+// CHECK: ;       struct struct.Agg
+// CHECK: ;       {
+// CHECK: ;
+// CHECK: ;           struct struct.Value
+// CHECK: ;           {
+// CHECK: ;
+// CHECK: ;               uint v;                               ; Offset:    0
+// CHECK: ;
+// CHECK: ;           } value[1];;                              ; Offset:    0
+// CHECK: ;
+// CHECK: ;           uint fodder;                              ; Offset:    4
+// CHECK: ;
+// CHECK: ;       } aggie;                                      ; Offset:    0
+// CHECK: ;
+// CHECK: ;
+// CHECK: ;   } cbuf;                                           ; Offset:    0 Size:     8
+// CHECK: ;
+// CHECK: ; }
+
+
+// Test Cbuffer validation likely to cause mistaken overlaps
+struct Value { uint v; }; // partial stripping will sometimes leave this without annotation
+
+struct Agg {
+  Value value[1];
+  uint fodder;
+};
+
+cbuffer cbuf : register(b1)
+{
+  Agg aggie;
+}
+
+RWBuffer<int> Out : register(u0);
+
+[shader("raygeneration")]
+void main()
+{
+  Out[0] = aggie.value[0].v;
+}

+ 48 - 3
tools/clang/test/HLSLFileCheck/d3dreflect/empty_struct2.hlsl

@@ -17,14 +17,19 @@ cbuffer Params_cbuffer : register(b0) {
   float4 foo;
   float4 foo;
 };
 };
 
 
-float4 main(float4 pos : POSITION) : SV_POSITION { return foo; }
+cbuffer Params_cbuffer2 : register(b1) {
+  InnerStruct constArray[1];
+  float4 bar;
+};
+
+float4 main(float4 pos : POSITION) : SV_POSITION { return foo + bar; }
 
 
 
 
 // CHECK: ID3D12ShaderReflection:
 // CHECK: ID3D12ShaderReflection:
 // CHECK:   D3D12_SHADER_BUFFER_DESC:
 // CHECK:   D3D12_SHADER_BUFFER_DESC:
 // CHECK:     Shader Version: Vertex 6.0
 // CHECK:     Shader Version: Vertex 6.0
-// CHECK:     ConstantBuffers: 1
-// CHECK:     BoundResources: 1
+// CHECK:     ConstantBuffers: 2
+// CHECK:     BoundResources: 2
 // CHECK:     InputParameters: 1
 // CHECK:     InputParameters: 1
 // CHECK:     OutputParameters: 1
 // CHECK:     OutputParameters: 1
 // CHECK:   Constant Buffers:
 // CHECK:   Constant Buffers:
@@ -54,6 +59,46 @@ float4 main(float4 pos : POSITION) : SV_POSITION { return foo; }
 // CHECK:               Columns: 4
 // CHECK:               Columns: 4
 // CHECK:           CBuffer: Params_cbuffer
 // CHECK:           CBuffer: Params_cbuffer
 // CHECK:       }
 // CHECK:       }
+// CHECK:    ID3D12ShaderReflectionConstantBuffer:
+// CHECK:      D3D12_SHADER_BUFFER_DESC: Name: Params_cbuffer2
+// CHECK:        Type: D3D_CT_CBUFFER
+// CHECK:        Size: 16
+// CHECK:        uFlags: 0
+// CHECK:        Num Variables: 2
+// CHECK:      {
+// CHECK:        ID3D12ShaderReflectionVariable:
+// CHECK:          D3D12_SHADER_VARIABLE_DESC: Name: constArray
+// CHECK:            Size: 0
+// CHECK:            StartOffset: 0
+// CHECK:            uFlags: 0
+// CHECK:            DefaultValue: <nullptr>
+// CHECK:          ID3D12ShaderReflectionType:
+// CHECK:            D3D12_SHADER_TYPE_DESC: Name: InnerStruct
+// CHECK:              Class: D3D_SVC_STRUCT
+// CHECK:              Type: D3D_SVT_VOID
+// CHECK:              Elements: 1
+// CHECK:              Rows: 1
+// CHECK:              Columns: 0
+// CHECK:              Members: 0
+// CHECK:              Offset: 0
+// CHECK:          CBuffer: Params_cbuffer2
+// CHECK:        ID3D12ShaderReflectionVariable:
+// CHECK:          D3D12_SHADER_VARIABLE_DESC: Name: bar
+// CHECK:            Size: 16
+// CHECK:            StartOffset: 0
+// CHECK:            uFlags: 0x2
+// CHECK:            DefaultValue: <nullptr>
+// CHECK:          ID3D12ShaderReflectionType:
+// CHECK:            D3D12_SHADER_TYPE_DESC: Name: float4
+// CHECK:              Class: D3D_SVC_VECTOR
+// CHECK:              Type: D3D_SVT_FLOAT
+// CHECK:              Elements: 0
+// CHECK:              Rows: 1
+// CHECK:              Columns: 4
+// CHECK:              Members: 0
+// CHECK:              Offset: 0
+// CHECK:          CBuffer: Params_cbuffer2
+// CHECK:      }
 // CHECK:   Bound Resources:
 // CHECK:   Bound Resources:
 // CHECK:     D3D12_SHADER_BUFFER_DESC: Name: Params_cbuffer
 // CHECK:     D3D12_SHADER_BUFFER_DESC: Name: Params_cbuffer
 // CHECK:       Type: D3D_SIT_CBUFFER
 // CHECK:       Type: D3D_SIT_CBUFFER