Stream: git-wasmtime

Topic: wasmtime / PR #11272 x64: handle ISA features more comple...

Wasmtime GitHub notifications bot (Jul 17 2025 at 23:01):

abrown opened PR #11272 from abrown:asm-features to bytecodealliance:main:

This adds full boolean term support for instructions emitted in the new assembler (terms like (_64b | compat) & avx2). Despite doing more checks, this may be quicker too: instead of building up a SmallVec<InstructionSet> to compare against, this generates Rust code like the following that queries what features are available in the target via the AvailableFeatures trait:

#[must_use] // cranelift/assembler-x64/meta/src/generate/inst.rs:227
pub fn is_available(&self, features: &impl AvailableFeatures) -> bool {
    (features._64b() || features.compat()) && features.avx2() // cranelift/assembler-x64/meta/src/generate/inst.rs:232
}

With all of this in place, this PR has a large mechanical translation of all the old, incorrect feature definitions (_64b | compat | avx2) into their new, correct definitions ((_64b | compat) & avx2). I expect this will see a lot more use of this when using more instructions from AVX512, AVX10, APX, etc.

Wasmtime GitHub notifications bot (Jul 17 2025 at 23:01):

abrown requested wasmtime-compiler-reviewers for a review on PR #11272.

Wasmtime GitHub notifications bot (Jul 17 2025 at 23:01):

abrown requested alexcrichton for a review on PR #11272.

Wasmtime GitHub notifications bot (Jul 17 2025 at 23:08):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Jul 17 2025 at 23:08):

abrown created PR review comment:

So this is a bit unfortunate at the moment: we've lost the ability to see why this check has failed. I mulled this over for a bit, thinking how we can get this back. One option is to have the assembler just "return the right string" through some other method which we can paste in here. But I'm leaning toward adding a Inst::features() -> Features, much like we had before, but that would return a boolean term like the one we have available at the meta level. Though this would duplicate the some of Features, it would return an enum that is more widely usable than a string while I still have all of this paged in.

Either option, "return a string" or "return a Features", is really fine at this poing and I'm interested in feedback. Also, this seems like something that could get fixed up in a follow-on PR.

Wasmtime GitHub notifications bot (Jul 17 2025 at 23:08):

abrown edited PR review comment.

Wasmtime GitHub notifications bot (Jul 17 2025 at 23:11):

abrown commented on PR #11272:

I do expect this to fail in Winch: something about how we're using -Ccranelift-has-avx2 in param_av2.wat doesn't seem to propagate the right features into the ISA flags used here:

https://github.com/bytecodealliance/wasmtime/blob/3c9305c0bb8d614b37f8d121aa115c2fd822296e/winch/codegen/src/isa/x64/masm.rs#L3205-L3208

Wasmtime GitHub notifications bot (Jul 22 2025 at 16:05):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 22 2025 at 16:05):

alexcrichton created PR review comment:

Inst::features sounds reasonable to me. Implementation-wise I might recommend a different representation of Features that doesn't use Box internally but instead uses &'static ... and put everything into consts. That way there's no need to actually allocate anything and it's all just pointing at a bunch of static data (which is in theory deduplicated across functions too)

Wasmtime GitHub notifications bot (Jul 22 2025 at 16:07):

alexcrichton created PR review comment:

I think the discrepancy here with Winch is use_avx2 vs has_avx2 perhaps? Here I think it'd be reasonable to switch to has_* (for other methods too) where use_* affects codegen but has_* is the literal CPU features (in theory)

Wasmtime GitHub notifications bot (Jul 22 2025 at 16:07):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Aug 05 2025 at 22:12):

abrown updated PR #11272.

Wasmtime GitHub notifications bot (Aug 05 2025 at 22:13):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Aug 05 2025 at 22:13):

abrown created PR review comment:

See de476c2.

Wasmtime GitHub notifications bot (Aug 05 2025 at 22:13):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Aug 05 2025 at 22:13):

abrown created PR review comment:

See f529126.

Wasmtime GitHub notifications bot (Aug 05 2025 at 22:15):

abrown requested alexcrichton for a review on PR #11272.

Wasmtime GitHub notifications bot (Aug 06 2025 at 00:11):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Aug 06 2025 at 14:57):

alexcrichton merged PR #11272.

Last updated: Dec 06 2025 at 06:05 UTC