From 3c09d9abe6d268ada063fd67c08d09fc0fcad613 Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Sat, 28 Sep 2019 15:16:19 -0400 Subject: [PATCH] Shader_Ir: Address Feedback and clang format. --- .../renderer_vulkan/vk_shader_decompiler.cpp | 43 +++++------- src/video_core/shader/ast.cpp | 14 ++-- src/video_core/shader/ast.h | 65 ++++++++++--------- src/video_core/shader/expr.h | 14 ++-- 4 files changed, 68 insertions(+), 68 deletions(-) diff --git a/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp b/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp index 2b55a3727a..8bcd04221f 100644 --- a/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp +++ b/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp @@ -1646,34 +1646,34 @@ private: class ExprDecompiler { public: - ExprDecompiler(SPIRVDecompiler& decomp) : decomp{decomp} {} + explicit ExprDecompiler(SPIRVDecompiler& decomp) : decomp{decomp} {} - void operator()(VideoCommon::Shader::ExprAnd& expr) { + Id operator()(VideoCommon::Shader::ExprAnd& expr) { const Id type_def = decomp.GetTypeDefinition(Type::Bool); const Id op1 = Visit(expr.operand1); const Id op2 = Visit(expr.operand2); - current_id = decomp.Emit(decomp.OpLogicalAnd(type_def, op1, op2)); + return decomp.Emit(decomp.OpLogicalAnd(type_def, op1, op2)); } - void operator()(VideoCommon::Shader::ExprOr& expr) { + Id operator()(VideoCommon::Shader::ExprOr& expr) { const Id type_def = decomp.GetTypeDefinition(Type::Bool); const Id op1 = Visit(expr.operand1); const Id op2 = Visit(expr.operand2); - current_id = decomp.Emit(decomp.OpLogicalOr(type_def, op1, op2)); + return decomp.Emit(decomp.OpLogicalOr(type_def, op1, op2)); } - void operator()(VideoCommon::Shader::ExprNot& expr) { + Id operator()(VideoCommon::Shader::ExprNot& expr) { const Id type_def = decomp.GetTypeDefinition(Type::Bool); const Id op1 = Visit(expr.operand1); - current_id = decomp.Emit(decomp.OpLogicalNot(type_def, op1)); + return decomp.Emit(decomp.OpLogicalNot(type_def, op1)); } - void operator()(VideoCommon::Shader::ExprPredicate& expr) { + Id operator()(VideoCommon::Shader::ExprPredicate& expr) { const auto pred = static_cast(expr.predicate); - current_id = decomp.Emit(decomp.OpLoad(decomp.t_bool, decomp.predicates.at(pred))); + return decomp.Emit(decomp.OpLoad(decomp.t_bool, decomp.predicates.at(pred))); } - void operator()(VideoCommon::Shader::ExprCondCode& expr) { + Id operator()(VideoCommon::Shader::ExprCondCode& expr) { const Node cc = decomp.ir.GetConditionCode(expr.cc); Id target; @@ -1690,35 +1690,28 @@ public: } else if (const auto flag = std::get_if(&*cc)) { target = decomp.internal_flags.at(static_cast(flag->GetFlag())); } - current_id = decomp.Emit(decomp.OpLoad(decomp.t_bool, target)); + return decomp.Emit(decomp.OpLoad(decomp.t_bool, target)); } - void operator()(VideoCommon::Shader::ExprVar& expr) { - current_id = - decomp.Emit(decomp.OpLoad(decomp.t_bool, decomp.flow_variables.at(expr.var_index))); + Id operator()(VideoCommon::Shader::ExprVar& expr) { + return decomp.Emit(decomp.OpLoad(decomp.t_bool, decomp.flow_variables.at(expr.var_index))); } - void operator()(VideoCommon::Shader::ExprBoolean& expr) { - current_id = expr.value ? decomp.v_true : decomp.v_false; - } - - Id GetResult() { - return current_id; + Id operator()(VideoCommon::Shader::ExprBoolean& expr) { + return expr.value ? decomp.v_true : decomp.v_false; } Id Visit(VideoCommon::Shader::Expr& node) { - std::visit(*this, *node); - return current_id; + return std::visit(*this, *node); } private: - Id current_id; SPIRVDecompiler& decomp; }; class ASTDecompiler { public: - ASTDecompiler(SPIRVDecompiler& decomp) : decomp{decomp} {} + explicit ASTDecompiler(SPIRVDecompiler& decomp) : decomp{decomp} {} void operator()(VideoCommon::Shader::ASTProgram& ast) { ASTNode current = ast.nodes.GetFirst(); @@ -1850,7 +1843,7 @@ public: private: SPIRVDecompiler& decomp; - Id current_loop_exit; + Id current_loop_exit{}; }; void SPIRVDecompiler::DecompileAST() { diff --git a/src/video_core/shader/ast.cpp b/src/video_core/shader/ast.cpp index fc440526f1..c4548f0bc9 100644 --- a/src/video_core/shader/ast.cpp +++ b/src/video_core/shader/ast.cpp @@ -442,8 +442,11 @@ void ASTManager::Decompile() { auto it = gotos.begin(); while (it != gotos.end()) { const ASTNode goto_node = *it; - const u32 label_index = goto_node->GetGotoLabel(); - const ASTNode label = labels[label_index]; + const auto label_index = goto_node->GetGotoLabel(); + if (!label_index) { + return; + } + const ASTNode label = labels[*label_index]; if (!full_decompile) { // We only decompile backward jumps if (!IsBackwardsJump(goto_node, label)) { @@ -498,8 +501,11 @@ void ASTManager::Decompile() { bool can_remove = true; ASTNode label = *it; for (const ASTNode goto_node : gotos) { - const u32 label_index = goto_node->GetGotoLabel(); - ASTNode glabel = labels[label_index]; + const auto label_index = goto_node->GetGotoLabel(); + if (!label_index) { + return; + } + ASTNode glabel = labels[*label_index]; if (glabel == label) { can_remove = false; break; diff --git a/src/video_core/shader/ast.h b/src/video_core/shader/ast.h index 1b73f301fb..8efd4c1477 100644 --- a/src/video_core/shader/ast.h +++ b/src/video_core/shader/ast.h @@ -44,7 +44,7 @@ enum class ASTZipperType : u32 { class ASTZipper final { public: - ASTZipper(); + explicit ASTZipper(); void Init(ASTNode first, ASTNode parent); @@ -71,74 +71,74 @@ public: class ASTProgram { public: - ASTProgram() : nodes{} {}; - ASTZipper nodes; + explicit ASTProgram() = default; + ASTZipper nodes{}; }; class ASTIfThen { public: - ASTIfThen(Expr condition) : condition(condition), nodes{} {} + explicit ASTIfThen(Expr condition) : condition(condition) {} Expr condition; - ASTZipper nodes; + ASTZipper nodes{}; }; class ASTIfElse { public: - ASTIfElse() : nodes{} {} - ASTZipper nodes; + explicit ASTIfElse() = default; + ASTZipper nodes{}; }; class ASTBlockEncoded { public: - ASTBlockEncoded(u32 start, u32 end) : start{start}, end{end} {} + explicit ASTBlockEncoded(u32 start, u32 end) : start{start}, end{end} {} u32 start; u32 end; }; class ASTBlockDecoded { public: - ASTBlockDecoded(NodeBlock& new_nodes) : nodes(std::move(new_nodes)) {} + explicit ASTBlockDecoded(NodeBlock& new_nodes) : nodes(std::move(new_nodes)) {} NodeBlock nodes; }; class ASTVarSet { public: - ASTVarSet(u32 index, Expr condition) : index{index}, condition{condition} {} + explicit ASTVarSet(u32 index, Expr condition) : index{index}, condition{condition} {} u32 index; Expr condition; }; class ASTLabel { public: - ASTLabel(u32 index) : index{index} {} + explicit ASTLabel(u32 index) : index{index} {} u32 index; bool unused{}; }; class ASTGoto { public: - ASTGoto(Expr condition, u32 label) : condition{condition}, label{label} {} + explicit ASTGoto(Expr condition, u32 label) : condition{condition}, label{label} {} Expr condition; u32 label; }; class ASTDoWhile { public: - ASTDoWhile(Expr condition) : condition(condition), nodes{} {} + explicit ASTDoWhile(Expr condition) : condition(condition) {} Expr condition; - ASTZipper nodes; + ASTZipper nodes{}; }; class ASTReturn { public: - ASTReturn(Expr condition, bool kills) : condition{condition}, kills{kills} {} + explicit ASTReturn(Expr condition, bool kills) : condition{condition}, kills{kills} {} Expr condition; bool kills; }; class ASTBreak { public: - ASTBreak(Expr condition) : condition{condition} {} + explicit ASTBreak(Expr condition) : condition{condition} {} Expr condition; }; @@ -177,11 +177,11 @@ public: return &data; } - ASTNode GetNext() { + ASTNode GetNext() const { return next; } - ASTNode GetPrevious() { + ASTNode GetPrevious() const { return previous; } @@ -189,12 +189,12 @@ public: return *manager; } - u32 GetGotoLabel() const { + std::optional GetGotoLabel() const { auto inner = std::get_if(&data); if (inner) { - return inner->label; + return {inner->label}; } - return -1; + return {}; } Expr GetGotoCondition() const { @@ -220,12 +220,12 @@ public: return true; } - u32 GetLabelIndex() const { + std::optional GetLabelIndex() const { auto inner = std::get_if(&data); if (inner) { - return inner->index; + return {inner->index}; } - return -1; + return {}; } Expr GetIfCondition() const { @@ -290,7 +290,7 @@ private: friend class ASTZipper; ASTData data; - ASTNode parent; + ASTNode parent{}; ASTNode next{}; ASTNode previous{}; ASTZipper* manager{}; @@ -327,13 +327,18 @@ public: void SanityCheck(); + void Clear(); + bool IsFullyDecompiled() const { if (full_decompile) { return gotos.size() == 0; } else { for (ASTNode goto_node : gotos) { - u32 label_index = goto_node->GetGotoLabel(); - ASTNode glabel = labels[label_index]; + auto label_index = goto_node->GetGotoLabel(); + if (!label_index) { + return false; + } + ASTNode glabel = labels[*label_index]; if (IsBackwardsJump(goto_node, glabel)) { return false; } @@ -346,8 +351,6 @@ public: return main_node; } - void Clear(); - u32 GetVariables() const { return variables; } @@ -372,9 +375,7 @@ private: void MoveOutward(ASTNode goto_node); u32 NewVariable() { - u32 new_var = variables; - variables++; - return new_var; + return variables++; } bool full_decompile{}; diff --git a/src/video_core/shader/expr.h b/src/video_core/shader/expr.h index 60598977a3..4c399cef9c 100644 --- a/src/video_core/shader/expr.h +++ b/src/video_core/shader/expr.h @@ -28,7 +28,7 @@ using Expr = std::shared_ptr; class ExprAnd final { public: - ExprAnd(Expr a, Expr b) : operand1{a}, operand2{b} {} + explicit ExprAnd(Expr a, Expr b) : operand1{a}, operand2{b} {} bool operator==(const ExprAnd& b) const; @@ -38,7 +38,7 @@ public: class ExprOr final { public: - ExprOr(Expr a, Expr b) : operand1{a}, operand2{b} {} + explicit ExprOr(Expr a, Expr b) : operand1{a}, operand2{b} {} bool operator==(const ExprOr& b) const; @@ -48,7 +48,7 @@ public: class ExprNot final { public: - ExprNot(Expr a) : operand1{a} {} + explicit ExprNot(Expr a) : operand1{a} {} bool operator==(const ExprNot& b) const; @@ -57,7 +57,7 @@ public: class ExprVar final { public: - ExprVar(u32 index) : var_index{index} {} + explicit ExprVar(u32 index) : var_index{index} {} bool operator==(const ExprVar& b) const { return var_index == b.var_index; @@ -68,7 +68,7 @@ public: class ExprPredicate final { public: - ExprPredicate(u32 predicate) : predicate{predicate} {} + explicit ExprPredicate(u32 predicate) : predicate{predicate} {} bool operator==(const ExprPredicate& b) const { return predicate == b.predicate; @@ -79,7 +79,7 @@ public: class ExprCondCode final { public: - ExprCondCode(ConditionCode cc) : cc{cc} {} + explicit ExprCondCode(ConditionCode cc) : cc{cc} {} bool operator==(const ExprCondCode& b) const { return cc == b.cc; @@ -90,7 +90,7 @@ public: class ExprBoolean final { public: - ExprBoolean(bool val) : value{val} {} + explicit ExprBoolean(bool val) : value{val} {} bool operator==(const ExprBoolean& b) const { return value == b.value;