JSSpecCompiler: Save references to return value and function arguments

Author: https://github.com/DanShaders
Commit: https://github.com/SerenityOS/serenity/commit/5825eaa264
Pull-request: https://github.com/SerenityOS/serenity/pull/22751
Reviewed-by: https://github.com/ADKaster ✅

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/CFGBuildingPass.cpp

@@ -16,7 +16,7 @@ void CFGBuildingPass::process_function()
     m_current_block = m_cfg->start_block = create_empty_block();
     m_cfg->end_block = create_empty_block();
     m_cfg->end_block->m_continuation = make_ref_counted<ControlFlowFunctionReturn>(
-        make_ref_counted<Variable>(m_function->m_return_value));
+        make_ref_counted<Variable>(m_function->m_named_return_value));
     m_is_expression_stack = { false };
 
     run_in_subtree(m_function->m_ast);
@@ -25,7 +25,7 @@ void CFGBuildingPass::process_function()
     //        error_tree will 100% confuse future passes.
     m_current_block->m_expressions.append(make_ref_counted<BinaryOperation>(
         BinaryOperator::Assignment,
-        make_ref_counted<Variable>(m_function->m_return_value),
+        make_ref_counted<Variable>(m_function->m_named_return_value),
         error_tree));
     m_current_block->m_continuation = make_ref_counted<ControlFlowJump>(m_cfg->end_block);
 }
@@ -63,7 +63,7 @@ RecursionDecision CFGBuildingPass::on_entry(Tree tree)
     if (auto return_node = as<ReturnNode>(tree); return_node) {
         Tree return_assignment = make_ref_counted<BinaryOperation>(
             BinaryOperator::Assignment,
-            make_ref_counted<Variable>(m_function->m_return_value),
+            make_ref_counted<Variable>(m_function->m_named_return_value),
             return_node->m_return_value);
         run_in_subtree(return_assignment);
         auto* return_block = exchange_current_with_empty();

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/ReferenceResolvingPass.cpp

@@ -12,6 +12,13 @@
 
 namespace JSSpecCompiler {
 
+void ReferenceResolvingPass::process_function()
+{
+    for (auto name : m_function->m_argument_names)
+        m_function->m_local_variables.set(name, make_ref_counted<NamedVariableDeclaration>(name));
+    GenericASTPass::process_function();
+}
+
 RecursionDecision ReferenceResolvingPass::on_entry(Tree tree)
 {
     if (auto binary_operation = as<BinaryOperation>(tree); binary_operation) {

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/ReferenceResolvingPass.h

@@ -19,8 +19,8 @@ public:
     using GenericASTPass::GenericASTPass;
 
 protected:
+    void process_function() override;
     RecursionDecision on_entry(Tree tree) override;
-
     void on_leave(Tree tree) override;
 };
 

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/SSABuildingPass.cpp

@@ -279,7 +279,7 @@ void SSABuildingPass::place_phi_nodes()
     OrderedHashMap<NamedVariableDeclarationRef, Vector<BasicBlockRef>> m_declarations;
     for (auto const& [name, var_decl] : m_function->m_local_variables)
         m_declarations.set(var_decl, { m_order[0] });
-    m_declarations.set(m_function->m_return_value, { m_order[0] });
+    m_declarations.set(m_function->m_named_return_value, { m_order[0] });
 
     VariableAssignmentCollector collector(m_declarations);
     for (auto const& block : m_order)
@@ -414,6 +414,12 @@ void SSABuildingPass::rename_variables(Vertex u, Vertex from)
         });
     renamer.run(u.block());
 
+    if (auto function_return = as<ControlFlowFunctionReturn>(u.block()->m_continuation); function_return) {
+        // CFG should have exactly one ControlFlowFunctionReturn.
+        VERIFY(m_function->m_return_value == nullptr);
+        m_function->m_return_value = function_return->m_return_value->m_ssa;
+    }
+
     for (size_t j : u->outgoing_edges)
         rename_variables(j, u);
 
@@ -423,12 +429,24 @@ void SSABuildingPass::rename_variables(Vertex u, Vertex from)
 
 void SSABuildingPass::rename_variables()
 {
-    for (auto const& [name, var_decl] : m_function->m_local_variables)
+    HashMap<StringView, size_t> argument_index_by_name;
+    for (size_t i = 0; i < m_function->m_argument_names.size(); ++i)
+        argument_index_by_name.set(m_function->m_argument_names[i], i);
+    m_function->m_arguments.resize(m_function->m_argument_names.size());
+
+    for (auto const& [name, var_decl] : m_function->m_local_variables) {
         make_new_ssa_variable_for(var_decl);
-    make_new_ssa_variable_for(m_function->m_return_value);
+
+        if (auto maybe_index = argument_index_by_name.get(name); maybe_index.has_value()) {
+            size_t index = maybe_index.value();
+            m_function->m_arguments[index] = m_def_stack.get(var_decl).value()[0];
+        }
+    }
+    make_new_ssa_variable_for(m_function->m_named_return_value);
 
     ++m_mark_version;
     rename_variables(0);
+    VERIFY(m_function->m_return_value);
     m_function->reindex_ssa_variables();
 }
 

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Function.cpp

@@ -30,10 +30,11 @@ FunctionDeclaration::FunctionDeclaration(StringView name)
 {
 }
 
-FunctionDefinition::FunctionDefinition(StringView name, Tree ast)
+FunctionDefinition::FunctionDefinition(StringView name, Tree ast, Vector<StringView>&& argument_names)
     : FunctionDeclaration(name)
     , m_ast(move(ast))
-    , m_return_value(make_ref_counted<NamedVariableDeclaration>("$return"sv))
+    , m_argument_names(move(argument_names))
+    , m_named_return_value(make_ref_counted<NamedVariableDeclaration>("$return"sv))
 {
 }
 

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Function.h

@@ -37,17 +37,26 @@ public:
 
 class FunctionDefinition : public FunctionDeclaration {
 public:
-    FunctionDefinition(StringView name, Tree ast);
+    FunctionDefinition(StringView name, Tree ast, Vector<StringView>&& argument_names);
 
     void reindex_ssa_variables();
 
     Tree m_ast;
-    NamedVariableDeclarationRef m_return_value;
+    Vector<StringView> m_argument_names;
+
+    // Populates during reference resolving
     // NOTE: The hash map here is ordered since we do not want random hash changes to break our test
     //       expectations (looking at you, SipHash).
     OrderedHashMap<StringView, NamedVariableDeclarationRef> m_local_variables;
-    Vector<SSAVariableDeclarationRef> m_local_ssa_variables;
+
+    // Fields populate during CFG building
+    NamedVariableDeclarationRef m_named_return_value;
     RefPtr<ControlFlowGraph> m_cfg;
+
+    // Fields populate during SSA building
+    Vector<SSAVariableDeclarationRef> m_arguments;
+    SSAVariableDeclarationRef m_return_value;
+    Vector<SSAVariableDeclarationRef> m_local_ssa_variables;
 };
 
 }

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/CppASTConverter.cpp

