Wasmtime GitHub notifications bot (May 01 2024 at 08:34): Wasmtime GitHub notifications bot (May 01 2024 at 08:34):jameysharp opened PR #8515 from jameysharp:data-const-expr to bytecodealliance:main:
This shouldn't change any behavior currently, but prepares us for supporting extended constant expressions.
I want to think about this a bit more before merging it but here's a draft for @fitzgen or anyone else who wants to take a look.
Wasmtime GitHub notifications bot (May 01 2024 at 08:36):jameysharp edited PR #8515:
This shouldn't change any behavior currently, but prepares us for supporting extended constant expressions.
I want to think about this a bit more before merging it but here's a draft for @fitzgen or anyone else who wants to take a look. This is basically the same change as #8514 but for memories instead of tables, and the existing code was more complicated for this case.
Wasmtime GitHub notifications bot (May 01 2024 at 18:29):jameysharp updated PR #8515.
Wasmtime GitHub notifications bot (May 01 2024 at 18:37):jameysharp updated PR #8515.
Wasmtime GitHub notifications bot (May 01 2024 at 18:56):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (May 01 2024 at 18:56):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (May 01 2024 at 18:56):fitzgen created PR review comment:
nitpicky suggestion, take it or leave it: I'd personally rename this
InitMemoryAtRunTimeand the other impl asInitMemoryAtCompileTime. I think it is a little more clear but maybe it is just me.
Wasmtime GitHub notifications bot (May 01 2024 at 18:56):fitzgen created PR review comment:
It might be better to turn
ConstEvalContextinto a trait -- I initially did that but then every use site was the same so I made it concrete. But if it were a trait, then we could use aResultfor getting globals and funcrefs and return anErrwhen they aren't available at compile time. This would allow both reusingevalhere so you don't have to do this matching (nor in the other PR, iirc) and also support const expression offsets like(i32.add (i32.const 3) (i32.const 5))for compile-time heap image computation.
Wasmtime GitHub notifications bot (May 01 2024 at 20:51):jameysharp submitted PR review.
Wasmtime GitHub notifications bot (May 01 2024 at 20:51):jameysharp created PR review comment:
I wasn't entirely happy with the names I picked and considered doing something like you suggest. So it's not hard to convince me on this one!
Wasmtime GitHub notifications bot (May 03 2024 at 20:28):jameysharp submitted PR review.
Wasmtime GitHub notifications bot (May 03 2024 at 20:28):jameysharp created PR review comment:
It turns out it's easy to turn
ConstEvalContextinto a trait, but it's not useful without bigger changes that I haven't been able to figure out yet.
ConstExprEvaluatorcan only be used within the wasmtime crate, because it relies onValRaw. Everything that does const evaluation in that crate already has access to anInstance. All the places that need const evaluation before instantiation are in wasmtime-environ or wasmtime-types.I tried factoring out a type-specific, integer-only const-evaluator that can live in wasmtime-types alongside
ConstExprandConstOp. But the general-purpose const-evaluator is still necessary for initializing globals and I'm not real happy with duplicating this functionality.Maybe we need a trait in wasmtime-types that
ValorValRawcan implement?
Wasmtime GitHub notifications bot (May 03 2024 at 21:41):jameysharp updated PR #8515.
Wasmtime GitHub notifications bot (May 03 2024 at 21:44):jameysharp updated PR #8515.
Wasmtime GitHub notifications bot (May 03 2024 at 21:44):jameysharp has marked PR #8515 as ready for review.
Wasmtime GitHub notifications bot (May 03 2024 at 21:44):jameysharp requested elliottt for a review on PR #8515.
Wasmtime GitHub notifications bot (May 03 2024 at 21:44):jameysharp requested wasmtime-core-reviewers for a review on PR #8515.
Wasmtime GitHub notifications bot (May 03 2024 at 21:49):jameysharp commented on PR #8515:
I've decided it's more complicated than I want to tackle in this PR to make const-eval available at compile time, so I hope to come back to that later. I've renamed types as @fitzgen suggested (though I decided I liked
AtInstantiationbetter thanAtRunTime).And I've spent a couple days thinking about this and decided I don't have particularly better ideas for anything in here, so I've marked it ready for review now.
Wasmtime GitHub notifications bot (May 03 2024 at 22:43):jameysharp edited PR #8515:
This shouldn't change any behavior currently, but prepares us for supporting extended constant expressions.
~I want to think about this a bit more before merging it but here's a draft for @fitzgen or anyone else who wants to take a look.~ This is ready for review. This is basically the same change as #8514 but for memories instead of tables, and the existing code was more complicated for this case.
Wasmtime GitHub notifications bot (May 03 2024 at 23:02):elliottt requested fitzgen for a review on PR #8515.
Wasmtime GitHub notifications bot (May 04 2024 at 15:56):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (May 04 2024 at 15:56):fitzgen created PR review comment:
Maybe we need a trait in wasmtime-types that
ValorValRawcan implement?Yeah I think we'll have to do something roughly like this.
Wasmtime GitHub notifications bot (May 04 2024 at 16:01):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (May 04 2024 at 16:24):fitzgen merged PR #8515.
Last updated: Nov 22 2024 at 16:03 UTC