Wasmtime GitHub notifications bot (Jul 15 2025 at 13:43): Wasmtime GitHub notifications bot (Jul 15 2025 at 13:43):masnagam opened PR #11241 from masnagam:feat-cranelift-codegen-block_name to bytecodealliance:main:
This PR implements a feature mentioned in #10766 and allows developers to set meaningful names to blocks.
Block names are used only in Cranelift textual IR representation.
print!("{block}")does not show the block name.The implementaion is naive and allocates a
Stringfor each block name.
So, it's recommend to use block names only for debugging purposes at this point.
Someone may implement more memory efficient mechanism for block names (such asTwinein LLVM) in the future.
Wasmtime GitHub notifications bot (Jul 15 2025 at 13:43):masnagam requested wasmtime-compiler-reviewers for a review on PR #11241.
Wasmtime GitHub notifications bot (Jul 15 2025 at 13:43):masnagam requested cfallin for a review on PR #11241.
Wasmtime GitHub notifications bot (Jul 15 2025 at 13:59):bjorn3 created PR review comment:
Putting this into a separate
SecondaryMap<Block, String>would reduce memory usage when the block name functionality isn't used and remove unnecessary padding and improve cpu cache efficiency when it is used.
Wasmtime GitHub notifications bot (Jul 15 2025 at 13:59):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Jul 15 2025 at 13:59):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Jul 15 2025 at 13:59):bjorn3 created PR review comment:
Also you can represent no name being given as an empty string.
Wasmtime GitHub notifications bot (Jul 15 2025 at 14:00):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Jul 15 2025 at 14:00):bjorn3 created PR review comment:
You need to use backticks both before and after.
Wasmtime GitHub notifications bot (Jul 15 2025 at 14:01):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Jul 15 2025 at 14:01):bjorn3 created PR review comment:
"It is" reads better to me.
Wasmtime GitHub notifications bot (Jul 15 2025 at 14:03):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Jul 15 2025 at 14:03):bjorn3 created PR review comment:
The block names should probably also be printed in for example
jump block1. Also would be nice to support them in the parser too.
Wasmtime GitHub notifications bot (Jul 15 2025 at 17:10):cfallin commented on PR #11241:
Thanks @masnagam for this PR!
Following on from @bjorn3's helpful comments, I think the main high-level thing that we would need to address to merge this is the round-tripping aspect of textual CLIF. Right now, this PR prints CLIF that can't be re-parsed (in addition to not using the user-provided names everywhere). I think we need to think about entity names from a syntax-design perspective first, come up with a consistent answer that represents all the needed information and can be round-tripped, then go from there.
A few considerations: entity numbering is deeply embedded in the way that the parser and the in-memory representation both work. It is what lets us have circular references without a separate name-resolution pass; it is what lets us avoid a symbol table. We'll want to re-think this: if entities can have names, then we need a symtab, we need a way to temporarily represent names during parsing and then resolve the references into numeric entity indices (allocating new indices for each name, and avoiding indices already used by the user), and we need to think about whether we want to print names and entity indices in the textual output. (Probably? Otherwise a roundtrip might not preserve them.)
We can sidestep a bunch of this if names are more like annotations and the entity indices are still the primary syntactic elements. For example, we could print comments alongside block definitions and references that give the names for those blocks. (As an aside, this is how I handle block names in waffle, another compiler library I designed that uses a Cranelift-like IR.) I'd personally be in favor of this -- it requires a little more care when writing textual CLIF (one still needs to come up with entity indices) but it keeps the language simpler and still allows for clarity when reading.
If we go that route, I think the needed changes are:
- Don't replace the entity name, but rather, print the string name as a comment at the end of the line
- Print a comment with a block name whenever a block is referenced in a terminator, jump table (used in
br_tableinstructions), or exception table (used intry_call/try_call_indirectinstructions)as well as all the other review comments above. What do you think?
Wasmtime GitHub notifications bot (Jul 16 2025 at 03:26):masnagam commented on PR #11241:
@bjorn3 @cfallin
Thank you for your comments.
At this point, there are many issues in my PR that I should have to consider before sending the PR.
I'd like to close this PR after discussion w/ you and restart investigating Cranelift implementation in depth and reconsidering this feature based on your comments.
I'll create a new PR if I succeeded to implement this feature.
(Or ask someone to check my software design before implementing it).Currently, I'm trying implementing a JavaScript engine using Cranelift JIT.
And I want to assign meaningful block names (and variable names) for debugging purposes.
This is my motivation to want to implement this feature.I need time to understand what you wrote because I don't have enough knowledge about Cranelift implementation at this point (as you may have noticed).
I'll come back here once I've finished investigating relevent code.
Wasmtime GitHub notifications bot (Jul 16 2025 at 03:33):masnagam created PR review comment:
oh, right. i completely forgot that...
Wasmtime GitHub notifications bot (Jul 16 2025 at 03:33):masnagam submitted PR review.
Wasmtime GitHub notifications bot (Jul 16 2025 at 06:04):masnagam closed without merge PR #11241.
Wasmtime GitHub notifications bot (Jul 16 2025 at 06:04):masnagam commented on PR #11241:
I spent a few hours for investigating the Cranelift source tree and eventually understood what you wrote in the previous comment.
And I strongly feel the the direction of my change was not good.
Especially, I don't like to modify the lexer for clif files.
Probably, we can handle any block names in clif files by introducing a table for block names.
But the change increases complexity and regresses simplicity and maintainability (and throughputs slightly).I chekced waffle and got some hints from it.
Currently, I'm thinking of introducing a supplemental separate file to my JavaScript implementation, which contains a map between block entity IDs (block*) and the corresponding block names.
We can easily convert block entity IDs in Craneleft IR textual representation (and its CFG in the DOT format) to corresponding block names by using the map.
That's enough for debugging purposes.
Probably a similar map can be introduced for variable names.Thank you for your kindly support :)
I close this PR.
Last updated: Dec 06 2025 at 07:03 UTC