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

x64: only enable VTune dependencies on x86_64 targets #4533

Merged
merged 1 commit into from
Jul 28, 2022

Conversation

abrown
Copy link
Contributor

@abrown abrown commented Jul 26, 2022

As described in #4523, the ittapi dependency necessary for Wasmtime's
VTune support does not compile on aarch64-linux-android. There are
several incompatible parts here: though ittapi supports all OS
combinations that Wasmtime does and builds on all CPU targets, VTune is
not primarily intended for aarch64 profiling and linux-android is not
a high priority platform for the library. We could conditionally depend
on ittapi for Wasmtime's supported OS combinations, but I think a
better answer is to limit it to x86_64 targets, since this more clearly
shows why the ittapi/VTune support is present and also fixes #4523.

As described in bytecodealliance#4523, the `ittapi` dependency necessary for Wasmtime's
VTune support does not compile on `aarch64-linux-android`. There are
several incompatible parts here: though `ittapi` supports all OS
combinations that Wasmtime does and builds on all CPU targets, VTune is
not primarily intended for aarch64 profiling and `linux-android` is not
a high priority platform for the library. We could conditionally depend
on `ittapi` for Wasmtime's supported OS combinations, but I think a
better answer is to limit it to x86_64 targets, since this more clearly
shows why the `ittapi`/VTune support is present and also fixes bytecodealliance#4523.
@abrown abrown force-pushed the conditional-vtune branch from 5a928f2 to 3a39834 Compare July 26, 2022 18:20
@alexcrichton
Copy link
Member

Could you double-check that this fixes at least the issue in #4523 (although I suspect more exist)

@abrown
Copy link
Contributor Author

abrown commented Jul 26, 2022

Could you double-check that this fixes at least the issue in #4523 (although I suspect more exist)

Like compile with cargo build --target aarch64-linux-android or something else?

@abrown
Copy link
Contributor Author

abrown commented Jul 26, 2022

Looks like something else is the problem now:

$ cargo build --release --target aarch64-linux-android
...

error: failed to run custom build command for `psm v0.1.18`

Caused by:
  process didn't exit successfully: `/home/abrown/Code/wasmtime/target/release/build/psm-d3851756418374a6/build-script-build` (exit status: 1)
  --- stdout
  OPT_LEVEL = Some("3")
  TARGET = Some("aarch64-linux-android")
  HOST = Some("x86_64-unknown-linux-gnu")
  CC_aarch64-linux-android = None
  CC_aarch64_linux_android = None
  TARGET_CC = None
  CC = None
  CFLAGS_aarch64-linux-android = None
  CFLAGS_aarch64_linux_android = None
  TARGET_CFLAGS = None
  CFLAGS = None
  CRATE_CC_NO_DEFAULTS = None
  DEBUG = Some("false")
  cargo:rustc-cfg=asm
  cargo:rustc-cfg=switchable_stack
  running: "aarch64-linux-android-clang" "-O3" "-DANDROID" "-ffunction-sections" "-fdata-sections" "-fPIC" "-Wall" "-Wextra" "-xassembler-with-cpp" "-DCFG_TARGET_OS_android" "-DCFG_TARGET_ARCH_aarch64" "-DCFG_TARGET_ENV_" "-o" "/home/abrown/Code/wasmtime/target/aarch64-linux-android/release/build/psm-975239bbdf5842b2/out/src/arch/aarch_aapcs64.o" "-c" "src/arch/aarch_aapcs64.s"

  --- stderr


  error occurred: Failed to find tool. Is `aarch64-linux-android-clang` installed?

@abrown abrown marked this pull request as ready for review July 26, 2022 20:07
@abrown abrown requested a review from alexcrichton July 26, 2022 20:40
@alexcrichton
Copy link
Member

Could you perhaps set AR=true CC=true and see how far the build goes? Given the error you got it's unlikely you even reached compiling ittapi-rs

@abrown
Copy link
Contributor Author

abrown commented Jul 27, 2022

Could you perhaps set AR=true CC=true and see how far the build goes? Given the error you got it's unlikely you even reached compiling ittapi-rs

$ AR=true CC=true cargo build --release --target aarch64-linux-android
...
error: could not find native static library `psm_s`, perhaps an -L flag is missing?

error: could not compile `psm` due to previous error

And, yeah, I do not see ittapi being built in the compiled crates up above that error.

@abrown
Copy link
Contributor Author

abrown commented Jul 27, 2022

@gneworld, could you test out this branch to see if it resolves #4523 for you? I don't think I have the right system environment (toolchains, libraries, etc.) to check if indeed this commit will fix the build issue you found.

@gneworld
Copy link

gneworld commented Jul 28, 2022

@gneworld, could you test out this branch to see if it resolves #4523 for you? I don't think I have the right system environment (toolchains, libraries, etc.) to check if indeed this commit will fix the build issue you found.

@abrown unfortunately,
Compiling wasmtime-jit v0.40.0
error[E0433]: failed to resolve: use of undeclared crate or module rustc_demangle
--> crates/jit/src/demangling.rs:7:28
|
7 | if let Ok(demangled) = rustc_demangle::try_demangle(name) {
| ^^^^^^^^^^^^^^ use of undeclared crate or module rustc_demangle

error[E0433]: failed to resolve: use of undeclared crate or module bincode
--> crates/jit/src/instantiate.rs:270:5
|
270 | bincode::serialize_into(&mut bytes, &info)?;
| ^^^^^^^ use of undeclared crate or module bincode

error[E0433]: failed to resolve: use of undeclared crate or module bincode
--> crates/jit/src/instantiate.rs:421:21
|
421 | None => bincode::deserialize(section(ELF_WASMTIME_INFO)?)
| ^^^^^^^ use of undeclared crate or module bincode

error[E0433]: failed to resolve: use of undeclared crate or module cpp_demangle
--> crates/jit/src/demangling.rs:9:49
|
9 | } else if let Ok(demangled) = cpp_demangle::Symbol::new(name) {
| ^^^^^^ not found in cpp_demangle
|
help: consider importing one of these items
|
3 | use object::Symbol;
|
3 | use object::write::Symbol;
|
help: if you import Symbol, refer to it directly
|
9 - } else if let Ok(demangled) = cpp_demangle::Symbol::new(name) {
9 + } else if let Ok(demangled) = Symbol::new(name) {
|

For more information about this error, try rustc --explain E0433.
error: could not compile wasmtime-jit due to 4 previous errors

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Thanks for testing @gneworld. As mentioned aarch64-linux-android is not yet a supported platform. PRs are welcome to help shore up platform support but otherwise there's likely to be compile errors like that

Otherwise though it looks like compilation got past ittapi-rs so I'm going to approve this for merge since it fixes at least this issue.

@alexcrichton alexcrichton merged commit 8137432 into bytecodealliance:main Jul 28, 2022
@abrown abrown deleted the conditional-vtune branch July 28, 2022 15:52
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.

Failed to build for aarch64-linux-android
3 participants