Skip to content

Enable cranelift-codegen's new all-arch feature to maintain full architecture support by default#193

Merged
sunfishcode merged 1 commit intobytecodealliance:masterfrom
mbestavros:architecture-control
Sep 19, 2019
Merged

Enable cranelift-codegen's new all-arch feature to maintain full architecture support by default#193
sunfishcode merged 1 commit intobytecodealliance:masterfrom
mbestavros:architecture-control

Conversation

@mbestavros
Copy link

This quick patch goes along with the corresponding PR in Cranelift adding the all-arch feature.

The changes in the linked PR will remove all architectural support by default, allowing dependent projects (such as Wasmtime) to specify exactly which architectures they want to support. In mainline Wasmtime's case, full architectural support is desired, so this patch will simply maintain existing functionality once the Cranelift changes are merged in.

Tagging @sunfishcode @npmccallum @steveej @alexcrichton for review.

@sunfishcode
Copy link
Member

This looks good! We can merge this once the Cranelift PR lands.

@mbestavros
Copy link
Author

@sunfishcode @bnjbvr Bump -- with the corresponding Cranelift PR merged, this should also be integrated to maintain the same level of support.

@steveej
Copy link
Contributor

steveej commented Aug 26, 2019

@tschneidereit do you know if we can run this through the new CI pipeline? I would be surprised if this breaks something but I'd still like to see the CI green before merging it.

@tschneidereit
Copy link
Member

@steveej apologies for the late reply! The new CI should run automatically for all new PRs/new commits in PRs, but you can also trigger it manually like so:

/AzurePipelines run

@tschneidereit
Copy link
Member

Unfortunately that doesn't seem to have worked :( Pushing a new commit here should work, though

@mbestavros mbestavros force-pushed the architecture-control branch 2 times, most recently from 88ee8de to 8f93d38 Compare August 29, 2019 16:09
@mbestavros
Copy link
Author

@steveej @sunfishcode The current test failures look related to the current 0.40 release of cranelift not having the required commit yet (seems like it just missed the cutoff for the 0.40 release...)

Is this a situation where we'd want to merge after the next Cranelift release?

@mbestavros
Copy link
Author

/AzurePipelines run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 193 in repo CraneStation/wasmtime

@mbestavros mbestavros force-pushed the architecture-control branch 2 times, most recently from 962496d to 3867c77 Compare September 3, 2019 20:09
@mbestavros
Copy link
Author

@sunfishcode Tests look like they pass now since the corresponding Cranelift commit is live -- is this ready to merge?

@sunfishcode
Copy link
Member

Yep, looks good.

@sunfishcode sunfishcode merged commit bd613ec into bytecodealliance:master Sep 19, 2019
mooori pushed a commit to mooori/wasmtime that referenced this pull request Jan 26, 2024
frank-emrich pushed a commit to frank-emrich/wasmtime that referenced this pull request Jun 12, 2024
avanhatt added a commit to wellesley-prog-sys/wasmtime that referenced this pull request Apr 9, 2025
Implement the encoded spec ops `clz` and `rev`. Use them to verify the
`clz` and `ctz` lowerings.

Updates avanhatt#42 #34

Co-authored-by: Vaishu Chintam <jc103@wellesley.edu>
dicej pushed a commit to dicej/wasmtime that referenced this pull request Jun 5, 2025
…n-diff-smaller

Reduce diff with main in wit-bindgen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants