Shader translation from SPIR-V fails targeting DX12

[radgeRayden]

I have previously filed an issue about this for wgpu-native: gfx-rs/wgpu-native#274
I also have an even smaller trace, it's just a very basic shader that indexes into a Storage Buffer. I've extracted the HLSL output for convenience, but the complete wgpu trace is below.
wgpu-trace.txt

Also the original shader (in SPIR-V form):
shader.zip

[teoxoy]

CreateGraphicsPipelineState is returning E_INVALIDARG which doesn't tell us much.

Could you post the trace of a debug build? We should get more info that way since the D3D12 debug layers are only enabled if cfg!(debug_assertions).

[radgeRayden]

CreateGraphicsPipelineState is returning E_INVALIDARG which doesn't tell us much.

Could you post the trace of a debug build? We should get more info that way since the D3D12 debug layers are only enabled if cfg!(debug_assertions).

wgpu-trace.txt

(edited with non cutoff log)

[teoxoy]

From the log, these are the expected bindings for the Vertex shader:

var GlobalVariable { name: Some("input-data"), space: Storage { access: LOAD | STORE }, binding: Some(ResourceBinding { group: 0, binding: 1 }), ty: [6], init: None }
var GlobalVariable { name: Some("uniforms"), space: Uniform, binding: Some(ResourceBinding { group: 0, binding: 0 }), ty: [13], init: None }

What are the bindings you are passing to wgpu?

[radgeRayden]

From the log, these are the expected bindings for the Vertex shader:

var GlobalVariable { name: Some("input-data"), space: Storage { access: LOAD | STORE }, binding: Some(ResourceBinding { group: 0, binding: 1 }), ty: [6], init: None }
var GlobalVariable { name: Some("uniforms"), space: Uniform, binding: Some(ResourceBinding { group: 0, binding: 0 }), ty: [13], init: None }

What are the bindings you are passing to wgpu?

Uniform Buffer at 0 and Storage Buffer at 1. Like I mentioned (in the other issue), my code works as expected on Vulkan.

[teoxoy]

For the storage buffer, is it a BufferBindingType::Storage { read_only: true } (i.e. is it read only on the API side)?

[radgeRayden]

Currently I am using auto layouts, so it seems like I don't get to set that. The Buffer usage is wgpu.BufferUsage.Storage | wgpu.BufferUsage.CopyDst | wgpu.BufferUsage.CopySrc.

[teoxoy]

What is happening here is that we derive the layout via how the global is used in the shader instead of how it's declared (in the shader).

wgpu/wgpu-core/src/validation.rs

Line 566 in 8a8e00a

read_only: !shader_usage.contains(GlobalUse::WRITE),

Relevant Spec Text:

This works fine for all other backends but not the dx12 backend as we will use either an SRV or an UAV for the buffer depending if it's read-only or not.

wgpu/wgpu-hal/src/dx12/conv.rs

Lines 105 to 114 in 8a8e00a

	Bt::Buffer {
	ty: wgt::BufferBindingType::Storage { read_only: true },
	..
	}
	| Bt::Texture { .. } => d3d12::DescriptorRangeType::SRV,
	Bt::Buffer {
	ty: wgt::BufferBindingType::Storage { read_only: false },
	..
	}
	| Bt::StorageTexture { .. } => d3d12::DescriptorRangeType::UAV,

Conclusion

• You should add the NonWritable SPIR-V decoration to the storage buffer as the vertex stage only supports read-only storage buffers (without the VERTEX_WRITABLE_STORAGE wgpu feature enabled), which will in turn avoid the underlying issues and get things running.
• We need to infer the binding type based on how it's declared in the shader instead of how it's used.

[radgeRayden]

Unfortunately after adding the NonWritable decoration to the buffer, I still get the same error. Maybe our generator is wrong, I can't tell yet (not an SPIR-V expert). Here's the shader disassembly followed by the trace.
wgpu-trace.txt

[teoxoy]

I see that the NonWritable decoration got added to the members of VertexAttributes and VertexAttributesBuffer. While that is allowed by the SPIR-V spec, naga will only take into account NonWritable decorations on the variable input-data itself.

For reference:

from https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#Decoration

[radgeRayden]

Thank you, I made the change you suggested and it worked.