Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 679
feat: add an alwaysInline builtin#2895
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
CountBleck commented Jan 12, 2025
@JairusSW let me know if anything else is desired for this builtin |
HerrCai0907 left a comment
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.
what is the difference between inline and this builtin?
CountBleck commented Jan 12, 2025
@HerrCai0907 this gives you fine-grained control over when a function is inlined (by specifying it in the relevant call sites, instead of in the definition). @JairusSW has an example as to how this would be useful in as-json. |
JairusSW commented Jan 12, 2025
@CountBleck would it be possible to just rename it to |
CountBleck commented Jan 12, 2025
I originally intended to name it |
HerrCai0907 left a comment
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.
LGTM for implement part.
But I have some concern about design. I think we should avoid to pollute global scope. load / store / call_indirect can be mapped to low level wasm instruction directly so we keep them in global scope. But for alwaysInline and unchecked, I think we should place them in a special namespace or start with some special prefix.
For C++, std library used compiler-implemented function will start with __. some builtin function will start with __builtin. I wonder should we also do something like this?
e.g.
use this like
builtin_opt.unchecked(....)builtin_opt.alwaysInline(....);CountBleck commented Jan 13, 2025
Sounds like a good idea, but I'm hesitant to rename/move |
HerrCai0907 commented Jan 13, 2025
At least we can start from |
MaxGraey commented Jan 15, 2025
How about to use |
CountBleck commented Jan 15, 2025
I'll do that. What do you think about a namespace for |
db37514 to 165cfeaCompare
HerrCai0907 left a comment • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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.
I still have some concern about adding not so important function in global scope. It may break some code and it is quiet annoyed to find a new suitable function name for user.
Some search result from github, since AS does not have special language identifier, I search for all function named inlined in TS.
https://github.com/search?q=%22+inlined%28%22+language%3ATypeScript+&type=code
CountBleck commented Jan 17, 2025
I also checked SourceGraph and can't get anything relevant. I'm not opposed to the namespace idea, but idk if it'd inconvenience users without much of a benefit. |
dcodeIO commented Jan 17, 2025
Could this perhaps be a function associated with the existing |
JairusSW commented Mar 4, 2025
I'd tend to agree with this approach, and it can be done like so: /** Annotates a method, function or constant global as always inlined. */declarefunctioninline(...args: any[]): any;declarenamespaceinline{/** Annodates a function call as inlined */functionalways(...args: any[]): any;} |
CountBleck commented Mar 4, 2025
I didn't realize I left this hanging for so long, sorry! |
This builtin operates similar to unchecked, by setting a new FlowFlag that is checked in makeCallDirect in the area where the @inline decorator is checked, thereby achieving the same functionality as it.
4e7734b into AssemblyScript:mainUh oh!
There was an error while loading. Please reload this page.
Changes proposed in this pull request:
⯈ Add an
alwaysInlinebuiltin that inlines any inner function calls