Stream: git-wasmtime

Topic: wasmtime / issue #5444 Sync cranelift's preset list with ...

view this post on Zulip Wasmtime GitHub notifications bot (Dec 15 2022 at 12:29):

bjorn3 opened issue #5444:

Feature

Cranelift already has a bunch of presets for cpu families. It doesn't include newer cpu families though. This is a request to also add those newer cpu families.

Benefit

This allows passing the rustc -Ctarget-cpu argument directly to Cranelift in cg_clif without requiring a map from LLVM cpu name to Cranelift preset.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 15 2022 at 16:40):

jameysharp labeled issue #5444:

Feature

Cranelift already has a bunch of presets for cpu families. It doesn't include newer cpu families though. This is a request to also add those newer cpu families.

Benefit

This allows passing the rustc -Ctarget-cpu argument directly to Cranelift in cg_clif without requiring a map from LLVM cpu name to Cranelift preset.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 15 2022 at 16:41):

jameysharp commented on issue #5444:

Sounds reasonable to me!

Do our existing names already match LLVM's names? If any don't, I think we should keep the current names as aliases in addition to the LLVM names.

I'd also encourage whoever implements this to leave a comment in each relevant file in cranelift/codegen/meta/src/isa/ with a link to the corresponding source in LLVM.

@bjorn3, I'm labeling this as a "good first issue". If you'd like somebody else to take it on, could you comment here with links to the best sources for LLVM's CPU family names?

view this post on Zulip Wasmtime GitHub notifications bot (Dec 15 2022 at 17:11):

bjorn3 commented on issue #5444:

Do our existing names already match LLVM's names? If any don't, I think we should keep the current names as aliases in addition to the LLVM names.

I believe they do at least for x86_64.

If you'd like somebody else to take it on, could you comment here with links to the best sources for LLVM's CPU family names?

For x86/x86_64 this is: https://github.com/llvm/llvm-project/blob/d4493dd1ed58ac3f1eab0c4ca6e363e2b15bfd1c/llvm/lib/Target/X86/X86.td#L1300-L1643 The actual feature lists are declared in the section above.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 15 2022 at 17:28):

cfallin commented on issue #5444:

A thought: would it make sense to have variants/architecture levels for x86 in target-lexicon, the same way it seems to have these for at least ARM, RISC-V and a few others? I'm definitely in favor of de-facto standardization in this case; there's no reason to try to define a different view of the world than what LLVM does. But the place for that might be in a crate whose job is to provide a taxonomy of targets :-)

view this post on Zulip Wasmtime GitHub notifications bot (Dec 15 2022 at 17:33):

bjorn3 commented on issue #5444:

target-lexicon parses rustc target triples. Rustc target triples distinguish between variants for arm, riscv and a couple of others, but they do not distinguish between target levels on x86_64 and only basic difference on x86 (i586 without sse and i686 with sse). These variants where rustc distinguishes between them generally affect the abi or are significant perf improvements for regular usecases.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 20 2022 at 13:39):

MozarellaMan commented on issue #5444:

Hey! I was looking at giving this a try, I just have a few questions. If I'm understanding this right - it would be basically making sure cranelift has at least these presets that rustc (on x86_64) has:

From running: rustc --print target-cpus

alderlake amdfam10 athlon athlon-4 athlon-fx athlon-mp athlon-tbird athlon-xp athlon64 athlon64-sse3 atom barcelona bdver1 bdver2 bdver3 bdver4 bonnell broadwell btver1 btver2 c3 c3-2 cannonlake cascadelake cooperlake core-avx-i core-avx2 core2 corei7 corei7-avx generic geode goldmont goldmont-plus haswell i386 i486 i586 i686 icelake-client icelake-server ivybridge k6 k6-2 k6-3 k8 k8-sse3 knl knm lakemont nehalem nocona opteron opteron-sse3 penryn pentium pentium-m pentium-mmx pentium4 pentium4m pentiumpro prescott rocketlake sandybridge sapphirerapids silvermont skx skylake skylake-avx512 slm tigerlake tremont westmere winchip-c6 winchip2 x86-64 x86-64-v2 x86-64-v3 x86-64-v4 yonah znver1 znver2 znver3

