Optimize SPV_EXT_opacity_micromap and SPV_EXT_shader_invocation_reorder#6571
Optimize SPV_EXT_opacity_micromap and SPV_EXT_shader_invocation_reorder#6571s-perron merged 3 commits intoKhronosGroup:mainfrom
SPV_EXT_opacity_micromap and SPV_EXT_shader_invocation_reorder#6571Conversation
SPV_EXT_opacity_micromap and SPV_EXT_shader_invocation_reorder
|
After adding what I believe to be erroneously missing diff --git a/source/opt/local_single_block_elim_pass.cpp b/source/opt/local_single_store_elim_pass.cpp
index 021aaca19..df35401eb 100644
--- a/source/opt/local_single_block_elim_pass.cpp
+++ b/source/opt/local_single_store_elim_pass.cpp
@@ -269,7 +122,6 @@ void LocalSingleBlockLoadStoreElimPass::InitExtensions() {
"SPV_GOOGLE_hlsl_functionality1",
"SPV_GOOGLE_user_type",
"SPV_NV_shader_subgroup_partitioned",
- "SPV_EXT_demote_to_helper_invocation",
"SPV_EXT_descriptor_indexing",
"SPV_EXT_descriptor_heap",
"SPV_NV_fragment_shader_barycentric",diff --git a/source/opt/local_single_block_elim_pass.cpp b/source/opt/local_access_chain_convert_pass.cpp
index 021aaca19..d604777a0 100644
--- a/source/opt/local_single_block_elim_pass.cpp
+++ b/source/opt/local_access_chain_convert_pass.cpp
@@ -255,7 +419,8 @@ void LocalSingleBlockLoadStoreElimPass::InitExtensions() {
"SPV_NV_geometry_shader_passthrough",
"SPV_AMD_texture_gather_bias_lod",
"SPV_KHR_storage_buffer_storage_class",
- "SPV_KHR_variable_pointers",
+ // SPV_KHR_variable_pointers
+ // Currently do not support extended pointer expressions
"SPV_AMD_gpu_shader_int16",
"SPV_KHR_post_depth_coverage",
"SPV_KHR_shader_atomic_counter_ops",
@@ -282,8 +447,6 @@ void LocalSingleBlockLoadStoreElimPass::InitExtensions() {
"SPV_KHR_ray_tracing",
"SPV_KHR_ray_query",
"SPV_EXT_fragment_invocation_density",
- "SPV_EXT_physical_storage_buffer",
- "SPV_KHR_physical_storage_buffer",
"SPV_KHR_terminate_invocation",
"SPV_KHR_shader_clock",
"SPV_KHR_subgroup_uniform_control_flow",diff --git a/source/opt/local_single_block_elim_pass.cpp b/source/opt/aggressive_dead_code_elim_pass.cpp
index 021aaca19..b8f239d2e 100644
--- a/source/opt/local_single_block_elim_pass.cpp
+++ b/source/opt/aggressive_dead_code_elim_pass.cpp
@@ -255,7 +1105,8 @@ void LocalSingleBlockLoadStoreElimPass::InitExtensions() {
"SPV_NV_geometry_shader_passthrough",
"SPV_AMD_texture_gather_bias_lod",
"SPV_KHR_storage_buffer_storage_class",
- "SPV_KHR_variable_pointers",
+ // SPV_KHR_variable_pointers
+ // Currently do not support extended pointer expressions
"SPV_AMD_gpu_shader_int16",
"SPV_KHR_post_depth_coverage",
"SPV_KHR_shader_atomic_counter_ops",
@@ -300,7 +1151,6 @@ void LocalSingleBlockLoadStoreElimPass::InitExtensions() {
"SPV_NV_cooperative_matrix",
"SPV_KHR_cooperative_matrix",
"SPV_KHR_ray_tracing_position_fetch",
- "SPV_AMDX_shader_enqueue",
"SPV_KHR_fragment_shading_rate",
"SPV_KHR_quad_control",
"SPV_NV_shader_invocation_reorder", |
fe3462a to
5d12867
Compare
|
|
That is a good question. This was a decision we made when the project first started. Those passes make very important assumptions about pointers. We assume no pointer aliasing, and that pointers do not escape. However, there are a handleful of capabilities and extensions that allow the spir-v to breaks those assumptions. We feel that it is easier to identify cases where the passes are not run than to identify when a new extension is added to spir-v that invalidates our assumptions. In the latter case, people will get get an error at run time because their variables do not seem to have the correct values. It will only happen in select situations. It will be very hard to debug. That is why we have it the way we do, even if it is a annoying how often people ignore updating this list when updating the validator for their new extensions. |
|
Thanks for the info. Just fwiw, these extensions weren't tested well yet and at least my custom And do you think we need to annotate the remaining differences between the optimizations, explaining why they're intentionally omitted in some places (like |
It's been a while since I last added extensions to these lists, but I just ran into a long-forgotten related issue again that DXC outputs invalid SPIR-V before it's stripped away by
spirv-opt: microsoft/DirectXShaderCompiler#8210.Shaders with these extensions weren't yet extensively tested but this at least gets us past
spir-valin both DXC (can also be disabled with-Vd) and the validation layers.I'm probably way out of my league to ask, but is there still much significance to
extensions_allowlist_? It seems like a maintenance burden that's very easy to miss, for (currently) a single extension that's specifically commented out to not be optimized.It's also mostly duplicated but not entirely in sync across all 4 passes either. I even noticed that
SPV_KHR_shader_clockwhich I added todead_code_elim_passin #4049 isn't in any other file, which must have been an oversight on my part but also means that presence of this extension erroneously disables all the other 3 passes.