Alex Crichton (Oct 19 2026 at 22:07): Alex Crichton (Oct 19 2026 at 22:07):I tested out removing #[readonly] from VMFuncRef fields as part of https://github.com/bytecodealliance/wasmtime/pull/14683 and I was surprised to see no substantive changes in the disas test suite. Everything was just loads dropping the readonly flag.
I tried adding a test like this:
(module
(type $f (func))
(func (param $x (ref $f))
(call_ref $f (local.get $x))
(call_ref $f (local.get $x))
)
)
but even that compiles to:
v3 = load.i64 user16 aligned readonly region2 v2+8
v4 = load.i64 notrap aligned readonly region3 v2+24
call_indirect sig0, v3(v4, v0)
v5 = load.i64 user16 aligned readonly region2 v2+8
v6 = load.i64 notrap aligned readonly region3 v2+24
call_indirect sig0, v5(v6, v0)
and I'm surprised by this -- is this a bug in Cranelift? v5 is dominated by v3 and v6 by v4, so shouldn't v5/v6 be eliminated loads with what's implemented in Cranelift?
Chris Fallin (Oct 19 2026 at 22:10):I would expect so! I'm in transit now but can stare at this a little bit later
Alex Crichton (Oct 19 2026 at 22:16):oh apparently .readonly() is only read in egraphs right now and it's only used when combined with can_move/notrap to make it a pure egraph node -- which in that case I'd expect v6 to be GVN'd... but that'd explain why v5 isn't GVN'd
Chris Fallin (Oct 19 2026 at 22:19):hmm, semantically at least I'd expect even trappable, cannot-move loads to disappear when dominated by another identical load. I had thought the "idempotent ops" mechanism was built for this case but my memory is fuzzy, will have to dig more
Alex Crichton (Oct 19 2026 at 22:19):oh wait right none of these are can_move, ok well that explains why nothing happens today at least
Chris Fallin (Oct 19 2026 at 22:20):(I guess these can't be notrap because we let a null funcref trap, right?)
Alex Crichton (Oct 19 2026 at 22:20):yeah, despite this being a non-nullable funcref...
Alex Crichton (Oct 19 2026 at 22:20):(we don't plumb that much type info yet)
Alex Crichton (Oct 19 2026 at 22:20):well, the lack of can_move explains why egraphs don't handle this
Chris Fallin (Oct 19 2026 at 22:20):in principle I guess they should be can-move? I mean readonly implies that whenever you read the thing, it'll have the same value
Chris Fallin (Oct 19 2026 at 22:20):(this is where my mental model for the semantics wants a clearer semantics)
Alex Crichton (Oct 19 2026 at 22:21):IIRC we did that historically but had to move away b/c it move above control flow saying "ok go actually do that"
Alex Crichton (Oct 19 2026 at 22:21):basically it was a control-flow sensitive decision to determine whether we should even do the load, but with can_move it moved up above the control flow
Chris Fallin (Oct 19 2026 at 22:21):hmm, ah, yeah. hoisting can move the location of the trap if there is a trap
Chris Fallin (Oct 19 2026 at 22:22):that shouldn't prevent us from GVN'ing on top of another one that is already forced though
Chris Fallin (Oct 19 2026 at 22:22):in principle at least
Alex Crichton (Oct 19 2026 at 22:22):makes sense yeah
Chris Fallin (Oct 19 2026 at 22:23):ok I'm on Portland lightrail with flaky cell right now but I'll poke at this more when I'm in my hotel, I'm now freshly annoyed by our semantics :-)
Alex Crichton (Oct 19 2026 at 22:23):heh no urgency of course, just wanted to flag it
Chris Fallin (Oct 23 2026 at 21:20):https://github.com/bytecodealliance/wasmtime/pull/14972 should address
Last updated: Oct 31 2026 at 23:03 UTC