LibWasm: Avoid excessive pop()-then-push() on the stack

Also make the stack a lot bigger, since we now have only one of these
instead of one per function call.

Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h

@@ -39,6 +39,12 @@ TYPEDEF_DISTINCT_NUMERIC_GENERAL(u64, true, true, false, false, false, true, Mem
 //        fancy than just a dumb interpreter.
 class Value {
 public:
+    Value()
+        : m_value(0)
+        , m_type(ValueType::I32)
+    {
+    }
+
     using AnyValueType = Variant<i32, i64, float, double, FunctionAddress, ExternAddress>;
     explicit Value(AnyValueType value)
         : m_value(move(value))
@@ -426,17 +432,18 @@ public:
     using EntryType = Variant<Value, Label, Frame>;
     Stack() = default;
 
-    [[nodiscard]] bool is_empty() const { return m_data.is_empty(); }
-    void push(EntryType entry) { m_data.append(move(entry)); }
-    auto pop() { return m_data.take_last(); }
-    auto& peek() const { return m_data.last(); }
+    [[nodiscard]] ALWAYS_INLINE bool is_empty() const { return m_data.is_empty(); }
+    FLATTEN void push(EntryType entry) { m_data.append(move(entry)); }
+    FLATTEN auto pop() { return m_data.take_last(); }
+    FLATTEN auto& peek() const { return m_data.last(); }
+    FLATTEN auto& peek() { return m_data.last(); }
 
-    auto size() const { return m_data.size(); }
-    auto& entries() const { return m_data; }
-    auto& entries() { return m_data; }
+    ALWAYS_INLINE auto size() const { return m_data.size(); }
+    ALWAYS_INLINE auto& entries() const { return m_data; }
+    ALWAYS_INLINE auto& entries() { return m_data; }
 
 private:
-    Vector<EntryType, 64> m_data;
+    Vector<EntryType, 1024> m_data;
 };
 
 using InstantiationResult = AK::Result<NonnullOwnPtr<ModuleInstance>, InstantiationError>;

Userland/Libraries/LibWasm/AbstractMachine/Interpreter.cpp

@@ -55,7 +55,6 @@ void BytecodeInterpreter::branch_to_label(Configuration& configuration, LabelInd
     auto results = pop_values(configuration, label->arity());
 
     size_t drop_count = index.value() + 1;
-
     for (; !configuration.stack().is_empty();) {
         auto& entry = configuration.stack().peek();
         if (entry.has<Label>()) {
@@ -71,28 +70,33 @@ void BytecodeInterpreter::branch_to_label(Configuration& configuration, LabelInd
     configuration.ip() = label->continuation();
 }
 
-ReadonlyBytes BytecodeInterpreter::load_from_memory(Configuration& configuration, const Instruction& instruction, size_t size)
+template<typename ReadType, typename PushType>
+void BytecodeInterpreter::load_and_push(Configuration& configuration, const Instruction& instruction)
 {
     auto& address = configuration.frame().module().memories().first();
     auto memory = configuration.store().get(address);
     if (!memory) {
         m_do_trap = true;
-        return {};
+        return;
     }
     auto& arg = instruction.arguments().get<Instruction::MemoryArgument>();
-    auto base = configuration.stack().pop().get<Value>().to<i32>();
+    auto base = configuration.stack().peek().get<Value>().to<i32>();
     if (!base.has_value()) {
         m_do_trap = true;
-        return {};
+        return;
     }
     auto instance_address = base.value() + static_cast<i64>(arg.offset);
-    if (instance_address < 0 || static_cast<u64>(instance_address + size) > memory->size()) {
+    if (instance_address < 0 || static_cast<u64>(instance_address + sizeof(ReadType)) > memory->size()) {
         m_do_trap = true;
-        dbgln("LibWasm: Memory access out of bounds (expected 0 <= {} and {} <= {})", instance_address, instance_address + size, memory->size());
-        return {};
+        dbgln("LibWasm: Memory access out of bounds (expected 0 <= {} and {} <= {})", instance_address, instance_address + sizeof(ReadType), memory->size());
+        return;
     }
-    dbgln_if(WASM_TRACE_DEBUG, "load({} : {}) -> stack", instance_address, size);
-    return memory->data().bytes().slice(instance_address, size);
+    dbgln_if(WASM_TRACE_DEBUG, "load({} : {}) -> stack", instance_address, sizeof(ReadType));
+    auto slice = memory->data().bytes().slice(instance_address, sizeof(ReadType));
+    if constexpr (sizeof(ReadType) == 1)
+        configuration.stack().peek() = Value(static_cast<PushType>(slice[0]));
+    else
+        configuration.stack().peek() = Value(read_value<PushType>(slice));
 }
 
 void BytecodeInterpreter::store_to_memory(Configuration& configuration, const Instruction& instruction, ReadonlyBytes data)
@@ -122,10 +126,15 @@ void BytecodeInterpreter::call_address(Configuration& configuration, FunctionAdd
     TRAP_IF_NOT(type);
     Vector<Value> args;
     args.ensure_capacity(type->parameters().size());
-    for (size_t i = 0; i < type->parameters().size(); ++i) {
-        args.prepend(move(configuration.stack().pop().get<Value>()));
+    auto span = configuration.stack().entries().span().slice_from_end(type->parameters().size());
+    for (auto& entry : span) {
+        auto* ptr = entry.get_pointer<Value>();
+        TRAP_IF_NOT(ptr != nullptr);
+        args.unchecked_append(*ptr);
     }
 
+    configuration.stack().entries().remove(configuration.stack().size() - span.size(), span.size());
+
     Result result { Trap {} };
     {
         CallFrameHandle handle { *this, configuration };
@@ -136,27 +145,29 @@ void BytecodeInterpreter::call_address(Configuration& configuration, FunctionAdd
         m_do_trap = true;
         return;
     }
+
+    configuration.stack().entries().ensure_capacity(configuration.stack().size() + result.values().size());
     for (auto& entry : result.values())
-        configuration.stack().push(Value(move(entry)));
+        configuration.stack().entries().unchecked_append(move(entry));
 }
 
 #define BINARY_NUMERIC_OPERATION(type, operator, cast, ...)                                       \
     do {                                                                                          \
         auto rhs = configuration.stack().pop().get<Value>().to<type>();                           \
-        auto lhs = configuration.stack().pop().get<Value>().to<type>();                           \
+        auto lhs = configuration.stack().peek().get<Value>().to<type>();                          \
         TRAP_IF_NOT(lhs.has_value());                                                             \
         TRAP_IF_NOT(rhs.has_value());                                                             \
         __VA_ARGS__;                                                                              \
         auto result = lhs.value() operator rhs.value();                                           \
         dbgln_if(WASM_TRACE_DEBUG, "{} {} {} = {}", lhs.value(), #operator, rhs.value(), result); \
-        configuration.stack().push(Value(cast(result)));                                          \
+        configuration.stack().peek() = Value(cast(result));                                       \
         return;                                                                                   \
     } while (false)
 
 #define OVF_CHECKED_BINARY_NUMERIC_OPERATION(type, operator, cast, ...)                            \
     do {                                                                                           \
         auto rhs = configuration.stack().pop().get<Value>().to<type>();                            \
-        auto ulhs = configuration.stack().pop().get<Value>().to<type>();                           \
+        auto ulhs = configuration.stack().peek().get<Value>().to<type>();                          \
         TRAP_IF_NOT(ulhs.has_value());                                                             \
         TRAP_IF_NOT(rhs.has_value());                                                              \
         dbgln_if(WASM_TRACE_DEBUG, "{} {} {} = ??", ulhs.value(), #operator, rhs.value());         \
@@ -166,44 +177,39 @@ void BytecodeInterpreter::call_address(Configuration& configuration, FunctionAdd
         TRAP_IF_NOT(!lhs.has_overflow());                                                          \
         auto result = lhs.value();                                                                 \
         dbgln_if(WASM_TRACE_DEBUG, "{} {} {} = {}", ulhs.value(), #operator, rhs.value(), result); \
-        configuration.stack().push(Value(cast(result)));                                           \
+        configuration.stack().peek() = Value(cast(result));                                        \
         return;                                                                                    \
     } while (false)
 
 #define BINARY_PREFIX_NUMERIC_OPERATION(type, operation, cast, ...)                                 \
     do {                                                                                            \
         auto rhs = configuration.stack().pop().get<Value>().to<type>();                             \
-        auto lhs = configuration.stack().pop().get<Value>().to<type>();                             \
+        auto lhs = configuration.stack().peek().get<Value>().to<type>();                            \
         TRAP_IF_NOT(lhs.has_value());                                                               \
         TRAP_IF_NOT(rhs.has_value());                                                               \
         auto result = operation(lhs.value(), rhs.value());                                          \
         dbgln_if(WASM_TRACE_DEBUG, "{}({} {}) = {}", #operation, lhs.value(), rhs.value(), result); \
-        configuration.stack().push(Value(cast(result)));                                            \
+        configuration.stack().peek() = Value(cast(result));                                         \
         return;                                                                                     \
     } while (false)
 
 #define UNARY_MAP(pop_type, operation, ...)                                               \
     do {                                                                                  \
-        auto value = configuration.stack().pop().get<Value>().to<pop_type>();             \
+        auto value = configuration.stack().peek().get<Value>().to<pop_type>();            \
         TRAP_IF_NOT(value.has_value());                                                   \
         auto result = operation(value.value());                                           \
         dbgln_if(WASM_TRACE_DEBUG, "map({}) {} = {}", #operation, value.value(), result); \
-        configuration.stack().push(Value(__VA_ARGS__(result)));                           \
+        configuration.stack().peek() = Value(__VA_ARGS__(result));                        \
         return;                                                                           \
     } while (false)
 
 #define UNARY_NUMERIC_OPERATION(type, operation) \
     UNARY_MAP(type, operation, type)
 
-#define LOAD_AND_PUSH(read_type, push_type)                                           \
-    do {                                                                              \
-        auto slice = load_from_memory(configuration, instruction, sizeof(read_type)); \
-        TRAP_IF_NOT(slice.size() == sizeof(read_type));                               \
-        if constexpr (sizeof(read_type) == 1)                                         \
-            configuration.stack().push(Value(static_cast<push_type>(slice[0])));      \
-        else                                                                          \
-            configuration.stack().push(Value(read_value<push_type>(slice)));          \
-        return;                                                                       \
+#define LOAD_AND_PUSH(read_type, push_type)                                     \
+    do {                                                                        \
+        return load_and_push<read_type, push_type>(configuration, instruction); \
+        return;                                                                 \
     } while (false)
 
 #define POP_AND_STORE(pop_type, store_type)                                                               \
@@ -329,10 +335,12 @@ MakeUnsigned<T> BytecodeInterpreter::checked_unsigned_truncate(V value)
 Vector<Value> BytecodeInterpreter::pop_values(Configuration& configuration, size_t count)
 {
     Vector<Value> results;
+    results.resize(count);
+
     for (size_t i = 0; i < count; ++i) {
         auto top_of_stack = configuration.stack().pop();
         if (auto value = top_of_stack.get_pointer<Value>())
-            results.prepend(move(*value));
+            results[i] = move(*value);
         else
             TRAP_IF_NOT_NORETURN(value);
     }
@@ -411,17 +419,12 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi
     case Instructions::structured_else.value(): {
         auto label = configuration.nth_label(0);
         TRAP_IF_NOT(label.has_value());
-        auto results = pop_values(configuration, label->arity());
+        size_t end = configuration.stack().size() - label->arity() - 1;
+        size_t start = end;
+        while (start > 0 && !configuration.stack().entries()[start].has<Label>())
+            --start;
 
-        // drop all locals
-        for (; !configuration.stack().is_empty();) {
-            auto entry = configuration.stack().pop();
-            if (entry.has<Label>())
-                break;
-        }
-
-        for (auto& result : results)
-            configuration.stack().push(move(result));
+        configuration.stack().entries().remove(start, end - start + 1);
 
         if (instruction.opcode() == Instructions::structured_end)
             return;
@@ -431,35 +434,19 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi
         return;
     }
     case Instructions::return_.value(): {
-        Vector<Stack::EntryType> results;
         auto& frame = configuration.frame();
-        results.ensure_capacity(frame.arity());
-        for (size_t i = 0; i < frame.arity(); ++i)
-            results.prepend(configuration.stack().pop());
-        // drop all locals
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
-        Optional<Label> last_label;
-#pragma GCC diagnostic pop
-        for (; !configuration.stack().is_empty();) {
-            auto entry = configuration.stack().pop();
-            if (entry.has<Label>()) {
-                last_label = entry.get<Label>();
-                continue;
-            }
+        size_t end = configuration.stack().size() - frame.arity();
+        size_t start = end;
+        for (; start + 1 > 0; --start) {
+            auto& entry = configuration.stack().entries()[start];
             if (entry.has<Frame>()) {
-                // Push the frame back
-                configuration.stack().push(move(entry));
-                // Push its label back (if there is one)
-                if (last_label.has_value())
-                    configuration.stack().push(last_label.release_value());
+                // Leave the frame, _and_ its label.
+                start += 2;
                 break;
             }
-            last_label.clear();
         }
-        // Push the results back
-        for (auto& result : results)
-            configuration.stack().push(move(result));
+
+        configuration.stack().entries().remove(start, end - start);
 
         // Jump past the call/indirect instruction
         configuration.ip() = configuration.frame().expression().instructions().size() - 1;
@@ -588,13 +575,13 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi
         auto address = configuration.frame().module().memories()[0];
         auto instance = configuration.store().get(address);
         i32 old_pages = instance->size() / Constants::page_size;
-        auto new_pages = configuration.stack().pop().get<Value>().to<i32>();
+        auto new_pages = configuration.stack().peek().get<Value>().to<i32>();
         TRAP_IF_NOT(new_pages.has_value());
         dbgln_if(WASM_TRACE_DEBUG, "memory.grow({}), previously {} pages...", *new_pages, old_pages);
         if (instance->grow(new_pages.value() * Constants::page_size))
-            configuration.stack().push(Value((i32)old_pages));
+            configuration.stack().peek() = Value((i32)old_pages);
         else
-            configuration.stack().push(Value((i32)-1));
+            configuration.stack().peek() = Value((i32)-1);
         return;
     }
     case Instructions::table_get.value():
@@ -613,8 +600,8 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi
         TRAP_IF_NOT(value.has_value());
         dbgln_if(WASM_TRACE_DEBUG, "select({})", value.value());
         auto rhs = move(configuration.stack().pop().get<Value>());
-        auto lhs = move(configuration.stack().pop().get<Value>());
-        configuration.stack().push(value.value() != 0 ? move(lhs) : move(rhs));
+        auto lhs = move(configuration.stack().peek().get<Value>());
+        configuration.stack().peek() = value.value() != 0 ? move(lhs) : move(rhs);
         return;
     }
     case Instructions::i32_eqz.value():

Userland/Libraries/LibWasm/AbstractMachine/Interpreter.h

@@ -39,7 +39,8 @@ struct BytecodeInterpreter : public Interpreter {
 protected:
     virtual void interpret(Configuration&, InstructionPointer&, const Instruction&);
     void branch_to_label(Configuration&, LabelIndex);
-    ReadonlyBytes load_from_memory(Configuration&, const Instruction&, size_t);
+    template<typename ReadT, typename PushT>
+    void load_and_push(Configuration&, const Instruction&);
     void store_to_memory(Configuration&, const Instruction&, ReadonlyBytes data);
     void call_address(Configuration&, FunctionAddress);