DCE: clean up the cfg for all functions that were processed. (#4840)
Which functions are processed is determined by which ones are on the
call tree from the entry points before dead code is removed. So it is
possible that a function is process because it is called from an entry
point, but the CFG is not cleaned up because the call to the function
was removed.
The fix is to process and cleanup every function in the module. Since
all of the dead functions would have already been removed in an earlier
step of DCE, it should not make a different in compile time.
Fixes #4731
diff --git a/source/opt/aggressive_dead_code_elim_pass.cpp b/source/opt/aggressive_dead_code_elim_pass.cpp
index 2486242..ffb499f 100644
--- a/source/opt/aggressive_dead_code_elim_pass.cpp
+++ b/source/opt/aggressive_dead_code_elim_pass.cpp
@@ -659,9 +659,14 @@
InitializeModuleScopeLiveInstructions();
- // Process all entry point functions.
- ProcessFunction pfn = [this](Function* fp) { return AggressiveDCE(fp); };
- modified |= context()->ProcessReachableCallTree(pfn);
+ // Run |AggressiveDCE| on the remaining functions. The order does not matter,
+ // since |AggressiveDCE| is intra-procedural. This can mean that function
+ // will become dead if all function call to them are removed. These dead
+ // function will still be in the module after this pass. We expect this to be
+ // rare.
+ for (Function& fp : *context()->module()) {
+ modified |= AggressiveDCE(&fp);
+ }
// If the decoration manager is kept live then the context will try to keep it
// up to date. ADCE deals with group decorations by changing the operands in
@@ -687,8 +692,9 @@
}
// Cleanup all CFG including all unreachable blocks.
- ProcessFunction cleanup = [this](Function* f) { return CFGCleanup(f); };
- modified |= context()->ProcessReachableCallTree(cleanup);
+ for (Function& fp : *context()->module()) {
+ modified |= CFGCleanup(&fp);
+ }
return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange;
}
diff --git a/test/opt/aggressive_dead_code_elim_test.cpp b/test/opt/aggressive_dead_code_elim_test.cpp
index d15c79d..89cb56f 100644
--- a/test/opt/aggressive_dead_code_elim_test.cpp
+++ b/test/opt/aggressive_dead_code_elim_test.cpp
@@ -7628,6 +7628,55 @@
SinglePassRunAndCheck<AggressiveDCEPass>(text, text, false);
}
+TEST_F(AggressiveDCETest, FunctionBecomesUnreachableAfterDCE) {
+ const std::string text = R"(
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %2 "main"
+ OpExecutionMode %2 OriginUpperLeft
+ OpSource ESSL 320
+ %void = OpTypeVoid
+ %4 = OpTypeFunction %void
+ %int = OpTypeInt 32 1
+%_ptr_Function_int = OpTypePointer Function %int
+ %7 = OpTypeFunction %int
+ %int_n1 = OpConstant %int -1
+ %2 = OpFunction %void None %4
+ %9 = OpLabel
+ OpKill
+ %10 = OpLabel
+ %11 = OpFunctionCall %int %12
+ OpReturn
+ OpFunctionEnd
+; CHECK: {{%\w+}} = OpFunction %int DontInline|Pure
+ %12 = OpFunction %int DontInline|Pure %7
+; CHECK-NEXT: {{%\w+}} = OpLabel
+ %13 = OpLabel
+ %14 = OpVariable %_ptr_Function_int Function
+; CHECK-NEXT: OpBranch [[header:%\w+]]
+ OpBranch %15
+; CHECK-NEXT: [[header]] = OpLabel
+; CHECK-NEXT: OpBranch [[merge:%\w+]]
+ %15 = OpLabel
+ OpLoopMerge %16 %17 None
+ OpBranch %18
+ %18 = OpLabel
+ %19 = OpLoad %int %14
+ OpBranch %17
+ %17 = OpLabel
+ OpBranch %15
+; CHECK-NEXT: [[merge]] = OpLabel
+ %16 = OpLabel
+; CHECK-NEXT: OpReturnValue %int_n1
+ OpReturnValue %int_n1
+; CHECK-NEXT: OpFunctionEnd
+ OpFunctionEnd
+)";
+
+ SinglePassRunAndMatch<AggressiveDCEPass>(text, true);
+}
+
} // namespace
} // namespace opt
} // namespace spvtools
