Skip to content

Text format #16

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
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Text format #16

wants to merge 4 commits into from

Conversation

ecmziegler
Copy link
Collaborator

Alternatively to the binary representation as a string value, one can use a human readable syntax to express compilation hints. These hints will have the same binary representation int eh module and can therefore also be translated into the string format if conversion tools don't support the alternative format.

This implements the suggestion from issue #14.

@ecmziegler ecmziegler requested review from kripken and tlively June 12, 2025 10:33
Comment on lines 56 to 59
@metadata.code.compilation_order(
(priority 1)
(hotness 100)
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the current branch hints have this form:

(@metadata.code.branch_hint "\01")
(..instruction attached to..)

Perhaps we can follow that? That is,

Suggested change
@metadata.code.compilation_order(
(priority 1)
(hotness 100)
)
(@metadata.code.compilation_order
(priority 1)
(hotness 100)
)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, of course. I misremembered the syntax.

@@ -37,7 +37,7 @@ The following contains a list of hints to be included in the first version of th
The section `metadata.code.compilation_order` contains the order in which functions should be compiled in order to minimize wait times until the compilation is completed. This is especially relevant during instantiation and startup but might also be relevant later.
* *byte offset* |U32| with value 0 (function level only)
* *hint length* |U32| in bytes
* *compilation order* |U32| starting at 0 (functions with the same order value will be compiled in an arbitrary order but before functions with a higher order value)
* *compilation priority* |U32| starting at 0 (functions with the same order value will be compiled in an arbitrary order but before functions with a higher order value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* *compilation priority* |U32| starting at 0 (functions with the same order value will be compiled in an arbitrary order but before functions with a higher order value)
* *compilation priority* |U32| starting at 0 (functions with the same priority value will be compiled in an arbitrary order but before functions with a higher priority value)

Should the hint itself be renamed to metadata.code.compilation_priority?

```
according to the formula above.

Alternatively to `freq` followed by a numeric value, one can provide `never_opt` and `always_opt` with binary representations of `"\00"` and `"\7f"` respectively.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to bikeshed a bit, what do you think about never and always instead of never_opt and always_opt?


The text representation allows for multiple targets
```
(@metadata.code.call_targets (target $func1 0.73) (target $func2 0.21))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be good to give a grammar here, something like

calltargetannot ::= ... | '(' '@metadata.code.call_targets' <target>* ')'
target ::= '(' 'target' funcidx freq ')'
freq ::= f32

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

Successfully merging this pull request may close these issues.

3 participants