wasm-micro-runtime
wasm-micro-runtime copied to clipboard
aot/jit: tail-call implementation looks wrong
return_call_indirectdoesn't seem to use a tail call at all.return_calluses llvmtailattribute, which is merely a hint. the recent versions of llvm has themusttailattirubute, which can be used instead.
Note: llvm c api doesn't seem to have an api to use musttail yet.
return_call_indirectdoesn't seem to use a tail call at all.return_calluses llvmtailattribute, which is merely a hint. the recent versions of llvm has themusttailattirubute, which can be used instead.Note: llvm c api doesn't seem to have an api to use
musttailyet.
Thanks for the reminding, we implemented tail-call long time ago, I remembered we tested the spec cases of that proposal for both interpreter and AOT. And yes, we should test again and try to add the testing to the CI.
For return_call_indirect, maybe it is enough to call_indirect first and then return the result:
https://github.com/bytecodealliance/wasm-micro-runtime/blob/main/core/iwasm/compilation/aot_compiler.c#L413-L416
For the musttail attribute, we may set it with llvm c++ API, we set the function attribute disable-tail-call when needed:
https://github.com/bytecodealliance/wasm-micro-runtime/blob/main/core/iwasm/compilation/aot_llvm_extra.cpp#L319
return_call_indirectdoesn't seem to use a tail call at all.return_calluses llvmtailattribute, which is merely a hint. the recent versions of llvm has themusttailattirubute, which can be used instead.Note: llvm c api doesn't seem to have an api to use
musttailyet.Thanks for the reminding, we implemented tail-call long time ago, I remembered we tested the spec cases of that proposal for both interpreter and AOT. And yes, we should test again and try to add the testing to the CI.
FYI, recently llvm fixed a codegen bug in wasm tail-call. that would make tail-call a bit more usable. (thus more chances to expose implementation issues i guess.)
For
return_call_indirect, maybe it is enough tocall_indirectfirst and thenreturnthe result: https://github.com/bytecodealliance/wasm-micro-runtime/blob/main/core/iwasm/compilation/aot_compiler.c#L413-L416
it isn't enough because it can consume unbounded amount of stack.
For the
musttailattribute, we may set it with llvm c++ API, we set the function attributedisable-tail-callwhen needed: https://github.com/bytecodealliance/wasm-micro-runtime/blob/main/core/iwasm/compilation/aot_llvm_extra.cpp#L319
sure.
for some archs (eg xtensa) native tail-call is not available. for such platforms, we might need to invent a special calling convention.
musttail can improve the situation a bit because it bails out earlier. (on aot-compilation time.)
besides the aot-generated code, our "call native" implementations (eg. aot_call_indirect .. invokeNative) need to perform tail call as well, i guess.
stack overflow check wrappers need to perform tail-call as well. (right now it only sometimes does.)
for platforms w/o native tailcall, i don't have any good idea to implement it yet. given the number of platforms where tail call doesn't always work, i'm a bit pessimistic: https://github.com/bytecodealliance/wasm-micro-runtime/blob/0d8ffebd3952a52ca1f2a7ee163549202c3890ed/core/iwasm/compilation/aot_llvm.c#L155-L198