Invalidate the decoration manager at the start of ADCE. (#2189)
* Invalidate the decoration manager at the start of ADCE.
If the decoration manager is kept live the the contex will try to keep
it up to date. ADCE deals with group decorations by changing the
operands in |OpGroupDecorate| instructions directly without informing
the decoration manager. This puts it in an invalid state, which will
cause an error when the context tries to update it. To Avoid this
problem, we will invalidate the decoration manager upfront.
At the same time, the decoration manager is now considered when checking
the consistency of the decoration manager.
diff --git a/source/opt/aggressive_dead_code_elim_pass.cpp b/source/opt/aggressive_dead_code_elim_pass.cpp
index 2793312..82d7499 100644
--- a/source/opt/aggressive_dead_code_elim_pass.cpp
+++ b/source/opt/aggressive_dead_code_elim_pass.cpp
@@ -555,6 +555,14 @@
// return unmodified.
if (!AllExtensionsSupported()) return Status::SuccessWithoutChange;
+ // 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
+ // |OpGroupDecorate| instruction directly without informing the decoration
+ // manager. This can put it in an invalid state which will cause an error
+ // when the context tries to update it. To avoid this problem invalidate
+ // the decoration manager upfront.
+ context()->InvalidateAnalyses(IRContext::Analysis::kAnalysisDecorations);
+
// Eliminate Dead functions.
bool modified = EliminateDeadFunctions();
diff --git a/source/opt/decoration_manager.cpp b/source/opt/decoration_manager.cpp
index 9990661..a12326b 100644
--- a/source/opt/decoration_manager.cpp
+++ b/source/opt/decoration_manager.cpp
@@ -517,6 +517,11 @@
break;
}
}
+
+bool operator==(const DecorationManager& lhs, const DecorationManager& rhs) {
+ return lhs.id_to_decoration_insts_ == rhs.id_to_decoration_insts_;
+}
+
} // namespace analysis
} // namespace opt
} // namespace spvtools
diff --git a/source/opt/decoration_manager.h b/source/opt/decoration_manager.h
index 2d87f5f..a5fb4c8 100644
--- a/source/opt/decoration_manager.h
+++ b/source/opt/decoration_manager.h
@@ -125,6 +125,12 @@
void AddMemberDecoration(uint32_t member, uint32_t inst_id,
uint32_t decoration, uint32_t decoration_value);
+ friend bool operator==(const DecorationManager&, const DecorationManager&);
+ friend bool operator!=(const DecorationManager& lhs,
+ const DecorationManager& rhs) {
+ return !(lhs == rhs);
+ }
+
private:
// Analyzes the defs and uses in the given |module| and populates data
// structures in this class. Does nothing if |module| is nullptr.
@@ -150,6 +156,25 @@
// group.
};
+ friend bool operator==(const TargetData& lhs, const TargetData& rhs) {
+ if (!std::is_permutation(lhs.direct_decorations.begin(),
+ lhs.direct_decorations.end(),
+ rhs.direct_decorations.begin())) {
+ return false;
+ }
+ if (!std::is_permutation(lhs.indirect_decorations.begin(),
+ lhs.indirect_decorations.end(),
+ rhs.indirect_decorations.begin())) {
+ return false;
+ }
+ if (!std::is_permutation(lhs.decorate_insts.begin(),
+ lhs.decorate_insts.end(),
+ rhs.decorate_insts.begin())) {
+ return false;
+ }
+ return true;
+ }
+
// Mapping from ids to the instructions applying a decoration to those ids.
// In other words, for each id you get all decoration instructions
// referencing that id, be it directly (SpvOpDecorate, SpvOpMemberDecorate
diff --git a/source/opt/ir_context.cpp b/source/opt/ir_context.cpp
index 31dbe5b..af9ac1a 100644
--- a/source/opt/ir_context.cpp
+++ b/source/opt/ir_context.cpp
@@ -252,6 +252,14 @@
return false;
}
+ if (AreAnalysesValid(kAnalysisDecorations)) {
+ analysis::DecorationManager* dec_mgr = get_decoration_mgr();
+ analysis::DecorationManager current(module());
+
+ if (*dec_mgr != current) {
+ return false;
+ }
+ }
return true;
}
diff --git a/test/opt/aggressive_dead_code_elim_test.cpp b/test/opt/aggressive_dead_code_elim_test.cpp
index 5db4445..e58a76f 100644
--- a/test/opt/aggressive_dead_code_elim_test.cpp
+++ b/test/opt/aggressive_dead_code_elim_test.cpp
@@ -5111,6 +5111,43 @@
SinglePassRunAndMatch<AggressiveDCEPass>(text, true);
}
+TEST_F(AggressiveDCETest, DeadDecorationGroupAndValidDecorationMgr) {
+ // The decoration group should be eliminated because the target of group
+ // decorate is dead.
+ const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+OpDecorate %1 Restrict
+OpDecorate %1 Aliased
+%1 = OpDecorationGroup
+OpGroupDecorate %1 %var
+%void = OpTypeVoid
+%func = OpTypeFunction %void
+%uint = OpTypeInt 32 0
+%uint_ptr = OpTypePointer Function %uint
+%main = OpFunction %void None %func
+%2 = OpLabel
+%var = OpVariable %uint_ptr Function
+OpReturn
+OpFunctionEnd
+ )";
+
+ auto pass = MakeUnique<AggressiveDCEPass>();
+ auto consumer = [](spv_message_level_t, const char*, const spv_position_t&,
+ const char* message) {
+ std::cerr << message << std::endl;
+ };
+ auto context = BuildModule(SPV_ENV_UNIVERSAL_1_1, consumer, text);
+
+ // Build the decoration manager before the pass.
+ context->get_decoration_mgr();
+
+ const auto status = pass->Run(context.get());
+ EXPECT_EQ(status, Pass::Status::SuccessWithChange);
+}
+
TEST_F(AggressiveDCETest, ParitallyDeadDecorationGroup) {
const std::string text = R"(
; CHECK: OpDecorate [[grp:%\w+]] Restrict
