Repository navigation
fix(shader,agc): declare ImageMSArray only for multisampled arrayed storage images - #9978
BKlaasWerkman wants to merge 1 commit into
Conversation
…torage images SPIR-V requires ImageMSArray only for an OpTypeImage with MS 1, Arrayed 1 and Sampled 2, and Vulkan requires shaderStorageImageMultisample for a module that declares it. The recompiler declared it for every multisampled arrayed image, including sampled ones, and the device listed it as supported without the feature, so a sampled Dim2DMsaaArray load produced an invalid module on devices without shaderStorageImageMultisample (VUID-VkShaderModuleCreateInfo-pCode-08740). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The source change is consistent with the SPIR-V condition cited in the PR: ImageMSArray is emitted only for multisampled arrayed storage images, after checking StorageImageMultisample, and the device advertises the related capabilities only when that feature is enabled. The new test inspects the generated module and fails if sampled multisampled image_load declares ImageMSArray. The two-GPU validation comparison reports the original VUID-08740 removed on Intel Arc, with the remaining failures/VUIDs identified on both baseline and change. Required conventions/intake/labels pass; hosted product jobs are not run automatically under repository policy.
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
Ran this on Apple silicon, a third vendor and a second device without shaderStorageImageMultisample (MoltenVK reports it as false on the M2 Max, like the Intel Arc in the description).
Setup: Apple M2 Max (32 GB), macOS 27.2, x86_64 build under Rosetta, Release, BUILD_TESTING=ON. Method: #8658 (feat/macos-port) @ dc0c8db500 plus this PR's diff (head 6d85dd2b5b), not main, since main does not build on macOS yet. Two drivers: Vulkan SDK 1.4.363.0 MoltenVK 1.4.2, and MoltenVK main with SPIRV-Cross #2704. ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1, VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, ctest -j1 --timeout 30 -V.
Focused set: ctest -R 'multisample|msaa|sample_order|storage_image|image_load|image_store|shader_device_profile' (42 entries).
| SDK MoltenVK 1.4.2 | MoltenVK main + #2704 | |
|---|---|---|
| test change only (red) | 38/42; agc_driver_native_msaa_vertex_input fails with the multisampled image_load declared ImageMSArray, which only multisampled arrayed storage images need |
same |
| this PR (green) | 39/42 | 39/42 |
- The 3 failures left on both sides are
agc_driver_multisample_retirement,agc_driver_sample_mask_multisampleandagc_driver_sample_order. They fail the same way on the #8658 base without this PR, so they are not from this change. - The tests that need
shaderStorageImageMultisampleskip on both sides (agc_driver_sample_multisample_target,agc_driver_multisample_centered_lines, the sample-order checkerboard). - Validation layer: the same VUID counts on both sides (04451 from the port's missing
VK_KHR_portability_subset, 03320, 00337, 09582). The layer reported no08740on MoltenVK, either with or without the fix. - Whole
^agcsuite with this PR, SDK driver: 852/870, the same 18 failures as the base.
GPUs tested: Apple M2 Max (MoltenVK 1.4.2 from Vulkan SDK 1.4.363.0, and MoltenVK main + SPIRV-Cross #2704), macOS 27.2.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Third-vendor column for the paired protocol — AMD RADV, where neither of the two GPUs above sits: Machine: CachyOS, kernel 7.2.10, GCC 16.2.1, Ninja, Release, Paired run —
Old code, new test — AI-assisted: yes (OpenCode); both runs and the old-code cross-check were executed on this machine. |
|
Rechecked on today's
Full AI-assisted: yes (Claude Code) Edited: the |
|
Tested on AMD under Windows, the case this PR is about (the driver does not expose
The one failure is the same on both: AI-assisted: yes |
What
The recompiler declared
ImageMSArrayfor every multisampled arrayed image, sampled ones included, and the device listedImageMSArrayas supported whether or notshaderStorageImageMultisampleis enabled. On a device without that feature, a sampledDim2DMsaaArrayimage_loadtherefore produced a module that Vulkan rejects. The Khronos layer reports it inagc_driver_native_msaa_vertex_inputon Intel Arc (Meteor Lake):Sources:
ImageMSArrayonly for anOpTypeImagewithMS1,Arrayed1 andSampled2. SPIRV-Tools at the pinned submodule commit checks exactly that: validate_image.cpp L699-704.shaderStorageImageMultisamplefor a module that declaresImageMSArray(VUID-VkShaderModuleCreateInfo-pCode-08740 and the SPIR-V environment capabilities table it refers to).Changes:
SpirvDescriptors.cpp:ImageMSArrayis declared only in the storage-image branch, after the existingStorageImageMultisamplecheck, so a storage multisampled array still fails withstorage image multisampling is unavailable on the target deviceon such a device.VulkanDevice.cpp:ImageMSArrayjoins the supported capabilities only whenshaderStorageImageMultisampleis enabled, likeStorageImageMultisample.SampleMultisampleTarget.cpp:RunLoadchecks that the compiled multisampledimage_loaddeclares noImageMSArray. A sampled load is device-independent (LoadImageis alwaysImageResourceClass::Sampled), so the check fails on every device, lavapipe included, if the capability comes back.Tested
Windows 11 Pro 10.0.26300, WinLibs GCC 15.2.0 r7 (UCRT), Release,
-DBUILD_TESTING=ON. Two GPUs in the same machine, selected withANYPS5_GPU:shaderStorageImageMultisample= true.0x7d55), driver 101.9150;shaderStorageImageMultisample= false.Base
maina701f9f, change 6d85dd2. 62 focused tests (ctest -R 'msaa|multisample|sample_order|sampled_cache|shader_device_profile|recompiler|spirv|image_load|image_store|storage_image|agc_shader_memory|native_depth' -V) withANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation(layer 1.4.363). Neither log haswas not found but was requested by env var VK_INSTANCE_LAYERS.main03320×3,00337×903320×3,00337×908740×1,03320×3,00337×603320×3,00337×608740is gone on Intel Arc; no new failure orVUID-on either GPU. Counts are validation messages (corrected; an earlier version of this table counted each message twice).agc_driver_sample_order,_grid_referenceand_recipe_depth_locations(AGC: agc_driver_sample_order tests read a depth target as all zeros on an NVIDIA Windows device and pass on Intel #9313, fixed by test(agc): read the sample order tests' depth through a sampled copy #9056); on Intel Arcagc_driver_multisample_retirement(test(agc): skip the MSAA retirement storage promotion without shaderStorageImageMultisample #8604).00337and03320come from the sample order tests (test(agc): read the sample order tests' depth through a sampled copy #9056, fix(agc): give a depth stencil copy's layout barrier both aspects #9057).shaderStorageImageMultisample.maina701f9f,agc_driver_native_msaa_vertex_inputfails on both GPUs withthe multisampled image_load declared ImageMSArray, which only multisampled arrayed storage images need, andagc_driver_sample_multisample_targetfails on the RTX 3060 Ti.python tools/check_conventions.py --base origin/mainpasses.tools/pr_overlap.pyreports only shared files with pull requests that conflict withmain; none of their diffs changesImageMSArray.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code