Shader_ir: Address feedback

6 files changed, 24 insertions, 65 deletions

diff --git a/src/video_core/renderer_opengl/gl_shader_decompiler.cpp b/src/video_core/renderer_opengl/gl_shader_decompiler.cpp
index bff1067a4..6a610a3bc 100644
--- a/src/video_core/renderer_opengl/gl_shader_decompiler.cpp
+++ b/src/video_core/renderer_opengl/gl_shader_decompiler.cpp
@@ -2271,7 +2271,11 @@ private:
     ShaderWriter code;
 };
 
-const std::string flow_var = "flow_var_";
+static constexpr std::string_view flow_var = "flow_var_";
+
+std::string GetFlowVariable(u32 i) {
+    return fmt::format("{}{}", flow_var, i);
+}
 
 class ExprDecompiler {
 public:
@@ -2326,7 +2330,7 @@ public:
     }
 
     void operator()(VideoCommon::Shader::ExprVar& expr) {
-        inner += flow_var + std::to_string(expr.var_index);
+        inner += GetFlowVariable(expr.var_index);
     }
 
     void operator()(VideoCommon::Shader::ExprBoolean& expr) {
@@ -2391,7 +2395,7 @@ public:
     void operator()(VideoCommon::Shader::ASTVarSet& ast) {
         ExprDecompiler expr_parser{decomp};
         std::visit(expr_parser, *ast.condition);
-        decomp.code.AddLine("{}{} = {};", flow_var, ast.index, expr_parser.GetResult());
+        decomp.code.AddLine("{} = {};", GetFlowVariable(ast.index), expr_parser.GetResult());
     }
 
     void operator()(VideoCommon::Shader::ASTLabel& ast) {
@@ -2462,7 +2466,7 @@ private:
 void GLSLDecompiler::DecompileAST() {
     const u32 num_flow_variables = ir.GetASTNumVariables();
     for (u32 i = 0; i < num_flow_variables; i++) {
-        code.AddLine("bool {}{} = false;", flow_var, i);
+        code.AddLine("bool {} = false;", GetFlowVariable(i));
     }
     ASTDecompiler decompiler{*this};
     VideoCommon::Shader::ASTNode program = ir.GetASTProgram();
diff --git a/src/video_core/renderer_opengl/gl_shader_gen.cpp b/src/video_core/renderer_opengl/gl_shader_gen.cpp
index 72a49ebdc..b5a43e79e 100644
--- a/src/video_core/renderer_opengl/gl_shader_gen.cpp
+++ b/src/video_core/renderer_opengl/gl_shader_gen.cpp
@@ -19,6 +19,8 @@ using VideoCommon::Shader::ShaderIR;
 static constexpr u32 PROGRAM_OFFSET = 10;
 static constexpr u32 COMPUTE_OFFSET = 0;
 
+static constexpr CompilerSettings settings{CompileDepth::NoFlowStack, true};
+
 ProgramResult GenerateVertexShader(const Device& device, const ShaderSetup& setup) {
     const std::string id = fmt::format("{:016x}", setup.program.unique_identifier);
 
@@ -33,9 +35,6 @@ layout (std140, binding = EMULATION_UBO_BINDING) uniform vs_config {
 
 )";
 
-    CompilerSettings settings;
-    settings.depth = CompileDepth::NoFlowStack;
-
     const ShaderIR program_ir(setup.program.code, PROGRAM_OFFSET, setup.program.size_a, settings);
     const auto stage = setup.IsDualProgram() ? ProgramType::VertexA : ProgramType::VertexB;
     ProgramResult program = Decompile(device, program_ir, stage, "vertex");
@@ -86,9 +85,6 @@ layout (std140, binding = EMULATION_UBO_BINDING) uniform gs_config {
 
 )";
 
-    CompilerSettings settings;
-    settings.depth = CompileDepth::NoFlowStack;
-
     const ShaderIR program_ir(setup.program.code, PROGRAM_OFFSET, setup.program.size_a, settings);
     ProgramResult program = Decompile(device, program_ir, ProgramType::Geometry, "geometry");
     out += program.first;
@@ -123,8 +119,6 @@ layout (std140, binding = EMULATION_UBO_BINDING) uniform fs_config {
 };
 
 )";
-    CompilerSettings settings;
-    settings.depth = CompileDepth::NoFlowStack;
 
     const ShaderIR program_ir(setup.program.code, PROGRAM_OFFSET, setup.program.size_a, settings);
     ProgramResult program = Decompile(device, program_ir, ProgramType::Fragment, "fragment");
@@ -145,9 +139,6 @@ ProgramResult GenerateComputeShader(const Device& device, const ShaderSetup& set
     std::string out = "// Shader Unique Id: CS" + id + "\n\n";
     out += GetCommonDeclarations();
 
-    CompilerSettings settings;
-    settings.depth = CompileDepth::NoFlowStack;
-
     const ShaderIR program_ir(setup.program.code, COMPUTE_OFFSET, setup.program.size_a, settings);
     ProgramResult program = Decompile(device, program_ir, ProgramType::Compute, "compute");
     out += program.first;
diff --git a/src/video_core/shader/ast.cpp b/src/video_core/shader/ast.cpp
index c4548f0bc..2eb065c3d 100644
--- a/src/video_core/shader/ast.cpp
+++ b/src/video_core/shader/ast.cpp
@@ -376,7 +376,7 @@ void ASTManager::Init() {
     false_condition = MakeExpr<ExprBoolean>(false);
 }
 
-ASTManager::ASTManager(ASTManager&& other)
+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},
@@ -384,7 +384,7 @@ ASTManager::ASTManager(ASTManager&& other)
     other.main_node.reset();
 }
 
-ASTManager& ASTManager::operator=(ASTManager&& other) {
+ASTManager& ASTManager::operator=(ASTManager&& other) noexcept {
     full_decompile = other.full_decompile;
     labels_map = std::move(other.labels_map);
     labels_count = other.labels_count;
@@ -490,7 +490,7 @@ void ASTManager::Decompile() {
         it++;
     }
     if (full_decompile) {
-        for (const ASTNode label : labels) {
+        for (const ASTNode& label : labels) {
             auto& manager = label->GetManager();
             manager.Remove(label);
         }
@@ -500,12 +500,12 @@ void ASTManager::Decompile() {
         while (it != labels.end()) {
             bool can_remove = true;
             ASTNode label = *it;
-            for (const ASTNode goto_node : gotos) {
+            for (const ASTNode& goto_node : gotos) {
                 const auto label_index = goto_node->GetGotoLabel();
                 if (!label_index) {
                     return;
                 }
-                ASTNode glabel = labels[*label_index];
+                ASTNode& glabel = labels[*label_index];
                 if (glabel == label) {
                     can_remove = false;
                     break;
@@ -543,40 +543,6 @@ bool ASTManager::IsBackwardsJump(ASTNode goto_node, ASTNode label_node) const {
     return false;
 }
 
-ASTNode CommonParent(ASTNode first, ASTNode second) {
-    if (first->GetParent() == second->GetParent()) {
-        return first->GetParent();
-    }
-    const u32 first_level = first->GetLevel();
-    const u32 second_level = second->GetLevel();
-    u32 min_level;
-    u32 max_level;
-    ASTNode max;
-    ASTNode min;
-    if (first_level > second_level) {
-        min_level = second_level;
-        min = second;
-        max_level = first_level;
-        max = first;
-    } else {
-        min_level = first_level;
-        min = first;
-        max_level = second_level;
-        max = second;
-    }
-
-    while (max_level > min_level) {
-        max_level--;
-        max = max->GetParent();
-    }
-
-    while (min->GetParent() != max->GetParent()) {
-        min = min->GetParent();
-        max = max->GetParent();
-    }
-    return min->GetParent();
-}
-
 bool ASTManager::IndirectlyRelated(ASTNode first, ASTNode second) {
     return !(first->GetParent() == second->GetParent() || DirectlyRelated(first, second));
 }
@@ -608,7 +574,7 @@ bool ASTManager::DirectlyRelated(ASTNode first, ASTNode second) {
         max = max->GetParent();
     }
 
-    return (min->GetParent() == max->GetParent());
+    return min->GetParent() == max->GetParent();
 }
 
 void ASTManager::ShowCurrentState(std::string state) {
@@ -617,7 +583,7 @@ void ASTManager::ShowCurrentState(std::string state) {
 }
 
 void ASTManager::SanityCheck() {
-    for (auto label : labels) {
+    for (auto& label : labels) {
         if (!label->GetParent()) {
             LOG_CRITICAL(HW_GPU, "Sanity Check Failed");
         }
diff --git a/src/video_core/shader/ast.h b/src/video_core/shader/ast.h
index 8efd4c147..ba234138e 100644
--- a/src/video_core/shader/ast.h
+++ b/src/video_core/shader/ast.h
@@ -97,7 +97,7 @@ public:
 
 class ASTBlockDecoded {
 public:
-    explicit ASTBlockDecoded(NodeBlock& new_nodes) : nodes(std::move(new_nodes)) {}
+    explicit ASTBlockDecoded(NodeBlock&& new_nodes) : nodes(std::move(new_nodes)) {}
     NodeBlock nodes;
 };
 
@@ -255,8 +255,8 @@ public:
         return std::holds_alternative<ASTBlockEncoded>(data);
     }
 
-    void TransformBlockEncoded(NodeBlock& nodes) {
-        data = ASTBlockDecoded(nodes);
+    void TransformBlockEncoded(NodeBlock&& nodes) {
+        data = ASTBlockDecoded(std::move(nodes));
     }
 
     bool IsLoop() const {
@@ -304,8 +304,8 @@ public:
     ASTManager(const ASTManager& o) = delete;
     ASTManager& operator=(const ASTManager& other) = delete;
 
-    ASTManager(ASTManager&& other);
-    ASTManager& operator=(ASTManager&& other);
+    ASTManager(ASTManager&& other) noexcept;
+    ASTManager& operator=(ASTManager&& other) noexcept;
 
     void Init();
 
@@ -362,8 +362,6 @@ public:
 private:
     bool IsBackwardsJump(ASTNode goto_node, ASTNode label_node) const;
 
-    ASTNode CommonParent(ASTNode first, ASTNode second);
-
     bool IndirectlyRelated(ASTNode first, ASTNode second);
 
     bool DirectlyRelated(ASTNode first, ASTNode second);
diff --git a/src/video_core/shader/control_flow.cpp b/src/video_core/shader/control_flow.cpp
index c2fa734e7..3c3a41ba6 100644
--- a/src/video_core/shader/control_flow.cpp
+++ b/src/video_core/shader/control_flow.cpp
@@ -58,7 +58,7 @@ struct BlockInfo {
 struct CFGRebuildState {
     explicit CFGRebuildState(const ProgramCode& program_code, const std::size_t program_size,
                              const u32 start)
-        : program_code{program_code}, program_size{program_size}, start{start} {}
+        : start{start}, program_code{program_code}, program_size{program_size} {}
 
     u32 start{};
     std::vector<BlockInfo> block_info{};
diff --git a/src/video_core/shader/decode.cpp b/src/video_core/shader/decode.cpp
index 6d4359295..2626b1616 100644
--- a/src/video_core/shader/decode.cpp
+++ b/src/video_core/shader/decode.cpp
@@ -90,7 +90,7 @@ public:
         if (node->IsBlockEncoded()) {
             auto block = std::get_if<ASTBlockEncoded>(node->GetInnerData());
             NodeBlock bb = ir.DecodeRange(block->start, block->end);
-            node->TransformBlockEncoded(bb);
+            node->TransformBlockEncoded(std::move(bb));
         }
     }