Implement Tail Call Optimize into ReturnCall Instruction by makachanm · Pull Request #400 · Samsung/walrus

makachanm · 2026-04-08T06:23:56Z

I implemented a tail call optimize into WebAssembly 3's ReturnCall recursive instruction.
It currently unfinished, (the issue with type checking in the standard's spec, indirect call is not completed..) but it passes Return Call Test code.

clover2123 · 2026-04-08T10:05:59Z

Please run tools/check_tidy.py --update to format the code according to the style guidelines.

makachanm · 2026-04-09T04:10:04Z

I applied. thanks.

zherczeg

This sounds like a good idea. I would simplify the parameter storing: the parameters are simply stored in a linear buffer, and copied by a memcpy.

zherczeg · 2026-04-09T04:43:35Z

+    }
+
+    std::vector<size_t> tco_paramStore;
+    size_t tco_paramSize;


Vector should have a size.

I recommend using the predefined Vector structure as shown below.
Also, regarding naming conventions, the Walrus project typically uses member variables that start with m_ and avoids underscores in the middle of names.

Vector<size_t> m_tcoParamStore;

If you are using a fixed-length vector, please use VectorWithFixedSize instead:

VectorWithFixedSize<size_t> m_tcoParamStore; m_tcoParamStore.reserve(newSize); // allocate array m_tcoParamStore.clear(); // reset array

zherczeg · 2026-04-09T04:48:17Z

+        return reinterpret_cast<ByteCodeStackOffset*>(reinterpret_cast<size_t>(this) + sizeof(Call));
+    }
+
+    uint16_t parameterOffsetsSize() const


Correct me if I wrong, but as far as I remember parameters are always stored in a linear memory area. This means their offsets do not need to be stored, they can be computed during running. The size of the tco buffer could be stored so the buffer could be pre-allocated.

zherczeg · 2026-04-09T04:52:42Z

Parameters are stored in a linear area:
https://github.com/Samsung/walrus/blob/main/src/interpreter/Interpreter.h#L57

Then the tco buffer could be copied here without changes.

makachanm · 2026-04-09T05:17:51Z

I'll rework parameter recover code with memcpy and Walrus's own vector implementation. Thanks for the feedback.

makachanm · 2026-04-10T05:25:17Z

I think lack of JIT version of TCO implement causes CI failure. I'll seperate it for future works.

zherczeg · 2026-04-10T06:32:35Z

+    void destroyTCO()
+    {
+        m_tcoparamStore.clear();
+        m_tcofunctionTarget = nullptr;


The names are wrong. They should be m_tcoParamStore and m_tcoFunction store.

makachanm · 2026-04-13T05:03:54Z

I've verified that all tests in the walrus suite are passing in the x64 local machine at interpret mode.

clover2123 · 2026-04-13T05:41:54Z

There is a conflict in ByteCode.h file.
Please rebase it and push this pr again

…nCallRef instruction

makachanm · 2026-04-14T01:59:53Z

CI is failing when Clang compiler is used. I had no problem with GCC.
I'll fix it with reordering some parameter initialize order.

clover2123 · 2026-04-15T07:13:09Z

+        Table* table = instance->table(code->tableIndex());
+
+        uint32_t idx = readValue<uint32_t>(bp, code->calleeOffset());
+        if (idx >= table->size()) {


Add UNLIKELY to this check code

clover2123 · 2026-04-15T07:26:02Z

+            Trap::throwException(state, "uninitialized element " + std::to_string(idx));
+        }
+        const FunctionType* ft = target->functionType();
+        if (!ft->equals(code->functionType())) {


UNLIKELY here too

clover2123 · 2026-04-15T07:26:37Z

+            Trap::throwException(state, "null function reference");
+        }
+        const FunctionType* ft = target->functionType();
+        if (!ft->equals(code->functionType())) {


clover2123 · 2026-04-15T08:57:02Z

+        if (newState.hasTCO()) {
+            state.m_tcoParamStore.reserve(newState.m_tcoParamStore.size());
+            VectorCopier<size_t>::copy(state.m_tcoParamStore.data(), newState.m_tcoParamStore.data(), newState.m_tcoParamStore.size());
+            state.m_tcoFunctionTarget = newState.m_tcoFunctionTarget;
+            return;
+        }


At this point, the vector needs to be copied into the upper ExecutionState.
Instead of this copy, what about allocating one global vector and reusing this vector?
This approach could reduce copy overhead.

I don't have a idea for where I declare a global TCO buffer. I think addition of new execution state manager will be affect the code's consistency. and It can be harder to decide when the timing the destroy a TCO buffer.

I think that Store would be a good place to keep the TCO buffer.
Store can be accessed through instance->module()->store() in the interpreter.

clover2123 · 2026-04-15T08:59:38Z

                                 uint8_t* bp,
                                 Instance* instance);

+    static void returnCallOperation(ExecutionState& state,


It seems that this function is declared but never used

clover2123 · 2026-04-15T09:03:53Z

+        if (state.hasTCO()) {
+            VectorCopier<size_t>::copy((size_t*)functionStackBase, state.m_tcoParamStore.data(), state.m_tcoParamStore.size());
+            state.destroyTCO();


When interpreter is called again due to tail call,
state.destroyTCO(); has been called so state doesn't have any valid TCO data at this point.
Would you check it again?

I'll check it later.

clover2123 · 2026-04-15T09:04:43Z

Lastly, have you been able to test this patch for recursive tail calls?

makachanm · 2026-04-15T09:39:45Z

Lastly, have you been able to test this patch for recursive tail calls?

I'll do it through recursive fibonacci test. Thanks for the mention.

makachanm · 2026-04-16T04:53:19Z

I found fibonacci test makes call stack exhaustion. I'm analyzing what's problem with.

makachanm requested review from clover2123 and ksh8281 as code owners April 8, 2026 06:23

zherczeg reviewed Apr 9, 2026

View reviewed changes

zherczeg reviewed Apr 10, 2026

View reviewed changes

makachanm force-pushed the impl_tco branch 3 times, most recently from 67bc7f8 to daa44c1 Compare April 13, 2026 09:30

makachanm marked this pull request as draft April 13, 2026 09:30

Implement Tail Call Optimize into ReturnCall ReturnCallIndirect Retur…

e8f4d8e

…nCallRef instruction

makachanm force-pushed the impl_tco branch from daa44c1 to e8f4d8e Compare April 13, 2026 10:13

makachanm marked this pull request as ready for review April 13, 2026 10:13

Reorder parameter order

318612b

clover2123 reviewed Apr 15, 2026

View reviewed changes

Conversation

makachanm commented Apr 8, 2026

Uh oh!

clover2123 commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

makachanm commented Apr 9, 2026

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zherczeg commented Apr 9, 2026

Uh oh!

makachanm commented Apr 9, 2026

Uh oh!

makachanm commented Apr 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

makachanm commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clover2123 commented Apr 13, 2026

Uh oh!

makachanm commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clover2123 commented Apr 15, 2026

Uh oh!

makachanm commented Apr 15, 2026

Uh oh!

makachanm commented Apr 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

clover2123 commented Apr 8, 2026 •

edited

Loading

makachanm commented Apr 13, 2026 •

edited

Loading

makachanm commented Apr 14, 2026 •

edited

Loading