ADCE: Treat privates like locals in entry point with no calls

This is needed for ongoing legalization of HLSL. It allows removal
of accesses to textures/buffers that are not used.

diff --git a/source/opt/aggressive_dead_code_elim_pass.cpp b/source/opt/aggressive_dead_code_elim_pass.cpp
index 4b06a417dea84a2e5f557e292a2a68507babe9a7..355865f56e23b0b91bdb6e5e52c671db5f0e4078 100644 (file)
--- a/source/opt/aggressive_dead_code_elim_pass.cpp
+++ b/source/opt/aggressive_dead_code_elim_pass.cpp
@@ -27,10 +27,12 @@ namespace {
 const uint32_t kTypePointerStorageClassInIdx = 0;
 const uint32_t kExtInstSetIdInIndx = 0;
 const uint32_t kExtInstInstructionInIndx = 1;
+const uint32_t kEntryPointFunctionIdInIdx = 1;
 
 }  // namespace anonymous
 
-bool AggressiveDCEPass::IsLocalVar(uint32_t varId) {
+bool AggressiveDCEPass::IsVarOfStorage(uint32_t varId, 
+      uint32_t storageClass) {
   const ir::Instruction* varInst = def_use_mgr_->GetDef(varId);
   const SpvOp op = varInst->opcode();
   if (op != SpvOpVariable) 
@@ -40,7 +42,12 @@ bool AggressiveDCEPass::IsLocalVar(uint32_t varId) {
   if (varTypeInst->opcode() != SpvOpTypePointer)
     return false;
   return varTypeInst->GetSingleWordInOperand(kTypePointerStorageClassInIdx) ==
-      SpvStorageClassFunction;
+      storageClass;
+}
+
+bool AggressiveDCEPass::IsLocalVar(uint32_t varId) {
+ return IsVarOfStorage(varId, SpvStorageClassFunction) ||
+        (IsVarOfStorage(varId, SpvStorageClassPrivate) && private_like_local_);
 }
 
 void AggressiveDCEPass::AddStores(uint32_t ptrId) {
@@ -121,6 +128,9 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
   // Add all control flow and instructions with external side effects 
   // to worklist
   // TODO(greg-lunarg): Handle Frexp, Modf more optimally
+  call_in_func_ = false;
+  func_is_entry_point_ = false;
+  private_stores_.clear();
   for (auto& blk : *func) {
     for (auto& inst : blk) {
       uint32_t op = inst.opcode();
@@ -128,10 +138,13 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
         case SpvOpStore: {
           uint32_t varId;
           (void) GetPtr(&inst, &varId);
-          // non-function-scope stores
-          if (!IsLocalVar(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))
+            private_stores_.push_back(&inst);
+          else if (!IsVarOfStorage(varId, SpvStorageClassFunction))
             worklist_.push(&inst);
-          }
         } break;
         case SpvOpExtInst: {
           // eg. GLSL frexp, modf
@@ -144,10 +157,28 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
           // TODO(greg-lunarg): function calls live only if write to non-local
           if (!IsCombinator(op))
             worklist_.push(&inst);
+          // Remember function calls
+          if (op == SpvOpFunctionCall)
+            call_in_func_ = true;
         } break;
       }
     }
   }
+  // See if current function is an entry point
+  for (auto& ei : module_->entry_points()) {
+    if (ei.GetSingleWordInOperand(kEntryPointFunctionIdInIdx) ==
+        func->result_id()) {
+      func_is_entry_point_ = true;
+      break;
+    }
+  }
+  // If the current function is an entry point and has no function calls,
+  // we can optimize private variables as locals
+  private_like_local_ = func_is_entry_point_ && !call_in_func_;
+  // If privates are not like local, add their stores to worklist
+  if (!private_like_local_)
+    for (auto& ps : private_stores_)
+      worklist_.push(ps);
   // Add OpGroupDecorates to worklist because they are a pain to remove
   // ids from.
   // TODO(greg-lunarg): Handle dead ids in OpGroupDecorate

diff --git a/source/opt/aggressive_dead_code_elim_pass.h b/source/opt/aggressive_dead_code_elim_pass.h
index 651d2fc0abb5173eb92ff77e1bf87db3a1388f57..5495c364102c55943582cd2face9c23fabb423d9 100644 (file)
--- a/source/opt/aggressive_dead_code_elim_pass.h
+++ b/source/opt/aggressive_dead_code_elim_pass.h
@@ -46,14 +46,18 @@ class AggressiveDCEPass : public MemPass {
   Status Process(ir::Module*) override;
 
  private:
+  // Return true if |varId| is variable of |storageClass|.
+  bool IsVarOfStorage(uint32_t varId, uint32_t storageClass);
+
+  // Return true if |varId| is variable of function storage class or is
+  // private variable and privates can be optimized like locals (see
+  // privates_like_local_)
+  bool IsLocalVar(uint32_t varId);
+
   // Add all store instruction which use |ptrId|, directly or indirectly,
   // to the live instruction worklist.
   void AddStores(uint32_t ptrId);
 
-  // Return true if object with |varId| is function scope variable or
-  // function parameter with pointer type.
-  bool IsLocalVar(uint32_t varId);
-
   // Initialize combinator data structures
   void InitCombinatorSets();
 
@@ -94,6 +98,15 @@ class AggressiveDCEPass : public MemPass {
   void Initialize(ir::Module* module);
   Pass::Status ProcessImpl();
 
+  // True if current function has a call instruction contained in it
+  bool call_in_func_;
+
+  // True if current function is an entry point
+  bool func_is_entry_point_;
+
+  // True if current function is entry point and has no function calls.
+  bool private_like_local_;
+
   // Live Instruction Worklist.  An instruction is added to this list
   // if it might have a side effect, either directly or indirectly.
   // If we don't know, then add it to this list.  Instructions are
@@ -101,6 +114,9 @@ class AggressiveDCEPass : public MemPass {
   // building up the live instructions set |live_insts_|.
   std::queue<ir::Instruction*> worklist_;
 
+  // Store instructions to variables of private storage
+  std::vector<ir::Instruction*> private_stores_;
+
   // Live Instructions
   std::unordered_set<const ir::Instruction*> live_insts_;
 

diff --git a/test/opt/aggressive_dead_code_elim_test.cpp b/test/opt/aggressive_dead_code_elim_test.cpp
index 85ba908a34fff02dc76eed4dd9ea0fd6bb0bcf9c..8e8b407f6a4eb6ab283dd170c44b348bd43d7239 100644 (file)
--- a/test/opt/aggressive_dead_code_elim_test.cpp
+++ b/test/opt/aggressive_dead_code_elim_test.cpp
@@ -1122,6 +1122,272 @@ OpFunctionEnd
       assembly, assembly, true, true);
 }
 
+TEST_F(AggressiveDCETest, PrivateStoreElimInEntryNoCalls) {
+  // 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;
+  //
+  // private vec4 dv;
+  // 
+  // void main()
+  // {
+  //     vec4 v = BaseColor;
+  //     dv = Dead;
+  //     OutColor = v;
+  // }
+
+  const std::string predefs =
+      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_Private_v4float = OpTypePointer Private %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_Private_v4float Private
+%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<opt::AggressiveDCEPass>(
+      predefs + main_before, predefs + main_after, true, true);
+}
+
+TEST_F(AggressiveDCETest, NoPrivateStoreElimIfLoad) {
+  // Should not eliminate stores to private when there is a load
+  // Note: Not legal GLSL
+  //
+  // #version 450
+  // 
+  // layout(location = 0) in vec4 BaseColor;
+  // layout(location = 0) out vec4 OutColor;
+  //
+  // private vec4 pv;
+  // 
+  // void main()
+  // {
+  //     pv = BaseColor;
+  //     OutColor = pv;
+  // }
+
+  const std::string assembly =
+      R"(OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main" %BaseColor %OutColor
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 450
+OpName %main "main"
+OpName %pv "pv"
+OpName %BaseColor "BaseColor"
+OpName %OutColor "OutColor"
+OpDecorate %BaseColor Location 0
+OpDecorate %OutColor Location 0
+%void = OpTypeVoid
+%7 = OpTypeFunction %void
+%float = OpTypeFloat 32
+%v4float = OpTypeVector %float 4
+%_ptr_Private_v4float = OpTypePointer Private %v4float
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%BaseColor = OpVariable %_ptr_Input_v4float Input
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%OutColor = OpVariable %_ptr_Output_v4float Output
+%pv = OpVariable %_ptr_Private_v4float Private
+%main = OpFunction %void None %7
+%13 = OpLabel
+%14 = OpLoad %v4float %BaseColor
+OpStore %pv %14
+%15 = OpLoad %v4float %pv
+%16 = OpFNegate %v4float %15
+OpStore %OutColor %16
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndCheck<opt::AggressiveDCEPass>(
+      assembly, assembly, true, true);
+}
+
+TEST_F(AggressiveDCETest, NoPrivateStoreElimWithCall) {
+  // Should not eliminate stores to private when function contains call
+  // Note: Not legal GLSL
+  //
+  // #version 450
+  // 
+  // layout(location = 0) in vec4 BaseColor;
+  // layout(location = 0) out vec4 OutColor;
+  //
+  // private vec4 v1;
+  // 
+  // void foo()
+  // {
+  //     OutColor = -v1;
+  // }
+  // 
+  // void main()
+  // {
+  //     v1 = BaseColor;
+  //     foo();
+  // }
+
+  const std::string assembly =
+      R"(OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main" %OutColor %BaseColor
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 450
+OpName %main "main"
+OpName %foo_ "foo("
+OpName %OutColor "OutColor"
+OpName %v1 "v1"
+OpName %BaseColor "BaseColor"
+OpDecorate %OutColor Location 0
+OpDecorate %BaseColor Location 0
+%void = OpTypeVoid
+%8 = OpTypeFunction %void
+%float = OpTypeFloat 32
+%v4float = OpTypeVector %float 4
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%OutColor = OpVariable %_ptr_Output_v4float Output
+%_ptr_Private_v4float = OpTypePointer Private %v4float
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%v1 = OpVariable %_ptr_Private_v4float Private
+%BaseColor = OpVariable %_ptr_Input_v4float Input
+%main = OpFunction %void None %8
+%14 = OpLabel
+%15 = OpLoad %v4float %BaseColor
+OpStore %v1 %15
+%16 = OpFunctionCall %void %foo_
+OpReturn
+OpFunctionEnd
+%foo_ = OpFunction %void None %8
+%17 = OpLabel
+%18 = OpLoad %v4float %v1
+%19 = OpFNegate %v4float %18
+OpStore %OutColor %19
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndCheck<opt::AggressiveDCEPass>(
+      assembly, assembly, true, true);
+}
+
+TEST_F(AggressiveDCETest, NoPrivateStoreElimInNonEntry) {
+  // Should not eliminate stores to private when function is not entry point
+  // Note: Not legal GLSL
+  //
+  // #version 450
+  // 
+  // layout(location = 0) in vec4 BaseColor;
+  // layout(location = 0) out vec4 OutColor;
+  //
+  // private vec4 v1;
+  // 
+  // void foo()
+  // {
+  //     v1 = BaseColor;
+  // }
+  // 
+  // void main()
+  // {
+  //     foo();
+  //     OutColor = -v1;
+  // }
+
+  const std::string assembly =
+      R"(OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main" %BaseColor %OutColor
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 450
+OpName %main "main"
+OpName %foo_ "foo("
+OpName %v1 "v1"
+OpName %BaseColor "BaseColor"
+OpName %OutColor "OutColor"
+OpDecorate %BaseColor Location 0
+OpDecorate %OutColor Location 0
+%void = OpTypeVoid
+%8 = OpTypeFunction %void
+%float = OpTypeFloat 32
+%v4float = OpTypeVector %float 4
+%_ptr_Private_v4float = OpTypePointer Private %v4float
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%BaseColor = OpVariable %_ptr_Input_v4float Input
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%v1 = OpVariable %_ptr_Private_v4float Private
+%OutColor = OpVariable %_ptr_Output_v4float Output
+%main = OpFunction %void None %8
+%14 = OpLabel
+%15 = OpFunctionCall %void %foo_
+%16 = OpLoad %v4float %v1
+%17 = OpFNegate %v4float %16
+OpStore %OutColor %17
+OpReturn
+OpFunctionEnd
+%foo_ = OpFunction %void None %8
+%18 = OpLabel
+%19 = OpLoad %v4float %BaseColor
+OpStore %v1 %19
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndCheck<opt::AggressiveDCEPass>(
+      assembly, assembly, true, true);
+}
+
 
 // TODO(greg-lunarg): Add tests to verify handling of these cases:
 //