From @bjorn3's link I can compare what feature settings LLVM has and what settings cranelift has in cranelift/codegen/meta/src/isa/. It looks like this would require a few new settings in cranelift.

For example LLVM has: def FeatureSGX : SubtargetFeature<"sgx", "HasSGX", "true", "Enable Software Guard Extensions">;
Which isn't present in cranelift's settings... but from what I understand these feature settings are only useful if cranelift actually uses them right?

Also, each BoolSettingIndex in cranelift/codegen/meta/src/isa/ has a comment field - what is that?

view this post on Zulip Wasmtime GitHub notifications bot (Dec 20 2022 at 14:11):

bjorn3 commented on issue #5444:

I think adding new features isn't necessary until Cranelift actually starts using them.

The comment argument seems to indicate which cpuid bit determines if the feature is available or not. Not sure if it is used anywhere though.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 20 2022 at 17:19):

jameysharp commented on issue #5444:

I agree, don't bother adding new features; we should only add them when we have a use for them.

The comment field appears to be literally used in a Rust doc comment in the generated source code. The Flags types that have those doc comments apparently aren't visible on docs.rs though, so that's not super helpful. Figuring out why they're missing would be nice...

view this post on Zulip Wasmtime GitHub notifications bot (Dec 22 2022 at 16:39):

MozarellaMan commented on issue #5444:

From looking at this a bit more, it seems like a ton of the target cpus in LLVM's list would basically be the same as the baseline cranelift preset. Is it worth adding these? Or just the newer CPU familes that use at least one of the features cranelift uses?

view this post on Zulip Wasmtime GitHub notifications bot (Dec 22 2022 at 16:43):

bjorn3 commented on issue #5444:

Is it worth adding these?

Yes, otherwise passing -Ctarget-cpu=some_cpu_that_wasnt_added_as_preset to rustc when using cg_clif would still fail due to the missing preset. Older ones are less of a priority though, so if you consider it too much work to add all, I did be happy if you only add newer ones too.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 14 2023 at 00:06):

bjorn3 commented on issue #5444:

Fixed by https://github.com/bytecodealliance/wasmtime/pull/5490

view this post on Zulip Wasmtime GitHub notifications bot (Jan 14 2023 at 00:06):

bjorn3 closed issue #5444:

Feature

Cranelift already has a bunch of presets for cpu families. It doesn't include newer cpu families though. This is a request to also add those newer cpu families.

Benefit

This allows passing the rustc -Ctarget-cpu argument directly to Cranelift in cg_clif without requiring a map from LLVM cpu name to Cranelift preset.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 14 2023 at 00:13):

jameysharp commented on issue #5444:

I wasn't going to close this yet since so far we only have the Intel CPU presets, not AMD or other vendors. But at least we have a solid idea of how to finish off the work now!

view this post on Zulip Wasmtime GitHub notifications bot (Jan 14 2023 at 00:13):

jameysharp reopened issue #5444:

Feature

Cranelift already has a bunch of presets for cpu families. It doesn't include newer cpu families though. This is a request to also add those newer cpu families.

Benefit

This allows passing the rustc -Ctarget-cpu argument directly to Cranelift in cg_clif without requiring a map from LLVM cpu name to Cranelift preset.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 14 2023 at 07:20):

bjorn3 commented on issue #5444:

Right, forgot about AMD.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 14 2023 at 14:48):

MozarellaMan commented on issue #5444:

Will be looking at doing the AMD ones now, sorry I just started with Intel ones to get feedback on the approach :)

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2024 at 19:11):

bjorn3 closed issue #5444:

Feature

Cranelift already has a bunch of presets for cpu families. It doesn't include newer cpu families though. This is a request to also add those newer cpu families.

Benefit

This allows passing the rustc -Ctarget-cpu argument directly to Cranelift in cg_clif without requiring a map from LLVM cpu name to Cranelift preset.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2024 at 19:11):

bjorn3 commented on issue #5444:

Fixed by https://github.com/bytecodealliance/wasmtime/pull/5575

Last updated: Oct 23 2024 at 20:03 UTC