Add SPV_EXT_physical_storage_buffer to opt whitelists (#2779) This also fixes ADCE to not remove possibly needed OpTypeForwardPointer. The bug, its fix and the corresponding test have a circular dependency with the extension, so they are packaged together.
diff --git a/source/opt/aggressive_dead_code_elim_pass.cpp b/source/opt/aggressive_dead_code_elim_pass.cpp index 11a9574..04bfea1 100644 --- a/source/opt/aggressive_dead_code_elim_pass.cpp +++ b/source/opt/aggressive_dead_code_elim_pass.cpp
@@ -836,6 +836,15 @@ // attributes here. for (auto& val : get_module()->types_values()) { if (IsDead(&val)) { + // Save forwarded pointer if pointer is live since closure does not mark + // this live as it does not have a result id. This is a little too + // conservative since it is not known if the structure type that needed + // it is still live. TODO(greg-lunarg): Only save if needed. + if (val.opcode() == SpvOpTypeForwardPointer) { + uint32_t ptr_ty_id = val.GetSingleWordInOperand(0); + Instruction* ptr_ty_inst = get_def_use_mgr()->GetDef(ptr_ty_id); + if (!IsDead(ptr_ty_inst)) continue; + } to_kill_.push_back(&val); } } @@ -918,6 +927,7 @@ "SPV_NV_mesh_shader", "SPV_NV_ray_tracing", "SPV_EXT_fragment_invocation_density", + "SPV_EXT_physical_storage_buffer", }); }
diff --git a/source/opt/local_single_block_elim_pass.cpp b/source/opt/local_single_block_elim_pass.cpp index cc1b837..aebbd00 100644 --- a/source/opt/local_single_block_elim_pass.cpp +++ b/source/opt/local_single_block_elim_pass.cpp
@@ -256,6 +256,7 @@ "SPV_NV_mesh_shader", "SPV_NV_ray_tracing", "SPV_EXT_fragment_invocation_density", + "SPV_EXT_physical_storage_buffer", }); }
diff --git a/source/opt/local_single_store_elim_pass.cpp b/source/opt/local_single_store_elim_pass.cpp index f47777e..d6beeab 100644 --- a/source/opt/local_single_store_elim_pass.cpp +++ b/source/opt/local_single_store_elim_pass.cpp
@@ -119,6 +119,7 @@ "SPV_NV_mesh_shader", "SPV_NV_ray_tracing", "SPV_EXT_fragment_invocation_density", + "SPV_EXT_physical_storage_buffer", }); } bool LocalSingleStoreElimPass::ProcessVariable(Instruction* var_inst) {
diff --git a/source/opt/local_ssa_elim_pass.cpp b/source/opt/local_ssa_elim_pass.cpp index 299bbe0..c3f4ab6 100644 --- a/source/opt/local_ssa_elim_pass.cpp +++ b/source/opt/local_ssa_elim_pass.cpp
@@ -105,6 +105,7 @@ "SPV_NV_mesh_shader", "SPV_NV_ray_tracing", "SPV_EXT_fragment_invocation_density", + "SPV_EXT_physical_storage_buffer", }); }
diff --git a/test/opt/aggressive_dead_code_elim_test.cpp b/test/opt/aggressive_dead_code_elim_test.cpp index b4ab10d..3a7fc27 100644 --- a/test/opt/aggressive_dead_code_elim_test.cpp +++ b/test/opt/aggressive_dead_code_elim_test.cpp
@@ -6616,6 +6616,152 @@ SinglePassRunAndCheck<AggressiveDCEPass>(spirv, spirv, true); } +TEST_F(AggressiveDCETest, NoEliminateForwardPointer) { + // clang-format off + // + // #version 450 + // #extension GL_EXT_buffer_reference : enable + // + // // forward reference + // layout(buffer_reference) buffer blockType; + // + // layout(buffer_reference, std430, buffer_reference_align = 16) buffer blockType { + // int x; + // blockType next; + // }; + // + // layout(std430) buffer rootBlock { + // blockType root; + // } r; + // + // void main() + // { + // blockType b = r.root; + // b = b.next; + // b.x = 531; + // } + // + // clang-format on + + const std::string predefs1 = + R"(OpCapability Shader +OpCapability PhysicalStorageBufferAddressesEXT +OpExtension "SPV_EXT_physical_storage_buffer" +OpExtension "SPV_KHR_storage_buffer_storage_class" +%1 = OpExtInstImport "GLSL.std.450" +OpMemoryModel PhysicalStorageBuffer64EXT GLSL450 +OpEntryPoint GLCompute %main "main" +OpExecutionMode %main LocalSize 1 1 1 +OpSource GLSL 450 +OpSourceExtension "GL_EXT_buffer_reference" +)"; + + const std::string names_before = + R"(OpName %main "main" +OpName %blockType "blockType" +OpMemberName %blockType 0 "x" +OpMemberName %blockType 1 "next" +OpName %b "b" +OpName %rootBlock "rootBlock" +OpMemberName %rootBlock 0 "root" +OpName %r "r" +OpMemberDecorate %blockType 0 Offset 0 +OpMemberDecorate %blockType 1 Offset 8 +OpDecorate %blockType Block +OpDecorate %b AliasedPointerEXT +OpMemberDecorate %rootBlock 0 Offset 0 +OpDecorate %rootBlock Block +OpDecorate %r DescriptorSet 0 +OpDecorate %r Binding 0 +)"; + + const std::string names_after = + R"(OpName %main "main" +OpName %blockType "blockType" +OpMemberName %blockType 0 "x" +OpMemberName %blockType 1 "next" +OpName %rootBlock "rootBlock" +OpMemberName %rootBlock 0 "root" +OpName %r "r" +OpMemberDecorate %blockType 0 Offset 0 +OpMemberDecorate %blockType 1 Offset 8 +OpDecorate %blockType Block +OpMemberDecorate %rootBlock 0 Offset 0 +OpDecorate %rootBlock Block +OpDecorate %r DescriptorSet 0 +OpDecorate %r Binding 0 +)"; + + const std::string predefs2_before = + R"(%void = OpTypeVoid +%3 = OpTypeFunction %void +OpTypeForwardPointer %_ptr_PhysicalStorageBufferEXT_blockType PhysicalStorageBufferEXT +%int = OpTypeInt 32 1 +%blockType = OpTypeStruct %int %_ptr_PhysicalStorageBufferEXT_blockType +%_ptr_PhysicalStorageBufferEXT_blockType = OpTypePointer PhysicalStorageBufferEXT %blockType +%_ptr_Function__ptr_PhysicalStorageBufferEXT_blockType = OpTypePointer Function %_ptr_PhysicalStorageBufferEXT_blockType +%rootBlock = OpTypeStruct %_ptr_PhysicalStorageBufferEXT_blockType +%_ptr_StorageBuffer_rootBlock = OpTypePointer StorageBuffer %rootBlock +%r = OpVariable %_ptr_StorageBuffer_rootBlock StorageBuffer +%int_0 = OpConstant %int 0 +%_ptr_StorageBuffer__ptr_PhysicalStorageBufferEXT_blockType = OpTypePointer StorageBuffer %_ptr_PhysicalStorageBufferEXT_blockType +%int_1 = OpConstant %int 1 +%_ptr_PhysicalStorageBufferEXT__ptr_PhysicalStorageBufferEXT_blockType = OpTypePointer PhysicalStorageBufferEXT %_ptr_PhysicalStorageBufferEXT_blockType +%int_531 = OpConstant %int 531 +%_ptr_PhysicalStorageBufferEXT_int = OpTypePointer PhysicalStorageBufferEXT %int +)"; + + const std::string predefs2_after = + R"(%void = OpTypeVoid +%8 = OpTypeFunction %void +OpTypeForwardPointer %_ptr_PhysicalStorageBufferEXT_blockType PhysicalStorageBufferEXT +%int = OpTypeInt 32 1 +%blockType = OpTypeStruct %int %_ptr_PhysicalStorageBufferEXT_blockType +%_ptr_PhysicalStorageBufferEXT_blockType = OpTypePointer PhysicalStorageBufferEXT %blockType +%rootBlock = OpTypeStruct %_ptr_PhysicalStorageBufferEXT_blockType +%_ptr_StorageBuffer_rootBlock = OpTypePointer StorageBuffer %rootBlock +%r = OpVariable %_ptr_StorageBuffer_rootBlock StorageBuffer +%int_0 = OpConstant %int 0 +%_ptr_StorageBuffer__ptr_PhysicalStorageBufferEXT_blockType = OpTypePointer StorageBuffer %_ptr_PhysicalStorageBufferEXT_blockType +%int_1 = OpConstant %int 1 +%_ptr_PhysicalStorageBufferEXT__ptr_PhysicalStorageBufferEXT_blockType = OpTypePointer PhysicalStorageBufferEXT %_ptr_PhysicalStorageBufferEXT_blockType +%int_531 = OpConstant %int 531 +%_ptr_PhysicalStorageBufferEXT_int = OpTypePointer PhysicalStorageBufferEXT %int +)"; + + const std::string func_before = + R"(%main = OpFunction %void None %3 +%5 = OpLabel +%b = OpVariable %_ptr_Function__ptr_PhysicalStorageBufferEXT_blockType Function +%16 = OpAccessChain %_ptr_StorageBuffer__ptr_PhysicalStorageBufferEXT_blockType %r %int_0 +%17 = OpLoad %_ptr_PhysicalStorageBufferEXT_blockType %16 +%21 = OpAccessChain %_ptr_PhysicalStorageBufferEXT__ptr_PhysicalStorageBufferEXT_blockType %17 %int_1 +%22 = OpLoad %_ptr_PhysicalStorageBufferEXT_blockType %21 Aligned 8 +OpStore %b %22 +%26 = OpAccessChain %_ptr_PhysicalStorageBufferEXT_int %22 %int_0 +OpStore %26 %int_531 Aligned 16 +OpReturn +OpFunctionEnd +)"; + + const std::string func_after = + R"(%main = OpFunction %void None %8 +%19 = OpLabel +%20 = OpAccessChain %_ptr_StorageBuffer__ptr_PhysicalStorageBufferEXT_blockType %r %int_0 +%21 = OpLoad %_ptr_PhysicalStorageBufferEXT_blockType %20 +%22 = OpAccessChain %_ptr_PhysicalStorageBufferEXT__ptr_PhysicalStorageBufferEXT_blockType %21 %int_1 +%23 = OpLoad %_ptr_PhysicalStorageBufferEXT_blockType %22 Aligned 8 +%24 = OpAccessChain %_ptr_PhysicalStorageBufferEXT_int %23 %int_0 +OpStore %24 %int_531 Aligned 16 +OpReturn +OpFunctionEnd +)"; + + SinglePassRunAndCheck<AggressiveDCEPass>( + predefs1 + names_before + predefs2_before + func_before, + predefs1 + names_after + predefs2_after + func_after, true, true); +} + // TODO(greg-lunarg): Add tests to verify handling of these cases: // // Check that logical addressing required