wasmtime / issue #5862 Support lifetimes in context in ISLE · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #5862 Support lifetimes in context in ISLE

Wasmtime GitHub notifications bot (Feb 23 2023 at 14:00):

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 a Context 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.

Wasmtime GitHub notifications bot (Feb 23 2023 at 14:02):

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 a Context 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.

Wasmtime GitHub notifications bot (Feb 23 2023 at 14:15):

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 a Context 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.

Wasmtime GitHub notifications bot (Feb 25 2023 at 03:12):

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?

Wasmtime GitHub notifications bot (Feb 25 2023 at 03:19):

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))

Wasmtime GitHub notifications bot (Feb 25 2023 at 03:47):

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: Nov 22 2024 at 16:03 UTC