lynzrand opened issue #5862:
Feature
Add lifetime parameters in ISLE-generated
Context
, so that it can return values with lifetime parameters (not'static
lifetime).// ISLE generated Context trait Context<'a> { fn get_foo(&mut self) -> FooRef<'a>; }
In this way, the context can be implemented like:
type FooRef<'a> = &'a Foo; #[derive(Debug)] struct Foo(i32); struct FooCtx<'a> { foo: &'a Foo } impl<'a> Context<'a> for FooCtx<'a> { fn get_foo(&mut self) -> FooRef<'a> { self.foo } }
(previously discussed in Zulip: https://bytecodealliance.zulipchat.com/#narrow/stream/217117-cranelift/topic/ISLE.20support.20for.20references.20with.20context.20lifetime)
Benefit
I'm currently using
cranelift-isle
for a project of my own. However, unlike Cranelift, the types that I use with ISLE cannot be cheaply cloned like those used in Cranelift. If aContext
can return values with non-'static
lifetime, I can efficiently reference into readonly input data structures.Implementation
I am implementing it in my fork: https://github.com/lynzrand/wasmtime/tree/isle-patch
Additions to the language:
- Lifetime identifiers:
'a
is a valid lifetime identifier.- A pragma statement:
(pragma context_lifetime 'a 'b ...)
.- A new kind of primitive type definition with lifetime:
(type Node (primitive (NodeRef 'a)))
.Modifications:
By adding a pragma statement:
(pragma context_lifetime 'a 'b)
... the codegen will now generate the context trait with the specified lifetimes:
trait Context<'a, 'b> { // ... }
These lifetimes can then be used in primitive type definitions:
(type Node (primitive (NodeRef 'a)))
... and reflected in generated code:
pub fn constructor_lower<'a, 'b, C: Context<'a, 'b>>( ctx: &mut C, arg0: NodeRef<'a>, ) -> ...
My implementation is currently pretty naive and lacks polishing. I plan to continue testing and polishing it as I continue on my project.
Alternatives
I can choose to use
impl Copy
indices or other kinds of keys to index structures instead of fancy references, like Cranelift does.
lynzrand edited issue #5862:
Feature
Add lifetime parameters in ISLE-generated
Context
, so that it can return values with lifetime parameters (not'static
lifetime).// ISLE generated Context trait Context<'a> { fn get_foo(&mut self) -> FooRef<'a>; }
In this way, the context can be implemented like:
type FooRef<'a> = &'a Foo; #[derive(Debug)] struct Foo(i32); struct FooCtx<'a> { foo: &'a Foo } impl<'a> Context<'a> for FooCtx<'a> { fn get_foo(&mut self) -> FooRef<'a> { self.foo } }
(previously discussed in Zulip: https://bytecodealliance.zulipchat.com/#narrow/stream/217117-cranelift/topic/ISLE.20support.20for.20references.20with.20context.20lifetime)
Benefit
I'm currently using
cranelift-isle
for a project of my own. However, unlike Cranelift, the types that I use with ISLE cannot be cheaply cloned like those used in Cranelift. If aContext
can return values with non-'static
lifetime, I can efficiently reference into readonly input data structures.Implementation
I am implementing it in my fork: https://github.com/lynzrand/wasmtime/tree/isle-patch
Additions to the language:
- Lifetime identifiers:
'a
is a valid lifetime identifier.- A pragma statement:
(pragma context_lifetime 'a 'b ...)
.- A new kind of primitive type definition with lifetime:
(type Node (primitive (NodeRef 'a)))
.Modifications:
By adding a pragma statement:
(pragma context_lifetime 'a 'b)
... the codegen will now generate the context trait with the specified lifetimes:
trait Context<'a, 'b> { // ... }
These lifetimes can then be used in primitive type definitions:
(type Node (primitive (NodeRef 'a))) (decl lower (Node) ...) (rule (lower (Node ...)) ...)
... and reflected in generated code:
pub fn constructor_lower<'a, 'b, C: Context<'a, 'b>>( ctx: &mut C, arg0: NodeRef<'a>, ) -> ...
My implementation is currently pretty naive and lacks polishing. I plan to continue testing and polishing it as I continue on my project.
Alternatives
I can choose to use
impl Copy
indices or other kinds of keys to index structures instead of fancy references, like Cranelift does.
lynzrand edited issue #5862:
Feature
Add lifetime parameters in ISLE-generated
Context
, so that it can return values with lifetime parameters (not'static
lifetime).// ISLE generated Context trait Context<'a> { fn get_foo(&mut self) -> FooRef<'a>; }
In this way, the context can be implemented like:
type FooRef<'a> = &'a Foo; #[derive(Debug)] struct Foo(i32); struct FooCtx<'a> { foo: &'a Foo } impl<'a> Context<'a> for FooCtx<'a> { fn get_foo(&mut self) -> FooRef<'a> { self.foo } }
(previously discussed in Zulip: https://bytecodealliance.zulipchat.com/#narrow/stream/217117-cranelift/topic/ISLE.20support.20for.20references.20with.20context.20lifetime)
Benefit
I'm currently using
cranelift-isle
for a project of my own. However, unlike Cranelift, the types that I use with ISLE cannot be cheaply cloned like those used in Cranelift. If aContext
can return values with non-'static
lifetime, I can efficiently reference into readonly input data structures.Implementation
I am implementing it in my fork: https://github.com/lynzrand/wasmtime/tree/isle-patch
Additions to the language:
- Lifetime identifiers:
'a
is a valid lifetime identifier.- A pragma statement:
(pragma context_lifetime 'a 'b ...)
.- A new kind of primitive type definition with lifetime:
(type Node (primitive (NodeRef 'a)))
.Modifications:
By adding a pragma statement:
(pragma context_lifetime 'a 'b)
... the codegen will now generate the context trait with the specified lifetimes:
trait Context<'a, 'b> { // ... }
These lifetimes can then be used in primitive type definitions:
(type Node (primitive (NodeRef 'a))) (decl lower (Node) ...) (rule (lower (Node ...)) ...)
... and reflected in generated code:
pub fn constructor_lower<'a, 'b, C: Context<'a, 'b>>( ctx: &mut C, arg0: NodeRef<'a>, ) -> ...
(I have yet to look into enums.)
My implementation is currently pretty naive and lacks polishing. I plan to continue testing and polishing it as I continue on my project.
Alternatives
I can choose to use
impl Copy
indices or other kinds of keys to index structures instead of fancy references, like Cranelift does.
cfallin commented on issue #5862:
This design looks reasonable to me!
The main reasons not to do it, IMHO, would be either that it significantly complexifies the codegen or is somehow incompatible with it. But both of these are implementation-related and we can judge them objectively once we see your implementation. At the language level, the way that you've proposed to add this seems pretty unintrusive. We likely won't use it in the Cranelift application of ISLE but we can keep it working and exercised with islec tests.
@jameysharp, do you foresee any issues with the new codegen in managing the new kinds of types?
lynzrand commented on issue #5862:
For the implementation, please see this branch: lynzrand/wasmtime@isle-patch
I have implemented it for primitive types, and are already using it in a tiny part of my codebase. I haven't met any issues _yet_.
Here's a piece of the actual source code I used:
;; Anything related to function refeerences are 'f, and to LLVM are 'a (pragma context_lifetime 'a 'f) (type OpCode extern (enum ;; Duplicated code with ir/src/opcode.rs. TODO: deduplicate Start Region EndRegion If IfTrue IfFalse Merge Return Finish IConst ;; ... )) (type Ty extern (enum Any ;; ... )) (type Node (primitive (NodeRef 'f))) (type ParamFmt (primitive (ParamFmtRef 'f))) (type NodeId (primitive NodeId)) (type LLV (primitive (BasicValueEnum 'a))) ;; LLVM Value (decl i32 (i32) ParamFmt) (extern extractor i32 param_i32) (decl Node (OpCode ParamFmt Ty) Node) (extern extractor Node ex_node) (decl lower (Node) LLV) (rule (lower (Node (OpCode.IConst) (i32 x) _)) (llvm_iconst x))
cfallin commented on issue #5862:
Skimming the diff on your branch, the implementation looks pretty reasonable -- please do feel free to put up a PR and we'll do a proper review, likely early next week :-)
Last updated: Dec 23 2024 at 12:05 UTC