Stream: git-wasmtime

Topic: wasmtime / issue #5409 Fix optimization rules for narrow ...

Wasmtime GitHub notifications bot (Dec 09 2022 at 20:07):

bjorn3 commented on issue #5409:

This was producing iconsts with set high bits beyond their types' width, which is not legal.

I thought backends were supposed to mask it. I believe cg_clif sometimes sign extends immediates.

Wasmtime GitHub notifications bot (Dec 09 2022 at 20:15):

cfallin commented on issue #5409:

This was producing iconsts with set high bits beyond their types' width, which is not legal.

I thought backends were supposed to mask it. I believe cg_clif sometimes sign extends immediates.

Hmm, it seems we discussed this and filed #3059 a while ago, and haven't done any thinking about it since. I suspect that we aren't doing such masking everywhere we need to be, and requiring rules everywhere to do the masking (e.g., const-folding an icmp eq would require comparing only the relevant bits) seems cumbersome. I'd rather we have an invariant that values stay in range.

For now I guess we can keep the status quo, but we should think about this some more (on that issue probably)!

Wasmtime GitHub notifications bot (Dec 09 2022 at 21:58):

jameysharp commented on issue #5409:

BTW I'm curious if the new optimization rules have any impact on any benchmarks. Do these patterns show up in any CLIF we're generating from wasm, or do our current benchmarks compile to the same sequence of instructions either way? Not terribly important but if this is a significant win for some reason, that'd be nice to know.

Wasmtime GitHub notifications bot (Dec 09 2022 at 22:09):

cfallin commented on issue #5409:

That's a good question, re: the icmp folding; I didn't do tests on this. Honestly I'd be somewhat surprised if they did though, given that the benchmarks are optimized .wasms and the sorts of redundancies that show up in the translated CLIF are not usually things that const-folding applies to.

Wasmtime GitHub notifications bot (Dec 09 2022 at 22:17):

cfallin commented on issue #5409:

And, to follow up, a quick smoketest with bz2 shows no runtime impact with the additional icmp folding roles.

Last updated: Dec 23 2024 at 12:05 UTC