Skip to content

Commit

Permalink
[spirv] separate resource from cbuffer
Browse files Browse the repository at this point in the history
The current DXC does not separate the resources from a cbuffer, which
results in a OpTypeStruct including resources. We have to separate the
resources from a cbuffer.

Fixes microsoft#4019
  • Loading branch information
jaebaek committed Oct 26, 2021
1 parent 53ccb3a commit 4c3d09c
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 1 deletion.
24 changes: 23 additions & 1 deletion tools/clang/lib/SPIRV/DeclResultIdMapper.cpp
Expand Up @@ -1093,14 +1093,20 @@ SpirvVariable *DeclResultIdMapper::createStructOrStructArrayVarOfExplicitLayout(
// HLSLBufferDecls).
assert(isa<VarDecl>(subDecl) || isa<FieldDecl>(subDecl));
const auto *declDecl = cast<DeclaratorDecl>(subDecl);
auto varType = declDecl->getType();
if (const auto *fieldVar = dyn_cast<VarDecl>(subDecl)) {
if (isResourceType(varType)) {
createExternVar(fieldVar);
continue;
}
}

// In case 'register(c#)' annotation is placed on a global variable.
const hlsl::RegisterAssignment *registerC =
forGlobals ? getRegisterCAssignment(declDecl) : nullptr;

// All fields are qualified with const. It will affect the debug name.
// We don't need it here.
auto varType = declDecl->getType();
varType.removeLocalConst();
HybridStructType::FieldInfo info(varType, declDecl->getName(),
declDecl->getAttr<VKOffsetAttr>(),
Expand Down Expand Up @@ -1179,7 +1185,12 @@ SpirvVariable *DeclResultIdMapper::createCTBuffer(const HLSLBufferDecl *decl) {
if (shouldSkipInStructLayout(subDecl))
continue;

// If subDecl is a variable with resource type, we already added a separate
// OpVariable for it in createStructOrStructArrayVarOfExplicitLayout().
const auto *varDecl = cast<VarDecl>(subDecl);
if (isResourceType(varDecl->getType()))
continue;

astDecls[varDecl] = createDeclSpirvInfo(bufferVar, index++);
}
resourceVars.emplace_back(
Expand Down Expand Up @@ -1344,7 +1355,12 @@ DeclResultIdMapper::createShaderRecordBuffer(const HLSLBufferDecl *decl,
if (shouldSkipInStructLayout(subDecl))
continue;

// If subDecl is a variable with resource type, we already added a separate
// OpVariable for it in createStructOrStructArrayVarOfExplicitLayout().
const auto *varDecl = cast<VarDecl>(subDecl);
if (isResourceType(varDecl->getType()))
continue;

astDecls[varDecl] = createDeclSpirvInfo(bufferVar, index++);
}
return bufferVar;
Expand Down Expand Up @@ -1381,6 +1397,12 @@ void DeclResultIdMapper::createGlobalsCBuffer(const VarDecl *var) {
return;
}

// If subDecl is a variable with resource type, we already added a
// separate OpVariable for it in
// createStructOrStructArrayVarOfExplicitLayout().
if (isResourceType(varDecl->getType()))
continue;

astDecls[varDecl] = createDeclSpirvInfo(globals, index++);
}
}
Expand Down
50 changes: 50 additions & 0 deletions tools/clang/test/CodeGenSPIRV/type.cbuffer.including.resource.hlsl
@@ -0,0 +1,50 @@
// Run: %dxc -T ps_6_0 -E main

// CHECK: OpDecorate %MyCbuffer DescriptorSet 0
// CHECK: OpDecorate %MyCbuffer Binding 1
// CHECK: OpDecorate %AnotherCBuffer DescriptorSet 0
// CHECK: OpDecorate %AnotherCBuffer Binding 2
// CHECK: OpDecorate %y DescriptorSet 0
// CHECK: OpDecorate %y Binding 0
// CHECK: OpDecorate %z DescriptorSet 0
// CHECK: OpDecorate %z Binding 3
// CHECK: OpDecorate %w DescriptorSet 0
// CHECK: OpDecorate %w Binding 4
// CHECK: OpMemberDecorate %type_MyCbuffer 0 Offset 0

// CHECK: %type_MyCbuffer = OpTypeStruct %v4float
// CHECK: %type_AnotherCBuffer = OpTypeStruct

// CHECK: %y = OpVariable %_ptr_UniformConstant_type_2d_image UniformConstant
// CHECK: %z = OpVariable %_ptr_UniformConstant_type_sampler UniformConstant
// CHECK: %MyCbuffer = OpVariable %_ptr_Uniform_type_MyCbuffer Uniform
// CHECK: %w = OpVariable %_ptr_UniformConstant_type_sampler UniformConstant
// CHECK: %AnotherCBuffer = OpVariable %_ptr_Uniform_type_AnotherCBuffer Uniform

cbuffer MyCbuffer : register(b1) {
float4 x;
Texture2D y;
SamplerState z;
};

cbuffer AnotherCBuffer : register(b2) {
SamplerState w;
}

float4 main(float2 uv : TEXCOORD) : SV_TARGET {
// CHECK: [[ptr_x:%\d+]] = OpAccessChain %_ptr_Uniform_v4float %MyCbuffer %int_0
// CHECK: [[x:%\d+]] = OpLoad %v4float [[ptr_x]]

// CHECK: [[y:%\d+]] = OpLoad %type_2d_image %y
// CHECK: [[z:%\d+]] = OpLoad %type_sampler %z
// CHECK: [[yz:%\d+]] = OpSampledImage %type_sampled_image [[y]] [[z]]
// CHECK: [[sample0:%\d+]] = OpImageSampleImplicitLod %v4float [[yz]]
// CHECK: [[add0:%\d+]] = OpFAdd %v4float [[x]] [[sample0]]

// CHECK: [[y:%\d+]] = OpLoad %type_2d_image %y
// CHECK: [[w:%\d+]] = OpLoad %type_sampler %w
// CHECK: [[yw:%\d+]] = OpSampledImage %type_sampled_image [[y]] [[w]]
// CHECK: [[sample1:%\d+]] = OpImageSampleImplicitLod %v4float [[yw]]
// CHECK: OpFAdd %v4float [[add0]] [[sample1]]
return x + y.Sample(z, uv) + y.Sample(w, uv);
}
3 changes: 3 additions & 0 deletions tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp
Expand Up @@ -92,6 +92,9 @@ TEST_F(FileTest, RWBufferTypeStructError) {
runFileTest("type.rwbuffer.struct.error.hlsl", Expect::Failure);
}
TEST_F(FileTest, CBufferType) { runFileTest("type.cbuffer.hlsl"); }
TEST_F(FileTest, TypeCBufferIncludingResource) {
runFileTest("type.cbuffer.including.resource.hlsl");
}
TEST_F(FileTest, ConstantBufferType) {
runFileTest("type.constant-buffer.hlsl");
}
Expand Down

0 comments on commit 4c3d09c

Please sign in to comment.