-
Notifications
You must be signed in to change notification settings - Fork 20
feat: tolk asm-functions #1510
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
base: main
Are you sure you want to change the base?
feat: tolk asm-functions #1510
Conversation
This comment has been minimized.
This comment has been minimized.
c2e9f7a to
af6faf8
Compare
This comment has been minimized.
This comment has been minimized.
|
Thank you for the improvements in |
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
|
/review |
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 for expanding the Tolk assembler docs in languages/tolk/features/asm-functions.mdx: I’ve left several suggestions around wording and example formatting—please apply the inline suggestions.
| fun hashStateInit(code: cell, data: cell): uint256 asm """ | ||
| DUP2 | ||
| HASHCU | ||
| ... | ||
| ONE HASHEXT_SHA256 | ||
| """ | ||
| ``` | ||
|
|
||
| It is treated as a single string and inserted as-is into Fift output. | ||
| In particular, it may contain `//` comments inside (valid comments for Fift). | ||
|
|
||
| ## Stack order for multiple slots | ||
|
|
||
| When calling a function, arguments are pushed in a declared order. | ||
| The last parameter becomes the topmost stack element. | ||
|
|
||
| If an instruction results in several slots, the resulting type should be a tensor or a struct. | ||
|
|
||
| For example, write a function `abs2` that calculates `abs()` for two values at once: `abs2(-5, -10)` = `(5, 10)`. | ||
| Stack layout (the right is the top) is written in comments. | ||
|
|
||
| ```tolk | ||
| fun abs2(v1: int, v2: int): (int, int) | ||
| asm // v1 v2 | ||
| "ABS" // v1 v2_abs | ||
| "SWAP" // v2_abs v1 | ||
| "ABS" // v2_abs v1_abs | ||
| "SWAP" // v1_abs v2_abs |
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.
[HIGH] Ellipsis token and trailing inline comments in code examples
In the hashStateInit example, the line containing a bare ... inside the asm """ block is not valid assembler code and violates the rule against ellipses that change syntax, making the example non–copy‑pasteable. In the abs2 example, the stack layout descriptions are written as trailing // comments on the same lines as code, which the style guide forbids for code samples. Both issues are classified as HIGH severity because they reduce clarity and can cause confusion or errors when readers reuse the examples.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
| For better understanding, let's look at regular functions first. | ||
| The compiler does all transformations automatically: | ||
|
|
||
| ```tolk | ||
| // transformed to: "returns (int, void)" | ||
| fun increment(mutate x: int): void { | ||
| x += 1; | ||
| // a hidden "return x" is inserted | ||
| } | ||
| fun demo() { | ||
| // transformed to: (newX, _) = increment(x); x = newX | ||
| increment(mutate x); | ||
| } | ||
| ``` | ||
|
|
||
| How to implement `increment()` via asm? | ||
|
|
||
| ```tolk | ||
| fun increment(mutate x: int): void | ||
| asm "INC" | ||
| ``` | ||
|
|
||
| The function still returns `void` (from the type system's perspective it does not return a value), | ||
| but `INC` leaves a number on the stack — that's a hidden "return x" from a manual variant. | ||
|
|
||
| Similarly, it works for `mutate self`. | ||
| An `asm` function should place `newSelf` onto the stack before the actual result: | ||
|
|
||
| ```tolk | ||
| // "TPUSH" pops (tuple) and pushes (newTuple); | ||
| // so, newSelf = newTuple, and return `void` (syn. "unit") | ||
| fun tuple.push<X>(mutate self, value: X): void | ||
| asm "TPUSH" | ||
| // "LDU" pops (slice) and pushes (int, newSlice); | ||
| // with `asm(-> 1 0)`, we make it (newSlice, int); | ||
| // so, newSelf = newSlice, and return `int` | ||
| fun slice.loadMessageFlags(mutate self): int | ||
| asm(-> 1 0) "4 LDU" | ||
| ``` | ||
|
|
||
| To return `self` for chaining, just specify a return type: | ||
|
|
||
| ```tolk | ||
| // "STU" pops (int, builder) and pushes (newBuilder); | ||
| // with `asm(op self)`, we put arguments to correct order; |
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.
[HIGH] First-person pronouns in prose and comments
The explanatory prose uses “let's” (“For better understanding, let's look at regular functions first.”) and code comments use “we” (“we make it (newSlice, int)”, “we put arguments to correct order”) to refer to authors and the reader. The style guide’s “Don't get personal” rule explicitly forbids first‑person plural pronouns for authors and inclusive phrasing that addresses the reader, marking such cases as HIGH severity. This wording shifts the tone from objective reference documentation to conversational narration, which the project aims to avoid.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
This comment was marked as outdated.
This comment was marked as outdated.
|
/review |
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 for the thorough asm docs in languages/tolk/features/asm-functions.mdx: I’ve added several suggestions to keep examples copy-pasteable and tone consistent with the style guide, so please apply the inline suggestions.
| fun hashStateInit(code: cell, data: cell): uint256 asm """ | ||
| DUP2 | ||
| HASHCU | ||
| ... |
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.
[HIGH] Literal ellipsis used as code inside snippet
The hashStateInit example includes a bare ... line inside the multi-line asm string, which will be emitted as literal assembler code rather than serving as a harmless placeholder. This makes the example non-runnable as shown and encourages readers to copy an invalid pattern into real code. The extended style guide requires using language-appropriate comments to mark omissions and forbids literal ellipses that alter syntax in copy‑pasteable examples (see https://github.com/ton-org/docs/blob/main/contribute/style-guide-extended.mdx?plain=1#L552-L555).
| ... | |
| // ... |
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
| fun abs2(v1: int, v2: int): (int, int) | ||
| asm // v1 v2 | ||
| "ABS" // v1 v2_abs | ||
| "SWAP" // v2_abs v1 | ||
| "ABS" // v2_abs v1_abs | ||
| "SWAP" // v1_abs v2_abs |
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.
[HIGH] Trailing end-of-line comments in code example
The abs2 example combines code and explanatory comments on the same lines, such as asm // v1 v2. This interleaving of code and comments makes the snippet harder to copy, paste, and edit while following the stack comments. The documentation style guide requires comments in examples to appear on their own lines immediately above the code they describe, rather than as trailing end-of-line comments (see https://github.com/ton-org/docs/blob/main/contribute/style-guide-extended.mdx?plain=1#L552-L555).
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
|
|
||
| The `mutate` keyword (see [mutability](/languages/tolk/syntax/mutability)) works by implicitly returning new values via the stack — both for regular and `asm` functions. | ||
|
|
||
| For better understanding, let's look at regular functions first. The compiler does all transformations automatically: |
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.
[HIGH] Use of first-person pronoun “let’s” in prose
The sentence “For better understanding, let's look at regular functions first.” uses the first-person plural “let's” to refer jointly to the author and reader. The documentation style guide specifies that articles must avoid first-person (“we”, “I”, “our”) and direct address (“you/your”) in order to maintain an impersonal, reference-style tone (see https://github.com/ton-org/docs/blob/main/contribute/style-guide-extended.mdx?plain=1#L170-L172). Keeping this phrasing would introduce a conversational narrative voice that is inconsistent with the rest of the docs.
| For better understanding, let's look at regular functions first. The compiler does all transformations automatically: | |
| For better understanding, examine regular functions first. The compiler applies all transformations automatically: |
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
|
|
||
| Introduce assembler functions only for rarely-used TVM instructions that are not covered by stdlib. For example, when manually parsing merkle proofs or calculating extended hashes. | ||
|
|
||
| However, attempting to micro-optimize with `asm` instead of writing straightforward code is not desired. The compiler is smart enough to generate optimal bytecode from consistent logic. For instance, it automatically inlines simple functions, so create one-liner methods without any worries about gas: |
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.
[HIGH] Marketing-style language about compiler behavior
The sentence “The compiler is smart enough to generate optimal bytecode from consistent logic.” uses subjective, marketing-style phrasing (“smart enough”, “optimal bytecode”) rather than a precise, factual description. The style guide explicitly discourages promotional language and vague superlatives in technical documentation, as they can be misleading about guarantees and obscure the concrete behaviors being documented (see https://github.com/ton-org/docs/blob/main/contribute/style-guide-extended.mdx?plain=1#L35-L40). In this context, the text should focus on the specific optimizations (such as inlining and store merging) that the compiler performs.
| However, attempting to micro-optimize with `asm` instead of writing straightforward code is not desired. The compiler is smart enough to generate optimal bytecode from consistent logic. For instance, it automatically inlines simple functions, so create one-liner methods without any worries about gas: | |
| However, attempting to micro-optimize with `asm` instead of writing straightforward code is not desired. The compiler generates efficient bytecode from consistent logic and automatically inlines simple functions, so one-liner methods remain gas-efficient: |
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
towards #1128