ISLE support for references with context lifetime · cranelift

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 Cranelift types used in the ISLE context, so I must use a reference for them.

As long as I'm aware of, ISLE doesn't emit a lifetime parameter on the context, so ISLE can't generate code like:

// ISLE generated Context
trait Context<'a> {
    fn get_foo(&mut self) -> FooRef<'a>;
}

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

I'm thinking of patching ISLE so that it can emit a lifetime parameter for the context type and defined primitive types. Is this a good idea, or is there any workaround I haven't yet discovered?

bjorn3 (Feb 21 2023 at 16:16):

bjorn3 (Feb 21 2023 at 16:17):

I don't think references will work as FooRef presumably is still needed when you mutate the IR, which invalidates all outstanding references.

Chris Fallin (Feb 21 2023 at 16:49):

Happy to think about options here, and if you have a working solution to start with, that's even better! The issue I ran into before with this is that all external calls are on a mut Context, so outstanding borrows cause lifetime-overlap errors. In the end it was simpler to use value semantics everywhere

Chris Fallin (Feb 21 2023 at 16:50):

I'd echo the suggestions to perhaps use entity indices as an alternative, unless there's a strong reason that won't work (e.g. the borrowed thing is deep in a data structure that can't be easily indexed)

Rynco Maekawa (Feb 22 2023 at 03:34):

Thanks for the reply! The reason why I think FooRef<'a> would work is that it has the same lifetime as the whole context (Context<'a>), so it is not constrained by the &mut Context borrow lifetime (I'm not mutating existing data structures; The whole input to ISLE is immutably borrowed and I'm generating new code (LLVM IR) based on that).

I think using entity indices is a good idea to test out with, but doing so would require changing many other parts of my codebase. So for now I'm going to try to add a type parameter to the context and see how well it goes.

bjorn3 (Feb 22 2023 at 09:46):

In that case keeping references around should indeed be fine. Adding support for it to ISLE makes sense.

Rynco Maekawa (Feb 23 2023 at 03:25):

I'm now digging into the code. My plan on changing the language is the following:

<def> ::= ...
        | "(" "pragma" <pragma> ")"  ;; Seems there's already a pragma parser in the code!

;; Adds the lifetime pragma. Lifetimes are always bound to the context.
<pragma> ::= "context_lifetime" <lifetime-ident>*

;; Lifetime identifiers start with quotes (`'`)
<lifetime-ident> ::= "'" ( "A".."Z" | "a".."z" | "_" | "$" | "0".."9" | "." )*

;; Primitive types can now have a lifetime parameter.
<typevalue> ::= "(" "primitive" <primitivetype> ")" ;; <-
              | ...

<primitivetype> ::= <ident>
                  | "(" <ident> <lifetime-ident>* ")"

And by the way, since I already have a plan, should I open up an issue in the wasmtime repo now?

Chris Fallin (Feb 23 2023 at 16:23):

Sure, please feel free to create an issue and all of us can discuss further there!

Chris Fallin (Feb 23 2023 at 16:25):

Support lifetimes in context in ISLE · Issue #5862 · bytecodealliance/wasmtime

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> { ...

Stream: cranelift

Topic: ISLE support for references with context lifetime

Rynco Maekawa (Feb 21 2023 at 10:06):