@@ -24,7 +24,11 @@ NonnullRefPtr<FunctionDefinition> CppASTConverter::convert()
     }
     auto tree = make_ref_counted<TreeList>(move(toplevel_statements));
 
-    return make_ref_counted<FunctionDefinition>(name, tree);
+    Vector<StringView> arguments;
+    for (auto const& parameter : m_function->parameters())
+        arguments.append(parameter->full_name());
+
+    return make_ref_counted<FunctionDefinition>(name, tree, move(arguments));
 }
 
 template<>

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/SpecParser.cpp

@@ -196,11 +196,12 @@ void SpecParsingStep::run(TranslationUnitRef translation_unit)
 
     auto spec_function = SpecFunction::create(&m_document->root()).release_value_but_fixme_should_propagate_errors();
 
-    auto* function = translation_unit->adopt_function(
-        make_ref_counted<FunctionDefinition>(spec_function.m_name, spec_function.m_algorithm.m_tree));
-
+    Vector<StringView> argument_names;
     for (auto const& argument : spec_function.m_arguments)
-        function->m_local_variables.set(argument.name, make_ref_counted<NamedVariableDeclaration>(argument.name));
+        argument_names.append(argument.name);
+
+    translation_unit->adopt_function(
+        make_ref_counted<FunctionDefinition>(spec_function.m_name, spec_function.m_algorithm.m_tree, move(argument_names)));
 }
 
 }

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Tests/simple.cpp.expectation

@@ -1,5 +1,5 @@
 ===== AST after parser =====
-f():
+f([ cond1, cond2 ]):
 TreeList
   IfBranch
     UnresolvedReference cond1
@@ -27,7 +27,7 @@ TreeList
     UnresolvedReference b
 
 ===== AST after function-call-canonicalization =====
-f():
+f([ cond1, cond2 ]):
 TreeList
   IfBranch
     UnresolvedReference cond1
@@ -55,7 +55,7 @@ TreeList
     UnresolvedReference b
 
 ===== AST after if-branch-merging =====
-f():
+f([ cond1, cond2 ]):
 TreeList
   IfElseIfChain
     UnresolvedReference cond1
@@ -81,16 +81,16 @@ TreeList
     UnresolvedReference b
 
 ===== AST after reference-resolving =====
-f():
+f([ cond1, cond2 ]):
 TreeList
   IfElseIfChain
-    UnresolvedReference cond1
+    Var cond1
     TreeList
       BinaryOperation Assignment
         Var a
         MathematicalConstant 1
       IfElseIfChain
-        UnresolvedReference cond2
+        Var cond2
         TreeList
           BinaryOperation Assignment
             Var b
@@ -107,16 +107,16 @@ TreeList
     Var b
 
 ===== AST after cfg-building =====
-f():
+f([ cond1, cond2 ]):
 TreeList
   IfElseIfChain
-    UnresolvedReference cond1
+    Var cond1
     TreeList
       BinaryOperation Assignment
         Var a
         MathematicalConstant 1
       IfElseIfChain
-        UnresolvedReference cond2
+        Var cond2
         TreeList
           BinaryOperation Assignment
             Var b
@@ -133,10 +133,10 @@ TreeList
     Var b
 
 ===== CFG after cfg-building =====
-f():
+f([ cond1, cond2 ]):
 0:
 ControlFlowBranch true=3 false=7
-  UnresolvedReference cond1
+  Var cond1
 
 1:
 ControlFlowFunctionReturn
@@ -153,7 +153,7 @@ BinaryOperation Assignment
   Var a
   MathematicalConstant 1
 ControlFlowBranch true=5 false=6
-  UnresolvedReference cond2
+  Var cond2
 
 4:
 ControlFlowJump jump=2
@@ -183,16 +183,16 @@ BinaryOperation Assignment
 ControlFlowJump jump=1
 
 ===== AST after cfg-simplification =====
-f():
+f([ cond1, cond2 ]):
 TreeList
   IfElseIfChain
-    UnresolvedReference cond1
+    Var cond1
     TreeList
       BinaryOperation Assignment
         Var a
         MathematicalConstant 1
       IfElseIfChain
-        UnresolvedReference cond2
+        Var cond2
         TreeList
           BinaryOperation Assignment
             Var b
@@ -209,10 +209,10 @@ TreeList
     Var b
 
 ===== CFG after cfg-simplification =====
-f():
+f([ cond1, cond2 ]):
 0:
 ControlFlowBranch true=3 false=6
-  UnresolvedReference cond1
+  Var cond1
 
 1:
 ControlFlowFunctionReturn
@@ -229,7 +229,7 @@ BinaryOperation Assignment
   Var a
   MathematicalConstant 1
 ControlFlowBranch true=4 false=5
-  UnresolvedReference cond2
+  Var cond2
 
 4:
 BinaryOperation Assignment
@@ -250,16 +250,16 @@ BinaryOperation Assignment
 ControlFlowJump jump=2
 
 ===== AST after ssa-building =====
-f():
+f([ cond1, cond2 ]):
 TreeList
   IfElseIfChain
-    UnresolvedReference cond1
+    Var cond1@0
     TreeList
       BinaryOperation Assignment
         Var a@1
         MathematicalConstant 1
       IfElseIfChain
-        UnresolvedReference cond2
+        Var cond2@0
         TreeList
           BinaryOperation Assignment
             Var b@1
@@ -276,17 +276,17 @@ TreeList
     Var b@2
 
 ===== CFG after ssa-building =====
-f():
+f([ cond1, cond2 ]):
 0:
 ControlFlowBranch true=1 false=6
-  UnresolvedReference cond1
+  Var cond1@0
 
 1:
 BinaryOperation Assignment
   Var a@1
   MathematicalConstant 1
 ControlFlowBranch true=2 false=5
-  UnresolvedReference cond2
+  Var cond2@0
 
 2:
 BinaryOperation Assignment
@@ -319,16 +319,16 @@ BinaryOperation Assignment
 ControlFlowJump jump=3
 
 ===== AST after dce =====
-f():
+f([ cond1, cond2 ]):
 TreeList
   IfElseIfChain
-    UnresolvedReference cond1
+    Var cond1@0
     TreeList
       BinaryOperation Assignment
         Var a@1
         MathematicalConstant 1
       IfElseIfChain
-        UnresolvedReference cond2
+        Var cond2@0
         TreeList
           BinaryOperation Assignment
             Var b@1
@@ -345,17 +345,17 @@ TreeList
     Var b@2
 
 ===== CFG after dce =====
-f():
+f([ cond1, cond2 ]):
 0:
 ControlFlowBranch true=1 false=6
-  UnresolvedReference cond1
+  Var cond1@0
 
 1:
 BinaryOperation Assignment
   Var a@1
   MathematicalConstant 1
 ControlFlowBranch true=2 false=5
-  UnresolvedReference cond2
+  Var cond2@0
 
 2:
 BinaryOperation Assignment

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/main.cpp

@@ -140,14 +140,14 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         if (step.dump_ast) {
             outln(stderr, "===== AST after {} =====", step.step->name());
             for (auto const& function : translation_unit.functions_to_compile) {
-                outln(stderr, "{}():", function->m_name);
+                outln(stderr, "{}({}):", function->m_name, function->m_argument_names);
                 outln(stderr, "{}", function->m_ast);
             }
         }
         if (step.dump_cfg && translation_unit.functions_to_compile[0]->m_cfg != nullptr) {
             outln(stderr, "===== CFG after {} =====", step.step->name());
             for (auto const& function : translation_unit.functions_to_compile) {
-                outln(stderr, "{}():", function->m_name);
+                outln(stderr, "{}({}):", function->m_name, function->m_argument_names);
                 outln(stderr, "{}", *function->m_cfg);
             }
         }