LibJS: Skip allocating locals for arguments that allowed to be local

This allows us to get rid of instructions that move arguments to locals
and allocate smaller JS::Value vector in ExecutionContext by reusing
slots that were already allocated for arguments.

With this change for following function:
```js
function f(x, y) {
    return x + y;
}
```

we now produce following bytecode:
```
[   0]    0: Add dst:reg6, lhs:arg0, rhs:arg1
[  10]       Return value:reg6
```

instead of:
```
[   0]    0: GetArgument 0, dst:x~1
[  10]       GetArgument 1, dst:y~0
[  20]       Add dst:reg6, lhs:x~1, rhs:y~0
[  30]       Return value:reg6
```

Libraries/LibJS/AST.cpp

@@ -1027,7 +1027,7 @@ void Identifier::dump(int indent) const
 {
     print_indent(indent);
     if (is_local()) {
-        outln("Identifier \"{}\" is_local=(true) index=({})", m_string, m_local_variable_index);
+        outln("Identifier \"{}\" is_local=(true) index=({})", m_string, m_local_index->index);
     } else if (is_global()) {
         outln("Identifier \"{}\" is_global=(true)", m_string);
     } else {
@@ -1668,7 +1668,12 @@ void ScopeNode::block_declaration_instantiation(VM& vm, Environment* environment
                 auto& running_execution_context = vm.running_execution_context();
                 auto number_of_registers = running_execution_context.executable->number_of_registers;
                 auto number_of_constants = running_execution_context.executable->constants.size();
-                running_execution_context.local(function_declaration.name_identifier()->local_variable_index() + number_of_registers + number_of_constants) = function;
+                auto local_index = function_declaration.name_identifier()->local_index();
+                if (local_index.is_variable()) {
+                    running_execution_context.local(local_index.index + number_of_registers + number_of_constants) = function;
+                } else {
+                    VERIFY_NOT_REACHED();
+                }
             } else {
                 VERIFY(is<DeclarativeEnvironment>(*environment));
                 static_cast<DeclarativeEnvironment&>(*environment).initialize_or_set_mutable_binding({}, vm, function_declaration.name(), function);

Libraries/LibJS/AST.h

@@ -675,13 +675,29 @@ public:
 
     FlyString const& string() const { return m_string; }
 
-    bool is_local() const { return m_local_variable_index.has_value(); }
+    struct Local {
-    size_t local_variable_index() const
+        enum Type {
+            Argument,
+            Variable,
+        };
+        Type type;
+        size_t index;
+
+        bool is_argument() const { return type == Argument; }
+        bool is_variable() const { return type == Variable; }
+
+        static Local variable(size_t index) { return { Variable, index }; }
+        static Local argument(size_t index) { return { Argument, index }; }
+    };
+
+    bool is_local() const { return m_local_index.has_value(); }
+    Local local_index() const
     {
-        VERIFY(m_local_variable_index.has_value());
+        VERIFY(m_local_index.has_value());
-        return m_local_variable_index.value();
+        return m_local_index.value();
     }
-    void set_local_variable_index(size_t index) { m_local_variable_index = index; }
+    void set_local_variable_index(size_t index) { m_local_index = Local::variable(index); }
+    void set_argument_index(size_t index) { m_local_index = Local::argument(index); }
 
     bool is_global() const { return m_is_global; }
     void set_is_global() { m_is_global = true; }
@@ -694,7 +710,7 @@ private:
 
     FlyString m_string;
 
-    Optional<size_t> m_local_variable_index;
+    Optional<Local> m_local_index;
     bool m_is_global { false };
 };
 
@@ -719,6 +735,20 @@ public:
     size_t size() const { return m_parameters.size(); }
     Vector<FunctionParameter> const& parameters() const { return m_parameters; }
 
+    Optional<size_t> get_index_of_parameter_name(FlyString const& name) const
+    {
+        // Iterate backwards to return the last parameter with the same name
+        for (int i = m_parameters.size() - 1; i >= 0; i--) {
+            auto& parameter = m_parameters[i];
+            if (parameter.binding.has<NonnullRefPtr<Identifier const>>()) {
+                auto& identifier = parameter.binding.get<NonnullRefPtr<Identifier const>>();
+                if (identifier->string() == name)
+                    return i;
+            }
+        }
+        return {};
+    }
+
 private:
     FunctionParameters(Vector<FunctionParameter> parameters)
         : m_parameters(move(parameters))

Libraries/LibJS/Bytecode/ASTCodegen.cpp

@@ -461,8 +461,14 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> Identifier::generate_by
     Bytecode::Generator::SourceLocationScope scope(generator, *this);
 
     if (is_local()) {
-        auto local = generator.local(local_variable_index());
+        auto local_index = this->local_index();
-        if (!generator.is_local_initialized(local_variable_index())) {
+        auto local = generator.local(local_index);
+        if (!generator.is_local_initialized(local_index)) {
+            if (local_index.is_argument()) {
+                // Arguments are initialized to undefined by default, so here we need to replace it with the empty value to
+                // trigger the TDZ check.
+                generator.emit<Bytecode::Op::Mov>(local, generator.add_constant(js_special_empty_value()));
+            }
             generator.emit<Bytecode::Op::ThrowIfTDZ>(local);
         }
         return local;
@@ -1605,7 +1611,7 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> VariableDeclaration::ge
         if (declaration_kind() != DeclarationKind::Var) {
             if (auto const* identifier = declarator->target().get_pointer<NonnullRefPtr<Identifier const>>()) {
                 if ((*identifier)->is_local()) {
-                    init_dst = generator.local((*identifier)->local_variable_index());
+                    init_dst = generator.local((*identifier)->local_index());
                 }
             }
         }
@@ -1625,7 +1631,7 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> VariableDeclaration::ge
 
         if (auto const* identifier = declarator->target().get_pointer<NonnullRefPtr<Identifier const>>()) {
             if ((*identifier)->is_local()) {
-                generator.set_local_initialized((*identifier)->local_variable_index());
+                generator.set_local_initialized((*identifier)->local_index());
             }
         }
     }
@@ -1736,7 +1742,7 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> CallExpression::generat
             call_type = Bytecode::Op::CallType::DirectEval;
         }
         if (identifier.is_local()) {
-            auto local = generator.local(identifier.local_variable_index());
+            auto local = generator.local(identifier.local_index());
             if (!generator.is_local_initialized(local.operand().index())) {
                 generator.emit<Bytecode::Op::ThrowIfTDZ>(local);
             }
@@ -2648,7 +2654,7 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> TryStatement::generate_
         TRY(m_handler->parameter().visit(
             [&](NonnullRefPtr<Identifier const> const& parameter) -> Bytecode::CodeGenerationErrorOr<void> {
                 if (parameter->is_local()) {
-                    auto local = generator.local(parameter->local_variable_index());
+                    auto local = generator.local(parameter->local_index());
                     generator.emit_mov(local, caught_value);
                 } else {
                     generator.begin_variable_scope();

Libraries/LibJS/Bytecode/Generator.cpp

@@ -58,7 +58,7 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
         Optional<Operand> dst;
         auto local_var_index = function.shared_data().m_local_variables_names.find_first_index("arguments"_fly_string);
         if (local_var_index.has_value())
-            dst = local(local_var_index.value());
+            dst = local(Identifier::Local::variable(local_var_index.value()));
 
         if (function.is_strict_mode() || !function.has_simple_parameter_list()) {
             emit<Op::CreateArguments>(dst, Op::CreateArguments::Kind::Unmapped, function.is_strict_mode());
@@ -77,11 +77,8 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
             auto& if_undefined_block = make_block();
             auto& if_not_undefined_block = make_block();
 
-            auto argument_reg = allocate_register();
-            emit<Op::Mov>(argument_reg.operand(), Operand { Operand::Type::Argument, param_index });
-
             emit<Op::JumpUndefined>(
-                argument_reg.operand(),
+                Operand { Operand::Type::Argument, param_index },
                 Label { if_undefined_block },
                 Label { if_not_undefined_block });
 
@@ -95,9 +92,7 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
 
         if (auto const* identifier = parameter.binding.get_pointer<NonnullRefPtr<Identifier const>>(); identifier) {
             if ((*identifier)->is_local()) {
-                auto local_variable_index = (*identifier)->local_variable_index();
+                set_local_initialized((*identifier)->local_index());
-                emit<Op::Mov>(local(local_variable_index), Operand { Operand::Type::Argument, param_index });
-                set_local_initialized((*identifier)->local_variable_index());
             } else {
                 auto id = intern_identifier((*identifier)->string());
                 if (function.shared_data().m_has_duplicates) {
@@ -122,7 +117,7 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
             for (auto const& variable_to_initialize : function.shared_data().m_var_names_to_initialize_binding) {
                 auto const& id = variable_to_initialize.identifier;
                 if (id.is_local()) {
-                    emit<Op::Mov>(local(id.local_variable_index()), add_constant(js_undefined()));
+                    emit<Op::Mov>(local(id.local_index()), add_constant(js_undefined()));
                 } else {
                     auto intern_id = intern_identifier(id.string());
                     emit<Op::CreateVariable>(intern_id, Op::EnvironmentMode::Var, false);
@@ -153,14 +148,14 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
                     emit<Op::Mov>(initial_value, add_constant(js_undefined()));
                 } else {
                     if (id.is_local()) {
-                        emit<Op::Mov>(initial_value, local(id.local_variable_index()));
+                        emit<Op::Mov>(initial_value, local(id.local_index()));
                     } else {
                         emit<Op::GetBinding>(initial_value, intern_identifier(id.string()));
                     }
                 }
 
                 if (id.is_local()) {
-                    emit<Op::Mov>(local(id.local_variable_index()), initial_value);
+                    emit<Op::Mov>(local(id.local_index()), initial_value);
                 } else {
                     auto intern_id = intern_identifier(id.string());
                     emit<Op::CreateVariable>(intern_id, Op::EnvironmentMode::Var, false);
@@ -204,7 +199,7 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
     for (auto const& declaration : function.shared_data().m_functions_to_initialize) {
         auto const& identifier = *declaration.name_identifier();
         if (identifier.is_local()) {
-            auto local_index = identifier.local_variable_index();
+            auto local_index = identifier.local_index();
             emit<Op::NewFunction>(local(local_index), declaration, OptionalNone {});
             set_local_initialized(local_index);
         } else {
@@ -516,9 +511,11 @@ void Generator::free_register(Register reg)
     m_free_registers.append(reg);
 }
 
-ScopedOperand Generator::local(u32 local_index)
+ScopedOperand Generator::local(Identifier::Local const& local)
 {
-    return ScopedOperand { *this, Operand { Operand::Type::Local, static_cast<u32>(local_index) } };
+    if (local.is_variable())
+        return ScopedOperand { *this, Operand { Operand::Type::Local, static_cast<u32>(local.index) } };
+    return ScopedOperand { *this, Operand { Operand::Type::Argument, static_cast<u32>(local.index) } };
 }
 
 Generator::SourceLocationScope::SourceLocationScope(Generator& generator, ASTNode const& node)
@@ -880,11 +877,12 @@ CodeGenerationErrorOr<Optional<ScopedOperand>> Generator::emit_delete_reference(
 void Generator::emit_set_variable(JS::Identifier const& identifier, ScopedOperand value, Bytecode::Op::BindingInitializationMode initialization_mode, Bytecode::Op::EnvironmentMode environment_mode)
 {
     if (identifier.is_local()) {
-        if (value.operand().is_local() && value.operand().index() == identifier.local_variable_index()) {
+        auto local_index = identifier.local_index();
+        if (value.operand().is_local() && local_index.is_variable() && value.operand().index() == local_index.index) {
             // Moving a local to itself is a no-op.
             return;
         }
-        emit<Bytecode::Op::Mov>(local(identifier.local_variable_index()), value);
+        emit<Bytecode::Op::Mov>(local(local_index), value);
     } else {
         auto identifier_index = intern_identifier(identifier.string());
         if (environment_mode == Bytecode::Op::EnvironmentMode::Lexical) {
@@ -1186,9 +1184,24 @@ bool Generator::is_local_initialized(u32 local_index) const
     return m_initialized_locals.find(local_index) != m_initialized_locals.end();
 }
 
-void Generator::set_local_initialized(u32 local_index)
+bool Generator::is_local_initialized(Identifier::Local const& local) const
 {
-    m_initialized_locals.set(local_index);
+    if (local.is_variable())
+        return m_initialized_locals.find(local.index) != m_initialized_locals.end();
+    if (local.is_argument())
+        return m_initialized_arguments.find(local.index) != m_initialized_arguments.end();
+    return true;
+}
+
+void Generator::set_local_initialized(Identifier::Local const& local)
+{
+    if (local.is_variable()) {
+        m_initialized_locals.set(local.index);
+    } else if (local.is_argument()) {
+        m_initialized_arguments.set(local.index);
+    } else {
+        VERIFY_NOT_REACHED();
+    }
 }
 
 ScopedOperand Generator::get_this(Optional<ScopedOperand> preferred_dst)

Libraries/LibJS/Bytecode/Generator.h

@@ -44,14 +44,15 @@ public:
     CodeGenerationErrorOr<void> emit_function_declaration_instantiation(ECMAScriptFunctionObject const& function);
 
     [[nodiscard]] ScopedOperand allocate_register();
-    [[nodiscard]] ScopedOperand local(u32 local_index);
+    [[nodiscard]] ScopedOperand local(Identifier::Local const&);
     [[nodiscard]] ScopedOperand accumulator();
     [[nodiscard]] ScopedOperand this_value();
 
     void free_register(Register);
 
-    void set_local_initialized(u32 local_index);
+    void set_local_initialized(Identifier::Local const&);
     [[nodiscard]] bool is_local_initialized(u32 local_index) const;
+    [[nodiscard]] bool is_local_initialized(Identifier::Local const&) const;
 
     class SourceLocationScope {
     public:
@@ -411,6 +412,7 @@ private:
     Vector<ScopedOperand> m_home_objects;
 
     HashTable<u32> m_initialized_locals;
+    HashTable<u32> m_initialized_arguments;
 
     bool m_finished { false };
     bool m_must_propagate_completion { true };

Libraries/LibJS/Parser.cpp

@@ -326,9 +326,10 @@ public:
                 scope_has_declaration = false;
             }
 
+            bool is_function_parameter = false;
             if (m_type == ScopeType::Function) {
                 if (!m_contains_access_to_arguments_object_in_non_strict_mode && m_function_parameters_candidates_for_local_variables.contains(identifier_group_name)) {
-                    scope_has_declaration = true;
+                    is_function_parameter = true;
                 } else if (m_forbidden_lexical_names.contains(identifier_group_name)) {
                     // NOTE: If an identifier is used as a function parameter that cannot be optimized locally or globally, it is simply ignored.
                     continue;
@@ -346,7 +347,7 @@ public:
                     for (auto& identifier : identifier_group.identifiers)
                         identifier->set_is_global();
                 }
-            } else if (scope_has_declaration) {
+            } else if (scope_has_declaration || is_function_parameter) {
                 if (hoistable_function_declaration)
                     continue;
 
@@ -368,9 +369,15 @@ public:
                         local_scope = m_top_level_scope;
                     }
 
-                    auto local_variable_index = local_scope->m_node->add_local_variable(identifier_group_name);
+                    if (is_function_parameter) {
-                    for (auto& identifier : identifier_group.identifiers)
+                        auto argument_index = local_scope->m_function_parameters->get_index_of_parameter_name(identifier_group_name);
-                        identifier->set_local_variable_index(local_variable_index);
+                        for (auto& identifier : identifier_group.identifiers)
+                            identifier->set_argument_index(argument_index.value());
+                    } else {
+                        auto local_variable_index = local_scope->m_node->add_local_variable(identifier_group_name);
+                        for (auto& identifier : identifier_group.identifiers)
+                            identifier->set_local_variable_index(local_variable_index);
+                    }
                 }
             } else {
                 if (m_function_parameters || m_type == ScopeType::ClassField || m_type == ScopeType::ClassStaticInit) {