Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pulley: Ungate SIMD proposal to enable more tests #9779

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Dec 10, 2024

This commit removes the simd proposal from "known panicking compiler
features" for Pulley by filling out some minor ABI bits around
loads/stores of v128 in the Cranelift ABI code. This unlocks running
spec_testsuite/*.wast in CI and expecting it to pass as otherwise
everything was rejected since that test suite requires SIMD and it never
even got to the Cranelift backend.

This then fixes a few minor issues here and there to get existing tests
passing now that they're actually being run. Some examples are:

  • Fix some copy/paste typos around callee-save clobbers and float
    registers.
  • Add vector load/store support to avoid panicking in ABI code (not well
    tested yet though).
  • Zero-extend the condition in brif's lowering to ensure that the
    entire register is defined. In the future more variants of br_if in
    Pulley are probably desired.
  • Limit store lowerings to just integer types, not vector types.
  • Fix Func::call with Pulley when the callee is a host function.

cc #9783

@alexcrichton alexcrichton requested review from a team as code owners December 10, 2024 18:32
@alexcrichton alexcrichton requested review from fitzgen and removed request for a team December 10, 2024 18:32
@alexcrichton
Copy link
Member Author

alexcrichton commented Dec 10, 2024

Note that this is currently based on #9775

This commit removes the simd proposal from "known panicking compiler
features" for Pulley by filling out some minor ABI bits around
loads/stores of v128 in the Cranelift ABI code. This unlocks running
`spec_testsuite/*.wast` in CI and expecting it to pass as otherwise
everything was rejected since that test suite requires SIMD and it never
even got to the Cranelift backend.

This then fixes a few minor issues here and there to get existing tests
passing now that they're actually being run. Some examples are:

* Fix some copy/paste typos around callee-save clobbers and float
  registers.
* Add vector load/store support to avoid panicking in ABI code (not well
  tested yet though).
* Zero-extend the condition in `brif`'s lowering to ensure that the
  entire register is defined. In the future more variants of `br_if` in
  Pulley are probably desired.
* Limit `store` lowerings to just integer types, not vector types.
* Fix `Func::call` with Pulley when the callee is a host function.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Dec 10, 2024
This commit is similar to bytecodealliance#9779 in that it's removing a proposal from
the "known list of panicking features" for Pulley to allow more tests to
run on Pulley. This then fills out a few miscellaneous instructions to
get a full suite of tests passing in Pulley related to memory64 and
other instructions.

prtest:full
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator pulley Issues related to the Pulley interpreter wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:config Issues related to the configuration of Wasmtime labels Dec 10, 2024
Copy link

Subscribe to Label Action

cc @fitzgen

This issue or pull request has been labeled: "cranelift", "pulley", "wasmtime:api", "wasmtime:config"

Thus the following users have been cc'd because of the following labels:

  • fitzgen: pulley

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Dec 10, 2024
Similar to bytecodealliance#9779 this remove the `GC_TYPES` feature from the list of
panicking features in Pulley. In doing so this then additionally fixes a
number of panics and then adds more tests that are working. Some other
minor instructions are filled out to ensure that tests are working on
both 32 and 64-bit platforms.
Copy link

Label Messager: wasmtime:config

It looks like you are changing Wasmtime's configuration options. Make sure to
complete this check list:

  • If you added a new Config method, you wrote extensive documentation for
    it.

    Our documentation should be of the following form:

    Short, simple summary sentence.
    
    More details. These details can be multiple paragraphs. There should be
    information about not just the method, but its parameters and results as
    well.
    
    Is this method fallible? If so, when can it return an error?
    
    Can this method panic? If so, when does it panic?
    
    # Example
    
    Optional example here.
    
  • If you added a new Config method, or modified an existing one, you
    ensured that this configuration is exercised by the fuzz targets.

    For example, if you expose a new strategy for allocating the next instance
    slot inside the pooling allocator, you should ensure that at least one of our
    fuzz targets exercises that new strategy.

    Often, all that is required of you is to ensure that there is a knob for this
    configuration option in wasmtime_fuzzing::Config (or one
    of its nested structs).

    Rarely, this may require authoring a new fuzz target to specifically test this
    configuration. See our docs on fuzzing for more details.

  • If you are enabling a configuration option by default, make sure that it
    has been fuzzed for at least two weeks before turning it on by default.


To modify this label's message, edit the .github/label-messager/wasmtime-config.md file.

To add new label messages or remove existing label messages, edit the
.github/label-messager.json configuration file.

Learn more.

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@fitzgen fitzgen added this pull request to the merge queue Dec 11, 2024
Merged via the queue into bytecodealliance:main with commit 363db4f Dec 11, 2024
132 checks passed
@alexcrichton alexcrichton deleted the pulley-ungate-simd branch December 11, 2024 15:18
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Dec 11, 2024
This commit is similar to bytecodealliance#9779 in that it's removing a proposal from
the "known list of panicking features" for Pulley to allow more tests to
run on Pulley. This then fills out a few miscellaneous instructions to
get a full suite of tests passing in Pulley related to memory64 and
other instructions.

prtest:full
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Dec 11, 2024
This commit is similar to bytecodealliance#9779 in that it's removing a proposal from
the "known list of panicking features" for Pulley to allow more tests to
run on Pulley. This then fills out a few miscellaneous instructions to
get a full suite of tests passing in Pulley related to memory64 and
other instructions.

prtest:full
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Dec 11, 2024
Similar to bytecodealliance#9779 this remove the `GC_TYPES` feature from the list of
panicking features in Pulley. In doing so this then additionally fixes a
number of panics and then adds more tests that are working. Some other
minor instructions are filled out to ensure that tests are working on
both 32 and 64-bit platforms.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Dec 11, 2024
This commit is similar to bytecodealliance#9779 in that it's removing a proposal from
the "known list of panicking features" for Pulley to allow more tests to
run on Pulley. This then fills out a few miscellaneous instructions to
get a full suite of tests passing in Pulley related to memory64 and
other instructions.

prtest:full
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Dec 11, 2024
Similar to bytecodealliance#9779 this remove the `GC_TYPES` feature from the list of
panicking features in Pulley. In doing so this then additionally fixes a
number of panics and then adds more tests that are working. Some other
minor instructions are filled out to ensure that tests are working on
both 32 and 64-bit platforms.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Dec 11, 2024
This commit is similar to bytecodealliance#9779 in that it's removing a proposal from
the "known list of panicking features" for Pulley to allow more tests to
run on Pulley. This then fills out a few miscellaneous instructions to
get a full suite of tests passing in Pulley related to memory64 and
other instructions.

prtest:full
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Dec 11, 2024
Similar to bytecodealliance#9779 this remove the `GC_TYPES` feature from the list of
panicking features in Pulley. In doing so this then additionally fixes a
number of panics and then adds more tests that are working. Some other
minor instructions are filled out to ensure that tests are working on
both 32 and 64-bit platforms.
github-merge-queue bot pushed a commit that referenced this pull request Dec 11, 2024
* pulley: Ungate memory64 feature

This commit is similar to #9779 in that it's removing a proposal from
the "known list of panicking features" for Pulley to allow more tests to
run on Pulley. This then fills out a few miscellaneous instructions to
get a full suite of tests passing in Pulley related to memory64 and
other instructions.

prtest:full

* Add notes about spectre speculation
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Dec 11, 2024
Similar to bytecodealliance#9779 this remove the `GC_TYPES` feature from the list of
panicking features in Pulley. In doing so this then additionally fixes a
number of panics and then adds more tests that are working. Some other
minor instructions are filled out to ensure that tests are working on
both 32 and 64-bit platforms.
github-merge-queue bot pushed a commit that referenced this pull request Dec 11, 2024
* pulley: Ungate the `GC_TYPES` feature from Pulley

Similar to #9779 this remove the `GC_TYPES` feature from the list of
panicking features in Pulley. In doing so this then additionally fixes a
number of panics and then adds more tests that are working. Some other
minor instructions are filled out to ensure that tests are working on
both 32 and 64-bit platforms.

* Flag test as now passing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift Issues related to the Cranelift code generator pulley Issues related to the Pulley interpreter wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:config Issues related to the configuration of Wasmtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants