ADCE: Add support for function calls

ADCE will now generate correct code in the presence of function calls.
This is needed for opaque type optimization needed by glslang. Currently
all function calls are marked as live. TODO: mark calls live only if they
write a non-local.

include/spirv-tools/optimizer.hpp

@@ -376,18 +376,6 @@ Optimizer::PassToken CreateCommonUniformElimPass();
 // eliminated with standard dead code elimination.
 Optimizer::PassToken CreateAggressiveDCEPass();
 
-// Creates a pass to consolidate uniform references.
-// For each entry point function in the module, first change all constant index
-// access chain loads into equivalent composite extracts. Then consolidate 
-// identical uniform loads into one uniform load. Finally, consolidate
-// identical uniform extracts into one uniform extract. This may require
-// moving a load or extract to a point which dominates all uses.
-//
-// This pass requires a module to have structured control flow ie shader
-// capability. It also requires logical addressing ie Addresses capability
-// is not enabled. It also currently does not support any extensions.
-Optimizer::PassToken CreateCommonUniformElimPass();
-
 // Creates a compact ids pass.
 // The pass remaps result ids to a compact and gapless range starting from %1.
 Optimizer::PassToken CreateCompactIdsPass();

source/opt/aggressive_dead_code_elim_pass.cpp

@@ -54,7 +54,7 @@ void AggressiveDCEPass::AddStores(uint32_t ptrId) {
       } break;
       case SpvOpLoad:
         break;
-      // Assume it stores eg frexp, modf
+      // Assume it stores eg frexp, modf, function call
       case SpvOpStore:
       default: {
         if (live_insts_.find(u.inst) == live_insts_.end())
@@ -90,11 +90,27 @@ bool AggressiveDCEPass::AllExtensionsSupported() const {
   return true;
 }
 
-void AggressiveDCEPass::KillInstIfTargetDead(ir::Instruction* inst) {
+bool AggressiveDCEPass::KillInstIfTargetDead(ir::Instruction* inst) {
   const uint32_t tId = inst->GetSingleWordInOperand(0);
   const ir::Instruction* tInst = def_use_mgr_->GetDef(tId);
-  if (dead_insts_.find(tInst) != dead_insts_.end())
+  if (dead_insts_.find(tInst) != dead_insts_.end()) {
     def_use_mgr_->KillInst(inst);
+    return true;
+  }
+  return false;
+}
+
+void AggressiveDCEPass::ProcessLoad(uint32_t varId) {
+  // Only process locals
+  if (!IsLocalVar(varId))
+    return;
+  // Return if already processed
+  if (live_local_vars_.find(varId) != live_local_vars_.end()) 
+    return;
+  // Mark all stores to varId as live
+  AddStores(varId);
+  // Cache varId as processed
+  live_local_vars_.insert(varId);
 }
 
 bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
@@ -102,8 +118,6 @@ 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
-  // TODO(greg-lunarg): Handle FunctionCall more optimally
-  // TODO(greg-lunarg): Handle CopyMemory more optimally
   for (auto& blk : *func) {
     for (auto& inst : blk) {
       uint32_t op = inst.opcode();
@@ -121,12 +135,9 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
           if (!IsCombinatorExt(&inst))
             worklist_.push(&inst);
         } break;
-        case SpvOpCopyMemory:
-        case SpvOpFunctionCall: {
-          return false;
-        } break;
         default: {
           // eg. control flow, function call, atomics
+          // TODO(greg-lunarg): function calls live only if write to non-local
           if (!IsCombinator(op))
             worklist_.push(&inst);
         } break;
@@ -154,12 +165,17 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
     if (liveInst->opcode() == SpvOpLoad) {
       uint32_t varId;
       (void) GetPtr(liveInst, &varId);
-      if (IsLocalVar(varId)) {
-        if (live_local_vars_.find(varId) == live_local_vars_.end()) {
-          AddStores(varId);
-          live_local_vars_.insert(varId);
-        }
-      }
+      ProcessLoad(varId);
+    }
+    // If function call, treat as if it loads from all pointer arguments
+    else if (liveInst->opcode() == SpvOpFunctionCall) {
+      liveInst->ForEachInId([this](const uint32_t* iid) {
+        // Skip non-ptr args
+        if (!IsPtr(*iid)) return;
+        uint32_t varId;
+        (void) GetPtr(*iid, &varId);
+        ProcessLoad(varId);
+      });
     }
     worklist_.pop();
   }
@@ -177,14 +193,14 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
   for (auto& di : module_->debugs()) {
     if (di.opcode() != SpvOpName)
       continue;
-    KillInstIfTargetDead(&di);
-    modified = true;
+    if (KillInstIfTargetDead(&di))
+      modified = true;
   }
   for (auto& ai : module_->annotations()) {
     if (ai.opcode() != SpvOpDecorate && ai.opcode() != SpvOpDecorateId)
       continue;
-    KillInstIfTargetDead(&ai);
-    modified = true;
+    if (KillInstIfTargetDead(&ai))
+      modified = true;
   }
   // Kill dead instructions
   for (auto& blk : *func) {

source/opt/aggressive_dead_code_elim_pass.h

@@ -72,8 +72,12 @@ class AggressiveDCEPass : public MemPass {
   // Return true if all extensions in this module are supported by this pass.
   bool AllExtensionsSupported() const;
 
-  // Kill debug or annotation |inst| if target operand is dead.
-  void KillInstIfTargetDead(ir::Instruction* inst);
+  // Kill debug or annotation |inst| if target operand is dead. Return true
+  // if inst killed.
+  bool KillInstIfTargetDead(ir::Instruction* inst);
+
+  // If |varId| is local, mark all stores of varId as live.
+  void ProcessLoad(uint32_t varId);
 
   // For function |func|, mark all Stores to non-function-scope variables
   // and block terminating instructions as live. Recursively mark the values

source/opt/mem_pass.cpp

@@ -73,12 +73,20 @@ bool MemPass::IsNonPtrAccessChain(const SpvOp opcode) const {
   return opcode == SpvOpAccessChain || opcode == SpvOpInBoundsAccessChain;
 }
 
+bool MemPass::IsPtr(uint32_t ptrId) {
+  uint32_t varId = ptrId;
+  ir::Instruction* ptrInst = def_use_mgr_->GetDef(varId);
+  while (ptrInst->opcode() == SpvOpCopyObject) {
+    varId = ptrInst->GetSingleWordInOperand(kCopyObjectOperandInIdx);
+    ptrInst = def_use_mgr_->GetDef(varId);
+  }
+  const SpvOp op = ptrInst->opcode();
+  return op == SpvOpVariable || IsNonPtrAccessChain(op);
+}
+
 ir::Instruction* MemPass::GetPtr(
-      ir::Instruction* ip, uint32_t* varId) {
-  const SpvOp op = ip->opcode();
-  assert(op == SpvOpStore || op == SpvOpLoad);
-  *varId = ip->GetSingleWordInOperand(
-      op == SpvOpStore ? kStorePtrIdInIdx : kLoadPtrIdInIdx);
+      uint32_t ptrId, uint32_t* varId) {
+  *varId = ptrId;
   ir::Instruction* ptrInst = def_use_mgr_->GetDef(*varId);
   while (ptrInst->opcode() == SpvOpCopyObject) {
     *varId = ptrInst->GetSingleWordInOperand(kCopyObjectOperandInIdx);
@@ -98,6 +106,15 @@ ir::Instruction* MemPass::GetPtr(
   return ptrInst;
 }
 
+ir::Instruction* MemPass::GetPtr(
+      ir::Instruction* ip, uint32_t* varId) {
+  const SpvOp op = ip->opcode();
+  assert(op == SpvOpStore || op == SpvOpLoad);
+  const uint32_t ptrId = ip->GetSingleWordInOperand(
+      op == SpvOpStore ? kStorePtrIdInIdx : kLoadPtrIdInIdx);
+  return GetPtr(ptrId, varId);
+}
+
 bool MemPass::IsTargetVar(uint32_t varId) {
   if (seen_non_target_vars_.find(varId) != seen_non_target_vars_.end())
     return false;

source/opt/mem_pass.h

@@ -53,6 +53,14 @@ class MemPass : public Pass {
   // Returns true if |opcode| is a non-ptr access chain op
   bool IsNonPtrAccessChain(const SpvOp opcode) const;
 
+  // Given the id |ptrId|, return true if the top-most non-CopyObj is
+  // a pointer. 
+  bool IsPtr(uint32_t ptrId);
+
+  // Given the id of a pointer |ptrId|, return the top-most non-CopyObj.
+  // Also return the base variable's id in |varId|.
+  ir::Instruction* GetPtr(uint32_t ptrId, uint32_t* varId);
+
   // Given a load or store |ip|, return the pointer instruction.
   // Also return the base variable's id in |varId|.
   ir::Instruction* GetPtr(ir::Instruction* ip, uint32_t* varId);

test/opt/aggressive_dead_code_elim_test.cpp

@@ -680,6 +680,133 @@ OpFunctionEnd
       assembly, assembly, true, true);
 }
 
+TEST_F(AggressiveDCETest, ElimWithCall) {
+  // This demonstrates that "dead" function calls are not eliminated.
+  // Also demonstrates that DCE will happen in presence of function call.
+  // #version 140
+  // in vec4 i1;
+  // in vec4 i2;
+  // 
+  // void nothing(vec4 v)
+  // {
+  // }
+  // 
+  // void main()
+  // {
+  //     vec4 v1 = i1;
+  //     vec4 v2 = i2;
+  //     nothing(v1);
+  //     gl_FragColor = vec4(0.0);
+  // }
+
+  const std::string defs_before =
+      R"( OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main" %i1 %i2 %gl_FragColor
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 140
+OpName %main "main"
+OpName %nothing_vf4_ "nothing(vf4;" 
+OpName %v "v"
+OpName %v1 "v1"
+OpName %i1 "i1"
+OpName %v2 "v2"
+OpName %i2 "i2"
+OpName %param "param" 
+OpName %gl_FragColor "gl_FragColor" 
+%void = OpTypeVoid
+%12 = OpTypeFunction %void
+%float = OpTypeFloat 32
+%v4float = OpTypeVector %float 4
+%_ptr_Function_v4float = OpTypePointer Function %v4float
+%16 = OpTypeFunction %void %_ptr_Function_v4float
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%i1 = OpVariable %_ptr_Input_v4float Input
+%i2 = OpVariable %_ptr_Input_v4float Input
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%gl_FragColor = OpVariable %_ptr_Output_v4float Output
+%float_0 = OpConstant %float 0
+%20 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0 
+)";
+
+  const std::string defs_after =
+      R"(OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main" %i1 %i2 %gl_FragColor
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 140
+OpName %main "main"
+OpName %nothing_vf4_ "nothing(vf4;"
+OpName %v "v"
+OpName %v1 "v1"
+OpName %i1 "i1"
+OpName %i2 "i2"
+OpName %param "param"
+OpName %gl_FragColor "gl_FragColor"
+%void = OpTypeVoid
+%12 = OpTypeFunction %void
+%float = OpTypeFloat 32
+%v4float = OpTypeVector %float 4
+%_ptr_Function_v4float = OpTypePointer Function %v4float
+%16 = OpTypeFunction %void %_ptr_Function_v4float
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%i1 = OpVariable %_ptr_Input_v4float Input
+%i2 = OpVariable %_ptr_Input_v4float Input
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%gl_FragColor = OpVariable %_ptr_Output_v4float Output
+%float_0 = OpConstant %float 0
+%20 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
+)";
+
+  const std::string func_before =
+      R"(%main = OpFunction %void None %12
+%21 = OpLabel
+%v1 = OpVariable %_ptr_Function_v4float Function 
+%v2 = OpVariable %_ptr_Function_v4float Function 
+%param = OpVariable %_ptr_Function_v4float Function
+%22 = OpLoad %v4float %i1
+OpStore %v1 %22
+%23 = OpLoad %v4float %i2
+OpStore %v2 %23
+%24 = OpLoad %v4float %v1
+OpStore %param %24
+%25 = OpFunctionCall %void %nothing_vf4_ %param
+OpStore %gl_FragColor %20
+OpReturn
+OpFunctionEnd
+%nothing_vf4_ = OpFunction %void None %16
+%v = OpFunctionParameter %_ptr_Function_v4float
+%26 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  const std::string func_after =
+      R"(%main = OpFunction %void None %12
+%21 = OpLabel
+%v1 = OpVariable %_ptr_Function_v4float Function
+%param = OpVariable %_ptr_Function_v4float Function
+%22 = OpLoad %v4float %i1
+OpStore %v1 %22
+%24 = OpLoad %v4float %v1
+OpStore %param %24
+%25 = OpFunctionCall %void %nothing_vf4_ %param
+OpStore %gl_FragColor %20
+OpReturn
+OpFunctionEnd
+%nothing_vf4_ = OpFunction %void None %16
+%v = OpFunctionParameter %_ptr_Function_v4float
+%26 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndCheck<opt::AggressiveDCEPass>(
+      defs_before + func_before, defs_after + func_after, true, true);
+}
+
 // TODO(greg-lunarg): Add tests to verify handling of these cases:
 //
 //    Check that logical addressing required