-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add function names to --emit-clif output #11040
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
Add function names to --emit-clif output #11040
Conversation
crates/cranelift/src/compiler.rs
Outdated
";;Intermediate Representation of function <{}>:\n", | ||
symbol.split("::").last().unwrap() |
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.
This would work too, right?
";;Intermediate Representation of function <{}>:\n", | |
symbol.split("::").last().unwrap() | |
";; Intermediate Representation of function <{symbol}>:\n", |
This avoids truncating a function name like wasm[0]::function[1]::my_library::SomeType::get
(as would be used if the names section contains my_library::SomeType::get
as name) to the pretty much useless get
.
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.
would using something like this function work better?
fn remove_first_two_segments(s: &str) -> Option<String> {
let mut parts = s.split("::").skip(2);
let rest: Vec<&str> = parts.collect();
if rest.is_empty() {
None
} else {
Some(format!("::{}", rest.join("::")))
}
}
also wasmtime explore uses "Intermediate Representation of function :" too should i figure out a way where it gives my_library::SomeType::get too?
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.
Nothing to add over what @bjorn3 pointed out.
Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com>
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.
Thanks!
Related Issues
Fixes: #10733
Explanation
This PR addresses issue #10733 by adding the function name as a comment to the beginning of each Cranelift IR file generated by the
--emit-clif
flag.Previously, the files generated by
--emit-clif
contained only the raw Cranelift IR, making it difficult to identify which function the IR belonged to without cross-referencing with other tools likewasmtime explore
. This change improves the usability of the--emit-clif
output by clearly labeling each function's IR.Input Needed
I'm unsure of where to place tests for this.