From 8e0c80f26914552e11144bd92dae726b66c3739d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 5 Oct 2019 08:06:44 -0400 Subject: video_core/ast: Supply const accessors for data where applicable Provides const equivalents of data accessors for use within const contexts. --- src/video_core/shader/ast.cpp | 66 ++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 35 deletions(-) (limited to 'src/video_core/shader/ast.cpp') diff --git a/src/video_core/shader/ast.cpp b/src/video_core/shader/ast.cpp index 2eb065c3d..87c722682 100644 --- a/src/video_core/shader/ast.cpp +++ b/src/video_core/shader/ast.cpp @@ -185,9 +185,7 @@ void ASTZipper::Remove(const ASTNode node) { class ExprPrinter final { public: - ExprPrinter() = default; - - void operator()(ExprAnd const& expr) { + void operator()(const ExprAnd& expr) { inner += "( "; std::visit(*this, *expr.operand1); inner += " && "; @@ -195,7 +193,7 @@ public: inner += ')'; } - void operator()(ExprOr const& expr) { + void operator()(const ExprOr& expr) { inner += "( "; std::visit(*this, *expr.operand1); inner += " || "; @@ -203,29 +201,29 @@ public: inner += ')'; } - void operator()(ExprNot const& expr) { + void operator()(const ExprNot& expr) { inner += "!"; std::visit(*this, *expr.operand1); } - void operator()(ExprPredicate const& expr) { + void operator()(const ExprPredicate& expr) { inner += "P" + std::to_string(expr.predicate); } - void operator()(ExprCondCode const& expr) { + void operator()(const ExprCondCode& expr) { u32 cc = static_cast(expr.cc); inner += "CC" + std::to_string(cc); } - void operator()(ExprVar const& expr) { + void operator()(const ExprVar& expr) { inner += "V" + std::to_string(expr.var_index); } - void operator()(ExprBoolean const& expr) { + void operator()(const ExprBoolean& expr) { inner += expr.value ? "true" : "false"; } - std::string& GetResult() { + const std::string& GetResult() const { return inner; } @@ -234,9 +232,7 @@ public: class ASTPrinter { public: - ASTPrinter() = default; - - void operator()(ASTProgram& ast) { + void operator()(const ASTProgram& ast) { scope++; inner += "program {\n"; ASTNode current = ast.nodes.GetFirst(); @@ -248,7 +244,7 @@ public: scope--; } - void operator()(ASTIfThen& ast) { + void operator()(const ASTIfThen& ast) { ExprPrinter expr_parser{}; std::visit(expr_parser, *ast.condition); inner += Ident() + "if (" + expr_parser.GetResult() + ") {\n"; @@ -262,7 +258,7 @@ public: inner += Ident() + "}\n"; } - void operator()(ASTIfElse& ast) { + void operator()(const ASTIfElse& ast) { inner += Ident() + "else {\n"; scope++; ASTNode current = ast.nodes.GetFirst(); @@ -274,34 +270,34 @@ public: inner += Ident() + "}\n"; } - void operator()(ASTBlockEncoded& ast) { + void operator()(const ASTBlockEncoded& ast) { inner += Ident() + "Block(" + std::to_string(ast.start) + ", " + std::to_string(ast.end) + ");\n"; } - void operator()(ASTBlockDecoded& ast) { + void operator()(const ASTBlockDecoded& ast) { inner += Ident() + "Block;\n"; } - void operator()(ASTVarSet& ast) { + void operator()(const ASTVarSet& ast) { ExprPrinter expr_parser{}; std::visit(expr_parser, *ast.condition); inner += Ident() + "V" + std::to_string(ast.index) + " := " + expr_parser.GetResult() + ";\n"; } - void operator()(ASTLabel& ast) { + void operator()(const ASTLabel& ast) { inner += "Label_" + std::to_string(ast.index) + ":\n"; } - void operator()(ASTGoto& ast) { + void operator()(const ASTGoto& ast) { ExprPrinter expr_parser{}; std::visit(expr_parser, *ast.condition); inner += Ident() + "(" + expr_parser.GetResult() + ") -> goto Label_" + std::to_string(ast.label) + ";\n"; } - void operator()(ASTDoWhile& ast) { + void operator()(const ASTDoWhile& ast) { ExprPrinter expr_parser{}; std::visit(expr_parser, *ast.condition); inner += Ident() + "do {\n"; @@ -315,14 +311,14 @@ public: inner += Ident() + "} while (" + expr_parser.GetResult() + ");\n"; } - void operator()(ASTReturn& ast) { + void operator()(const ASTReturn& ast) { ExprPrinter expr_parser{}; std::visit(expr_parser, *ast.condition); inner += Ident() + "(" + expr_parser.GetResult() + ") -> " + (ast.kills ? "discard" : "exit") + ";\n"; } - void operator()(ASTBreak& ast) { + void operator()(const ASTBreak& ast) { ExprPrinter expr_parser{}; std::visit(expr_parser, *ast.condition); inner += Ident() + "(" + expr_parser.GetResult() + ") -> break;\n"; @@ -341,7 +337,7 @@ public: std::visit(*this, *node->GetInnerData()); } - std::string& GetResult() { + const std::string& GetResult() const { return inner; } @@ -696,7 +692,7 @@ class ASTClearer { public: ASTClearer() = default; - void operator()(ASTProgram& ast) { + void operator()(const ASTProgram& ast) { ASTNode current = ast.nodes.GetFirst(); while (current) { Visit(current); @@ -704,7 +700,7 @@ public: } } - void operator()(ASTIfThen& ast) { + void operator()(const ASTIfThen& ast) { ASTNode current = ast.nodes.GetFirst(); while (current) { Visit(current); @@ -712,7 +708,7 @@ public: } } - void operator()(ASTIfElse& ast) { + void operator()(const ASTIfElse& ast) { ASTNode current = ast.nodes.GetFirst(); while (current) { Visit(current); @@ -720,19 +716,19 @@ public: } } - void operator()(ASTBlockEncoded& ast) {} + void operator()([[maybe_unused]] const ASTBlockEncoded& ast) {} void operator()(ASTBlockDecoded& ast) { ast.nodes.clear(); } - void operator()(ASTVarSet& ast) {} + void operator()([[maybe_unused]] const ASTVarSet& ast) {} - void operator()(ASTLabel& ast) {} + void operator()([[maybe_unused]] const ASTLabel& ast) {} - void operator()(ASTGoto& ast) {} + void operator()([[maybe_unused]] const ASTGoto& ast) {} - void operator()(ASTDoWhile& ast) { + void operator()(const ASTDoWhile& ast) { ASTNode current = ast.nodes.GetFirst(); while (current) { Visit(current); @@ -740,11 +736,11 @@ public: } } - void operator()(ASTReturn& ast) {} + void operator()([[maybe_unused]] const ASTReturn& ast) {} - void operator()(ASTBreak& ast) {} + void operator()([[maybe_unused]] const ASTBreak& ast) {} - void Visit(ASTNode& node) { + void Visit(const ASTNode& node) { std::visit(*this, *node->GetInnerData()); node->Clear(); } -- cgit v1.2.3 From 8eb1398f8d90fb2813f438b9fffac716b6ec51d2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 5 Oct 2019 08:17:32 -0400 Subject: video_core/{ast, expr}: Use std::move where applicable Avoids unnecessary atomic reference count increments and decrements. --- src/video_core/shader/ast.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) (limited to 'src/video_core/shader/ast.cpp') diff --git a/src/video_core/shader/ast.cpp b/src/video_core/shader/ast.cpp index 87c722682..986e4cd64 100644 --- a/src/video_core/shader/ast.cpp +++ b/src/video_core/shader/ast.cpp @@ -17,6 +17,7 @@ void ASTZipper::Init(const ASTNode new_first, const ASTNode parent) { ASSERT(new_first->manager == nullptr); first = new_first; last = new_first; + ASTNode current = first; while (current) { current->manager = this; @@ -92,7 +93,7 @@ void ASTZipper::InsertBefore(const ASTNode new_node, const ASTNode at_node) { new_node->manager = this; } -void ASTZipper::DetachTail(const ASTNode node) { +void ASTZipper::DetachTail(ASTNode node) { ASSERT(node->manager == this); if (node == first) { first.reset(); @@ -103,7 +104,8 @@ void ASTZipper::DetachTail(const ASTNode node) { last = node->previous; last->next.reset(); node->previous.reset(); - ASTNode current = node; + + ASTNode current = std::move(node); while (current) { current->manager = nullptr; current->parent.reset(); @@ -413,19 +415,19 @@ void ASTManager::InsertLabel(u32 address) { void ASTManager::InsertGoto(Expr condition, u32 address) { const u32 index = labels_map[address]; - const ASTNode goto_node = ASTBase::Make(main_node, condition, index); + const ASTNode goto_node = ASTBase::Make(main_node, std::move(condition), index); gotos.push_back(goto_node); program->nodes.PushBack(goto_node); } void ASTManager::InsertBlock(u32 start_address, u32 end_address) { - const ASTNode block = ASTBase::Make(main_node, start_address, end_address); - program->nodes.PushBack(block); + ASTNode block = ASTBase::Make(main_node, start_address, end_address); + program->nodes.PushBack(std::move(block)); } void ASTManager::InsertReturn(Expr condition, bool kills) { - const ASTNode node = ASTBase::Make(main_node, condition, kills); - program->nodes.PushBack(node); + ASTNode node = ASTBase::Make(main_node, std::move(condition), kills); + program->nodes.PushBack(std::move(node)); } // The decompile algorithm is based on @@ -539,11 +541,11 @@ bool ASTManager::IsBackwardsJump(ASTNode goto_node, ASTNode label_node) const { return false; } -bool ASTManager::IndirectlyRelated(ASTNode first, ASTNode second) { +bool ASTManager::IndirectlyRelated(const ASTNode& first, const ASTNode& second) const { return !(first->GetParent() == second->GetParent() || DirectlyRelated(first, second)); } -bool ASTManager::DirectlyRelated(ASTNode first, ASTNode second) { +bool ASTManager::DirectlyRelated(const ASTNode& first, const ASTNode& second) const { if (first->GetParent() == second->GetParent()) { return false; } -- cgit v1.2.3 From 3a20d9734fa8996434895bc3d27e4b255ae98bea Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 5 Oct 2019 08:43:44 -0400 Subject: video_core/ast: Default the move constructor and assignment operator This is behaviorally equivalent and also fixes a bug where some members weren't being moved over. --- src/video_core/shader/ast.cpp | 24 ------------------------ 1 file changed, 24 deletions(-) (limited to 'src/video_core/shader/ast.cpp') diff --git a/src/video_core/shader/ast.cpp b/src/video_core/shader/ast.cpp index 986e4cd64..2627c563c 100644 --- a/src/video_core/shader/ast.cpp +++ b/src/video_core/shader/ast.cpp @@ -374,30 +374,6 @@ void ASTManager::Init() { false_condition = MakeExpr(false); } -ASTManager::ASTManager(ASTManager&& other) noexcept - : labels_map(std::move(other.labels_map)), labels_count{other.labels_count}, - gotos(std::move(other.gotos)), labels(std::move(other.labels)), variables{other.variables}, - program{other.program}, main_node{other.main_node}, false_condition{other.false_condition}, - disable_else_derivation{other.disable_else_derivation} { - other.main_node.reset(); -} - -ASTManager& ASTManager::operator=(ASTManager&& other) noexcept { - full_decompile = other.full_decompile; - labels_map = std::move(other.labels_map); - labels_count = other.labels_count; - gotos = std::move(other.gotos); - labels = std::move(other.labels); - variables = other.variables; - program = other.program; - main_node = other.main_node; - false_condition = other.false_condition; - disable_else_derivation = other.disable_else_derivation; - - other.main_node.reset(); - return *this; -} - void ASTManager::DeclareLabel(u32 address) { const auto pair = labels_map.emplace(address, labels_count); if (pair.second) { -- cgit v1.2.3 From 5a0a9c7449f4f264fae8619d3881c6c0e09865ef Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 5 Oct 2019 08:45:28 -0400 Subject: video_core/ast: Replace std::string with a constexpr std::string_view Same behavior, but without the need to heap allocate --- src/video_core/shader/ast.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'src/video_core/shader/ast.cpp') diff --git a/src/video_core/shader/ast.cpp b/src/video_core/shader/ast.cpp index 2627c563c..f2ab0cc00 100644 --- a/src/video_core/shader/ast.cpp +++ b/src/video_core/shader/ast.cpp @@ -350,11 +350,9 @@ private: std::string tabs_memo{}; u32 memo_scope{}; - static std::string tabs; + static constexpr std::string_view tabs{" "}; }; -std::string ASTPrinter::tabs = " "; - std::string ASTManager::Print() { ASTPrinter printer{}; printer.Visit(main_node); -- cgit v1.2.3 From 3c54edae2438bd0dced6c552b42ff2be4eebd6b6 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 5 Oct 2019 08:46:54 -0400 Subject: video_core/ast: Eliminate variable shadowing warnings --- src/video_core/shader/ast.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/video_core/shader/ast.cpp') diff --git a/src/video_core/shader/ast.cpp b/src/video_core/shader/ast.cpp index f2ab0cc00..6eba78025 100644 --- a/src/video_core/shader/ast.cpp +++ b/src/video_core/shader/ast.cpp @@ -468,10 +468,10 @@ void ASTManager::Decompile() { } labels.clear(); } else { - auto it = labels.begin(); - while (it != labels.end()) { + auto label_it = labels.begin(); + while (label_it != labels.end()) { bool can_remove = true; - ASTNode label = *it; + ASTNode label = *label_it; for (const ASTNode& goto_node : gotos) { const auto label_index = goto_node->GetGotoLabel(); if (!label_index) { -- cgit v1.2.3 From 6c41d1cd7eadf1030c02d661d7f360b98f4a8943 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 5 Oct 2019 08:48:15 -0400 Subject: video_core/ast: Make ShowCurrentState() take a string_view instead of std::string Allows the function to be non-allocating in terms of the output string. --- src/video_core/shader/ast.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/video_core/shader/ast.cpp') diff --git a/src/video_core/shader/ast.cpp b/src/video_core/shader/ast.cpp index 6eba78025..436d45f4b 100644 --- a/src/video_core/shader/ast.cpp +++ b/src/video_core/shader/ast.cpp @@ -549,7 +549,7 @@ bool ASTManager::DirectlyRelated(const ASTNode& first, const ASTNode& second) co return min->GetParent() == max->GetParent(); } -void ASTManager::ShowCurrentState(std::string state) { +void ASTManager::ShowCurrentState(std::string_view state) { LOG_CRITICAL(HW_GPU, "\nState {}:\n\n{}\n", state, Print()); SanityCheck(); } -- cgit v1.2.3