From 9ec3f81e5c5de63b864c1d568ee9b125dffc3367 Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Tue, 8 May 2018 14:02:03 -0400 Subject: [PATCH] Remove dead Workgroup variables in ADCE. If there is a shader with a variable in the workgroup storage class that is stored to, but not loadeds, then we know nothing will read those loads. It should be safe to remove them. This is implemented in ADCE by treating workgroup variables the same way that private variables are treated. Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1550. --- source/opt/aggressive_dead_code_elim_pass.cpp | 17 ++- test/opt/aggressive_dead_code_elim_test.cpp | 106 ++++++++++++++++++ 2 files changed, 119 insertions(+), 4 deletions(-) diff --git a/source/opt/aggressive_dead_code_elim_pass.cpp b/source/opt/aggressive_dead_code_elim_pass.cpp index e43ba1b1d..5ea209001 100644 --- a/source/opt/aggressive_dead_code_elim_pass.cpp +++ b/source/opt/aggressive_dead_code_elim_pass.cpp @@ -91,8 +91,15 @@ bool AggressiveDCEPass::IsVarOfStorage(uint32_t varId, uint32_t storageClass) { } bool AggressiveDCEPass::IsLocalVar(uint32_t varId) { - return IsVarOfStorage(varId, SpvStorageClassFunction) || - (IsVarOfStorage(varId, SpvStorageClassPrivate) && private_like_local_); + if (IsVarOfStorage(varId, SpvStorageClassFunction)) { + return true; + } + if (!private_like_local_) { + return false; + } + + return IsVarOfStorage(varId, SpvStorageClassPrivate) || + IsVarOfStorage(varId, SpvStorageClassWorkgroup); } void AggressiveDCEPass::AddStores(uint32_t ptrId) { @@ -326,8 +333,10 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) { (void)GetPtr(&*ii, &varId); // Mark stores as live if their variable is not function scope // and is not private scope. Remember private stores for possible - // later inclusion - if (IsVarOfStorage(varId, SpvStorageClassPrivate)) + // later inclusion. We cannot call IsLocalVar at this point because + // private_like_local_ has not been set yet. + if (IsVarOfStorage(varId, SpvStorageClassPrivate) || + IsVarOfStorage(varId, SpvStorageClassWorkgroup)) private_stores_.push_back(&*ii); else if (!IsVarOfStorage(varId, SpvStorageClassFunction)) AddToWorklist(&*ii); diff --git a/test/opt/aggressive_dead_code_elim_test.cpp b/test/opt/aggressive_dead_code_elim_test.cpp index b888be573..372883a02 100644 --- a/test/opt/aggressive_dead_code_elim_test.cpp +++ b/test/opt/aggressive_dead_code_elim_test.cpp @@ -1268,6 +1268,112 @@ OpFunctionEnd SinglePassRunAndCheck(assembly, assembly, true, true); } +TEST_F(AggressiveDCETest, WorkgroupStoreElimInEntryNoCalls) { + // Eliminate stores to private in entry point with no calls + // Note: Not legal GLSL + // + // layout(location = 0) in vec4 BaseColor; + // layout(location = 1) in vec4 Dead; + // layout(location = 0) out vec4 OutColor; + // + // workgroup vec4 dv; + // + // void main() + // { + // vec4 v = BaseColor; + // dv = Dead; + // OutColor = v; + // } + + const std::string predefs_before = + R"(OpCapability Shader +%1 = OpExtInstImport "GLSL.std.450" +OpMemoryModel Logical GLSL450 +OpEntryPoint Fragment %main "main" %BaseColor %Dead %OutColor +OpExecutionMode %main OriginUpperLeft +OpSource GLSL 450 +OpName %main "main" +OpName %v "v" +OpName %BaseColor "BaseColor" +OpName %dv "dv" +OpName %Dead "Dead" +OpName %OutColor "OutColor" +OpDecorate %BaseColor Location 0 +OpDecorate %Dead Location 1 +OpDecorate %OutColor Location 0 +%void = OpTypeVoid +%9 = OpTypeFunction %void +%float = OpTypeFloat 32 +%v4float = OpTypeVector %float 4 +%_ptr_Function_v4float = OpTypePointer Function %v4float +%_ptr_Workgroup_v4float = OpTypePointer Workgroup %v4float +%_ptr_Input_v4float = OpTypePointer Input %v4float +%BaseColor = OpVariable %_ptr_Input_v4float Input +%Dead = OpVariable %_ptr_Input_v4float Input +%_ptr_Output_v4float = OpTypePointer Output %v4float +%dv = OpVariable %_ptr_Workgroup_v4float Workgroup +%OutColor = OpVariable %_ptr_Output_v4float Output +)"; + + const std::string predefs_after = + R"(OpCapability Shader +%1 = OpExtInstImport "GLSL.std.450" +OpMemoryModel Logical GLSL450 +OpEntryPoint Fragment %main "main" %BaseColor %Dead %OutColor +OpExecutionMode %main OriginUpperLeft +OpSource GLSL 450 +OpName %main "main" +OpName %v "v" +OpName %BaseColor "BaseColor" +OpName %Dead "Dead" +OpName %OutColor "OutColor" +OpDecorate %BaseColor Location 0 +OpDecorate %Dead Location 1 +OpDecorate %OutColor Location 0 +%void = OpTypeVoid +%9 = OpTypeFunction %void +%float = OpTypeFloat 32 +%v4float = OpTypeVector %float 4 +%_ptr_Function_v4float = OpTypePointer Function %v4float +%_ptr_Input_v4float = OpTypePointer Input %v4float +%BaseColor = OpVariable %_ptr_Input_v4float Input +%Dead = OpVariable %_ptr_Input_v4float Input +%_ptr_Output_v4float = OpTypePointer Output %v4float +%OutColor = OpVariable %_ptr_Output_v4float Output +)"; + + const std::string main_before = + R"(%main = OpFunction %void None %9 +%16 = OpLabel +%v = OpVariable %_ptr_Function_v4float Function +%17 = OpLoad %v4float %BaseColor +OpStore %v %17 +%18 = OpLoad %v4float %Dead +OpStore %dv %18 +%19 = OpLoad %v4float %v +%20 = OpFNegate %v4float %19 +OpStore %OutColor %20 +OpReturn +OpFunctionEnd +)"; + + const std::string main_after = + R"(%main = OpFunction %void None %9 +%16 = OpLabel +%v = OpVariable %_ptr_Function_v4float Function +%17 = OpLoad %v4float %BaseColor +OpStore %v %17 +%19 = OpLoad %v4float %v +%20 = OpFNegate %v4float %19 +OpStore %OutColor %20 +OpReturn +OpFunctionEnd +)"; + + SinglePassRunAndCheck( + predefs_before + main_before, predefs_after + main_after, true, true); +} + TEST_F(AggressiveDCETest, EliminateDeadIfThenElse) { // #version 450 //