-
Notifications
You must be signed in to change notification settings - Fork 12
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
Nuke the rest of middleware code #126
Conversation
In an effort to make the code a little more straightforward I once again am removing code. This time it is remaining cleanup from the middleware removal. This ended up also requiring me to take a second look at how we parse the module data during codegen, since previously that code was wrapped by the middleware code.
for _ in 0..local_count { | ||
builder.set_srcloc(cur_srcloc(reader)); | ||
let (count, ty) = reader.read_local_decl()?; | ||
builder.set_srcloc(ir::SourceLoc::new(local_reader.original_position() as u32)); |
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.
TODO: This is actually incorrect as it will assign all locals to the byte offset at which the locals reader started reading.
On the other hand… I'm not sure if we emit debug info in any meaningful capacity right now and this is preexisting anyway.
Makes sense to me! My overall feelings here are:
|
let _tt = timing::wasm_translate_function(); | ||
info!( | ||
"translate({} bytes, {}{})", | ||
reader.bytes_remaining(), | ||
func.name, | ||
func.signature | ||
); | ||
let _span = tracing::info_span!( |
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.
I wonder if we should rip out the timing
module and just use spans? Still not sure about general applicability of tracing for profiling -- rust-analyzer still uses hand-rolled infra.
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.
The timing module is cranelift
's, not part of this codebase. Will definitely be punting on either decision for the time being. Unlike R-A this project is intended to be embedded, so making it use something off-shelf has significant benefits.
In an effort to make the code a little more straightforward1 I once again am
removing code. This time it is remaining cleanup from the middleware removal.
This ended up also requiring me to take a second look at how we parse the module
data during codegen, since previously that code was wrapped by the middleware
code.
Footnotes
…to modify… ↩