Skip to content

Add TargetIsa::map_dwarf_register; fixes #1471 - #1487

Merged
abrown merged 1 commit into
bytecodealliance:masterfrom
abrown:fix-1471
Apr 9, 2020
Merged

Add TargetIsa::map_dwarf_register; fixes #1471#1487
abrown merged 1 commit into
bytecodealliance:masterfrom
abrown:fix-1471

Conversation

@abrown

@abrown abrown commented Apr 8, 2020

Copy link
Copy Markdown
Member

This exposes the functionality of fde::map_reg on the TargetIsa trait, avoiding compilation errors on architectures where register mapping is not yet supported. The change is conditially compiled under the unwind feature.

@abrown
abrown requested a review from peterhuene April 8, 2020 21:21
@abrown

abrown commented Apr 8, 2020

Copy link
Copy Markdown
Member Author

@peterhuene, not sure how this will play with #1466 but this does seem like the better long-term solution. Also, I hope my liberal use of #[cfg(feature = "unwind")] is what we are looking for here.

@abrown

abrown commented Apr 8, 2020

Copy link
Copy Markdown
Member Author

@stefson, any chance you can try a compile of this branch to see if it actually fixes #1471?

@peterhuene peterhuene left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM 👍, but let's get @yurydelendik's opinion as well.

Just one nit comment; I think this should unblock the arm build of wasmtime-environ.

Comment thread cranelift/codegen/src/isa/mod.rs Outdated
@peterhuene

Copy link
Copy Markdown
Member

I'll tackle any conflicts this might cause for #1466.

@yurydelendik

Copy link
Copy Markdown
Contributor

LGTM 👍, but let's get @yurydelendik's opinion as well.

I'm good with the approach.

@github-actions github-actions Bot added the cranelift Issues related to the Cranelift code generator label Apr 8, 2020
@github-actions

github-actions Bot commented Apr 8, 2020

Copy link
Copy Markdown

Subscribe to Label Action

This issue or pull request has been labeled: "cranelift"

Users Subscribed to "cranelift"

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

Learn more.

This exposes the functionality of `fde::map_reg` on the `TargetIsa` trait, avoiding compilation errors on architectures where register mapping is not yet supported. The change is conditially compiled under the `unwind` feature.
@stefson

stefson commented Apr 9, 2020

Copy link
Copy Markdown
Contributor

@stefson, any chance you can try a compile of this branch to see if it actually fixes #1471?

@abrown it seems to depend on an unreleased version of witx ?

/tmp/wasmtime $ cargo build --target armv7-unknown-linux-gnueabihf
Updating crates.io index
error: failed to select a version for the requirement witx = "^0.8.5"
candidate versions found which didn't match: 0.8.4
location searched: /tmp/wasmtime/crates/wasi-common/WASI/tools/witx
required by package wig v0.15.0 (/tmp/wasmtime/crates/wasi-common/wig)

@abrown

abrown commented Apr 9, 2020

Copy link
Copy Markdown
Member Author

@stefson, I see witx at 0.8.5 available on crates.io so I'm not exactly sure what is going on (@pchickey, is it clear to you?). Perhaps there is a git submodule update needed when jumping on to this branch?

@stefson

stefson commented Apr 9, 2020

Copy link
Copy Markdown
Contributor

I forgot to update the submodules, I'm sorry :c

But yes, with your branch it is now back to the old state of an error in wasmtime-obj, where it flakes out with an unsupported platform error, which is expected.

@abrown
abrown merged commit 6fd0451 into bytecodealliance:master Apr 9, 2020
@abrown
abrown deleted the fix-1471 branch April 9, 2020 16:45
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants