-
Notifications
You must be signed in to change notification settings - Fork 650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Emit load_addr and load_size if WAMR_ENABLE_COMPILER is set #3835
Conversation
1a20b49
to
8739d35
Compare
Currently, the open-source builds set WASM_ENABLE_DUMP_CALL_STACK, which causes these two fields to be emitted. They are required by aot_emit_exception.cc. Internally at Google, we don't enable call stack dumps, so we've been using the attached patch to make sure the fields are emitted anyway.
8739d35
to
e9da33c
Compare
@sjamesr could you tell a bit about your usecase? WAMR compiler usually is used as a developer tools and rarely in production, so size micro optimizations like this one is usually not a problem. You can still enable dump callstacks feature for the runtime itself though. So my recommended approach would be to:
|
Currently we build the compiler as you suggest, without WASM_ENABLE_DUMP_CALL_STACK defined. The change I sent is required to get the compiler to build in this case, and we apply this patch locally before building. Upstream, the compiler is built with the option set. If you apply the following patch to disable it, I think you'll find wamrc doesn't build. I was thinking about enabling WASM_ENABLE_DUMP_CALL_STACK for the compiler, because that's what is done upstream, but it's good to know that leaving it off should be OK.
|
Got it. Yeah personally I don't see why the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Currently, the open-source builds set WASM_ENABLE_DUMP_CALL_STACK, which causes these two fields to be emitted. They are required by aot_emit_exception.cc.
Internally at Google, we don't enable call stack dumps, so we've been using the attached patch to make sure the fields are emitted anyway.
Does it make sense to build the compiler without ENABLE_DUMP_CALL_STACK? This could just be an oversight on our part when we import WAMR.