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
