Skip to content

Define text format #14

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

Open
kripken opened this issue May 13, 2025 · 6 comments
Open

Define text format #14

kripken opened this issue May 13, 2025 · 6 comments

Comments

@kripken
Copy link
Member

kripken commented May 13, 2025

In discussion it was clarified that the custom annotations spec says nothing about how future proposals use annotations in the text format. We may define non-spec conventions, but I think that means that this proposal needs to define how the hints appear in the text format.

@ecmziegler
Copy link
Collaborator

ecmziegler commented May 14, 2025

Interesting idea! I didn't even consider that we would have that option. But it would certainly make those annotations much more readable than the cryptic string encoded binary data. I could imagine something like this:

@metadata.code.call_targets(
  (call_target $target1 89)
  (call_target $target2 10)
)
(call.ref (ref $target_ref) ...)

I'm not sure if the call_target can be defined in a context-specific manner or if we would need a namespace prefix in front of it.

It would also allow for better readability of special values

@metadata.code.instruction_frequency((instruction_frequency.high))
@metadata.code.instruction_frequency((instruction_frequency.low))
@metadata.code.instruction_frequency((instruction_frequency.value 100))

@yuri91
Copy link

yuri91 commented May 14, 2025

@ecmziegler while it can be a good idea to define a more "structured" syntax for the text format, it should have a different identifier than @metadata.code.<x>, and exist in parallel to it.

The intention of the metadata.code prefix on annotations is to have a way to map binary and text format for all custom sections following the code metadata format, so that tools can round trip them even with limited knowledge about them.

even for branch hinting, the idea was to potentially add something like (@branch_hint likely) br_if in the future, if there is interest for it.

But this means adding extra logic for the parsing specifically for it in the tools, while the metadata.code syntax is simpler and general.

@ecmziegler
Copy link
Collaborator

@yuri91 I would see this as an alternative way to describe it. The binary syntax is still allowed. But of course, it means that tools which don't understand this format, cannot parse it. On the other hand, tools that don't understand the alternative identifier also wouldn't be able to parse it, so nothing is won by it using a different one.

Round trips also just work for simple cases. If e.g. a tool decides to reorder function indices, a lot of the annotations will be garbled anyway. Realistically, a tool that doesn't understand an annotation is likely better off dropping than keeping it.

@yuri91
Copy link

yuri91 commented May 14, 2025

as long as there is no ambiguity, sure, the same identifier could be used with different syntactical variants.

And about the round-tripping: absolutely, it has limited uses without any knowledge of the semantics of the section, but I would argue that being able to convert to text even just for reading and/or doing a minor tweak to a module is valuable.

It was especially crucial for adding some form of testing in the official test suite.

@ecmziegler
Copy link
Collaborator

Yes, conversion to/from text should be possible in a straightforward way. The way I see it, we could make

@metadata.code.call_targets(
  (call_target $target1 89)
  (call_target $target2 10)
)

and

@metadata.code.call_targets("\3a\59\3b\0a")

mean the same thing.

@kripken
Copy link
Member Author

kripken commented May 15, 2025

One thing we should update in this proposal is spec text for the syntax of function annotations.

@yuri91 mentions in the previous discussion that branch hinting added syntax for instructions, and this is the first proposal that adds functions. I think all we need is one or two lines of text that say something similar for functions as for instructions, namely, ".. function annotations are considered attached to the first function that follows them .."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants