Generalize "global values" to "templates". #1299
Conversation
Rename "global values" to "templates", to better describe their role as expressions which can be provided by frontends and expanded by the code generator as needed. And, start to generalize them, by introducing `Call` and `IfElse` templates. This is a step towards generalizing templates into a form that can be independent of Cranelift, and allow embedding environments to provide a single description of their data structures which is independent of any specific JIT. And, it's a step toward being able to use templates to describe GC barrier instruction sequences. This also helps avoid some confusion between Cranelift IR global values, wasm global variables, global variable data objects, and "global" variables in regalloc.
There was a problem hiding this comment.
I'll be on PTO this afternoon, so I won't have time to review it until I get back (around Jan 6th). I've got one strong objection to the current PR, though, from what I've skimmed in test cases: this does put templates within a single function, causing many template copies. It'd be fine if it were only for globals (because then it would behave as previously), but since this is expanded for calls and if/else, we should really try to make them global to several functions to share their representation, as indicated in my other comment in the issue. At the very least I'd like to know why it shouldn't be done like that, or if you're planning to do it later.
Also, please break up this PR into smaller commits to make the review possible ;). For instance a first commit introduces templates everywhere, 2. global is converted into a template, 3. if/else and call templates are introduced.
(If they're not present, but from looking at the diffstat I don't think they are) can you add tests for the call and if/else templates too, please?
|
And a supplementary question: this seems to redefine a new IR for templates, with call, if/then/else, etc. Wouldn't it be possible to reuse the same IR/layout notions that Cranelift already knows, to avoid a lot of concept (and i expect code) duplication? Otherwise, users would be limited to the template IR for their needs, and would need to add template IR nodes as they require, which wouldn't make a very pleasant developer experience. |
|
I believe that @sunfishcode was intending that this could be reused across lightbeam and cranelift so that we could use the same barrier implementations at whatever tier JIT we were on in Wasmtime. This is a trade off: we add a little complexity in the form of a new (or at least, expanding) IR language, and we sacrifice some control over the precise generated code for a barrier / global / table. What we gain is the portability. And this is much more limited in scope compared to the template strawperson I wrote up in #1176. The only precise control we really need for barriers are which side of an if/else should fallthrough, I think. This does address that with the A reasonable alternative would be to say that lightbeam, as a baseline-style JIT rather than an optimizing one, would always use out-of-line calls for barriers, and then we could have a proper subset of clif for the templates. |
Spidermonkey will need to emit pre/post barriers for global.set/get to a reference type. #1176 and #1299 plan to add a template concept that could be used to implement this. Once that has been stabilized, we should be able to remove this code in favor of templates easily.
|
Thanks for the PR again, and as a procedural note the Cranelift repository has now merged into the wasmtime repository. PRs are no longer landing in this repository, and unfortunately there's no "one button" solution to move a PR to the wasmtime repository. A script has been prepared, however, to assist you in transferring this PR to the wasmtime repo. Feel free to reach out on Zulip with any questions! |
Rename "global values" to "templates", to better describe their role as
expressions which can be provided by frontends and expanded by the code
generator as needed. And, start to generalize them, by introducing
CallandIfElsetemplates.This is a step towards generalizing templates into a form that can be
independent of Cranelift, and allow embedding environments to provide a
single description of their data structures which is independent of any
specific JIT. And, it's a step toward being able to use templates to
describe GC barrier instruction sequences.
This also helps avoid some confusion between Cranelift IR global values,
wasm global variables, global variable data objects, and "global"
variables in regalloc.
[Edited - added the bug that discusses this]