wgpu icon indicating copy to clipboard operation
wgpu copied to clipboard

Fix shaderf16 support on vulkan/nvidia

Open cryvosh opened this issue 6 months ago • 3 comments

Connections N/A

Description On my Laptop 4090, vulkaninfo reports:

VkPhysicalDeviceVulkan11Features:
---------------------------------
storageBuffer16BitAccess = true
uniformAndStorageBuffer16BitAccess = true
storagePushConstant16 = true
storageInputOutput16 = false

However the wgpu vulkan backend requires all four to be true for the SHADER_F16 feature to be supported. This doesn't seem necessary, and as far as I can tell, storage_input_output16 isn't even read anywhere in wgpu.

This is also how dawn handles f16 support checking here.

Testing My personal project using f16s (developed in chrome wasm which doesn't have this issue) was tested and works fine on Windows 11 native, Vulkan Backend.

Squash or Rebase? Squash

Checklist

  • [x] Run cargo fmt.
  • [ ] Run taplo format.
  • [ ] Run cargo clippy --tests. If applicable, add:
    • [ ] --target wasm32-unknown-unknown
  • [ ] Run cargo xtask test to run tests.
  • [ ] If this contains user-facing changes, add a CHANGELOG.md entry.

cryvosh avatar May 01 '25 21:05 cryvosh

storage_input_output16 is somehow implicitly used here: https://github.com/gfx-rs/wgpu/blob/50eb207a77963a3818e8d05f65da34f1f6e6018c/naga/src/back/spv/writer.rs#L1207

As for Dawn https://issues.chromium.org/issues/42251212:

Tint can now polyfill f16 shader IO by using f32 types, so we can enable the F16 feature on Vulkan without this capability.

I think naga also does pollyfill somewhere.

sagudev avatar May 02 '25 06:05 sagudev

Any idea where would be the best place to add the polyfill?

cryvosh avatar May 03 '25 10:05 cryvosh

Related: https://github.com/gfx-rs/wgpu/issues/7468

LegNeato avatar May 24 '25 20:05 LegNeato

This fixes an issue I had when trying to profile code using f16 in Nsight.

tychedelia avatar Jun 29 '25 21:06 tychedelia

@cryvosh can we take this PR out of draft, and leave pollyfilling to a followup?

JMS55 avatar Jul 01 '25 01:07 JMS55

All of the Vulkan features are required for the f16 feature. Dawn gets away with not requiring storageInputOutput16 since they polyfill the functionality, naga doesn't currently have this polyfill.

teoxoy avatar Jul 01 '25 11:07 teoxoy

Landing this PR as is would make the functionality of the SHADER_F16 feature non-portable. If we want to not require storageInputOutput16 we can do so under a new feature SHADER_F16_WO_IO until naga can do the polyfill.

teoxoy avatar Jul 01 '25 11:07 teoxoy

CI seems to correctly catch this as a vulkan validation error.

cwfitzgerald avatar Jul 01 '25 17:07 cwfitzgerald

As this isn't actually allowed, I'm going to close this PR - we would accept a followup which adds the polyfill using f32.

cwfitzgerald avatar Jul 01 '25 17:07 cwfitzgerald

For future reference, what's not allowed?

cryvosh avatar Jul 01 '25 17:07 cryvosh

Basically, if you use f16 as part of the input or output of a shader (like Vertex input or Fragment shader input) you need the input/output capability. However you can polyfill it by using f32, we just have not implemented this.

cwfitzgerald avatar Jul 01 '25 18:07 cwfitzgerald

Oh gotcha, I thought you meant I broke some rule

I'll look into adding the polyfill and open new PR when I get the chance

(in meantime any suggestions on where / at what stage in the pipeline to add this would be appreciated)

cryvosh avatar Jul 01 '25 18:07 cryvosh