Stream: git-wasmtime

Topic: wasmtime / Issue #1214 cranelift-module: expose trap info...

Wasmtime GitHub notifications bot (Mar 05 2020 at 09:11):

philipc commented on Issue #1214:

My intent in https://github.com/bytecodealliance/wasmtime/issues/1184#issuecomment-590618409 was that the backends would no longer store traps at all. e.g. the caller would pass a binemit::TrapSink to Module::define_function (or a new variant of that), and Module::define_function_bytes would no longer need to know about traps. But it's not something that I strongly feel needs to change if you think it's better to keep storing them in the backend.

Wasmtime GitHub notifications bot (Mar 05 2020 at 15:31):

froydnj commented on Issue #1214:

My intent in #1184 (comment) was that the backends would no longer store traps at all. e.g. the caller would pass a binemit::TrapSink to Module::define_function (or a new variant of that), and Module::define_function_bytes would no longer need to know about traps. But it's not something that I strongly feel needs to change if you think it's better to keep storing them in the backend.

Ah, I didn't fully grok what you were saying there; thank you for the additional clarification!

I think passing in binemit::TrapSink would be slightly cleaner, but that implies that traps are somehow more important than relocs or stackmaps, and I'm not clear on why that'd be true. I guess you can argue that the backend should be responsible for applying relocations on its own, and nobody seems to (currently) do anything with stackmaps anyway (at least in the context of cranelift-module)?

In the absence of strong feelings, I am inclined to leave the patch as-is.

Wasmtime GitHub notifications bot (Mar 05 2020 at 17:08):

pchickey commented on Issue #1214:

I agree that relocations are something we just give the backend.

We'll have to do something with stackmaps in cranelift-module eventually, the lack of pressure is because they are used for GC, which only wasmtime is going to implement, and wasmtime isnt using cranelift-module yet. There's a reasonable argument that whatever design we pick for traps, we should use for stack maps as well, for the sake of consistency. Passing in a sink for traps and stackmaps gives the consumer more control than returning a vector, so I'm in favor of that design, if we're going to revise.

I'm also ok with landing this as-is and revising it later, speaking as a downstream. Its not a huge difference to us either way, so I'll defer to bjorn and philipc.

Wasmtime GitHub notifications bot (Mar 05 2020 at 20:56):

philipc commented on Issue #1214:

Yeah this is ok as-is.

Wasmtime GitHub notifications bot (Mar 05 2020 at 22:17):

pchickey commented on Issue #1214:

Lets get @tschneidereit to add you as a collab so you can merge these when they're ready to go

Wasmtime GitHub notifications bot (Mar 09 2020 at 11:10):

tschneidereit commented on Issue #1214:

Lets get @tschneidereit to add you as a collab so you can merge these when they're ready to go

Agreed!

Last updated: Dec 23 2024 at 12:05 UTC