Skip to content

wasm-builder: Use riscv32emac-unknown-none-polkavm.json target#6419

Merged
jarkkojs merged 41 commits intoparitytech:masterfrom
jarkkojs:wasm-builder-riscv32emac
Dec 4, 2024
Merged

wasm-builder: Use riscv32emac-unknown-none-polkavm.json target#6419
jarkkojs merged 41 commits intoparitytech:masterfrom
jarkkojs:wasm-builder-riscv32emac

Conversation

@jarkkojs
Copy link
Copy Markdown
Contributor

@jarkkojs jarkkojs commented Nov 9, 2024

Description

Closes #6335.

Integration

N/A

Review Notes

RuntimeTarget is converted to return path to the custom target JSON file

@jarkkojs jarkkojs requested a review from koute as a code owner November 9, 2024 01:34
@jarkkojs jarkkojs requested a review from athei November 9, 2024 01:36
@jarkkojs
Copy link
Copy Markdown
Contributor Author

jarkkojs commented Nov 9, 2024

How do I pass check-labels?

@koute
Copy link
Copy Markdown
Contributor

koute commented Nov 9, 2024

How do I pass check-labels?

If you click on the job it tells you why it fails:

You need to include some of the T* label(s)

AFAIK, here is the repo with the rules, and here are some docs.

@koute koute added the T0-node This PR/Issue is related to the topic “node”. label Nov 9, 2024
@jarkkojs
Copy link
Copy Markdown
Contributor Author

After change max-atomic-width to 64:

  thread 'main' panicked at substrate/utils/wasm-builder/src/wasm_project.rs:946:21:
  internal error: couldn't read the metadata of "/home/jarkko/work/github.com/jarkkojs/polkadot-sdk/substrate/utils/wasm-builder/riscv32emac-unknown-none-polkavm.json/release/kitchensink-runtime-blob": Not a directory (os error 20)
  stack backtrace:
     0: rust_begin_unwind
     1: core::panicking::panic_fmt
     2: substrate_wasm_builder::wasm_project::build_bloaty_blob
     3: substrate_wasm_builder::wasm_project::create_and_compile
     4: substrate_wasm_builder::builder::build_project
     5: substrate_wasm_builder::builder::WasmBuilder::build
     6: substrate_wasm_builder::builder::WasmBuilder::build_using_defaults
     7: build_script_build::main
     8: core::ops::function::FnOnce::call_once
  note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
( RUST_BACKTRACE=1 SUBSTRATE_RUNTIME_TARGET=riscv SUBSTRATE_ENABLE_POLKAVM=1 )  1341.20s user 124.56s system 570% cpu 4:16.75 total

@athei
Copy link
Copy Markdown
Member

athei commented Nov 12, 2024

So the issue here is compiling PolkaVM itself to risc32emac? Since the kitchensink runtime uses PolkaVM to run contracts?

@jarkkojs
Copy link
Copy Markdown
Contributor Author

So the issue here is compiling PolkaVM itself to risc32emac? Since the kitchensink runtime uses PolkaVM to run contracts?

I th

How do I pass check-labels?

If you click on the job it tells you why it fails:

You need to include some of the T* label(s)

AFAIK, here is the repo with the rules, and here are some docs.

Aah so labels are labels in Github? OK cool I thought I has something missing in the commit itself.

@athei
Copy link
Copy Markdown
Member

athei commented Nov 12, 2024

Yes it checks that you have all the required github labels

@jarkkojs
Copy link
Copy Markdown
Contributor Author

jarkkojs commented Nov 12, 2024

@koute looking pretty good but i don't know what the remaining error messages even mean :-)

Summary of remaining issues:

  • continuous-integration/gitlab-zombienet: all of the three simply timeout
  • Build Misc / build-runtimes-polkavm: Failed to write metrics: promwrite: sending remote write request: Post "placeholder": unsupported protocol scheme ""
  • Build Misc / All build misc jobs passed: Failed to write metrics: promwrite: sending remote write request: Post "placeholder": unsupported protocol scheme ""

Signed-off-by: Jarkko Sakkinen <jarkko@parity.io>
@athei
Copy link
Copy Markdown
Member

athei commented Nov 13, 2024

They seem all to be unrelated to your changes. The CI is notoriously flaky. Merging master and praying is what you do in these cases.

jarkkojs and others added 3 commits November 14, 2024 08:34
Co-authored-by: Koute <koute@users.noreply.github.com>
Signed-off-by: Jarkko Sakkinen <jarkko@parity.io>
Signed-off-by: Jarkko Sakkinen <jarkko@parity.io>
@jarkkojs jarkkojs requested review from athei and koute November 14, 2024 09:57
Signed-off-by: Jarkko Sakkinen <jarkko@parity.io>
@athei
Copy link
Copy Markdown
Member

athei commented Nov 14, 2024

I am confused that this even works. The polkavm-linker and polkavm-derive dependencies is still set to 0.9.3. Shouldn't it be bumped in this PR, too?

polkadot-sdk/Cargo.toml

Lines 1093 to 1094 in a1af8ed

polkavm-derive = "0.9.1"
polkavm-linker = "0.9.2"

@jarkkojs
Copy link
Copy Markdown
Contributor Author

I am confused that this even works. The polkavm-linker and polkavm-derive dependencies is still set to 0.9.3. Shouldn't it be bumped in this PR, too?

polkadot-sdk/Cargo.toml

Lines 1093 to 1094 in a1af8ed

polkavm-derive = "0.9.1"
polkavm-linker = "0.9.2"

I have no idea, @koute ?

@koute
Copy link
Copy Markdown
Contributor

koute commented Nov 14, 2024

https://github.com/paritytech/polkadot-sdk/actions/runs/11835548683/job/32978647159?pr=6419#step:5:988

Looks like the build runtime tests are failing, so yes, the derive and the linker crates should also be bumped.

@jarkkojs
Copy link
Copy Markdown
Contributor Author

jarkkojs commented Nov 14, 2024

https://github.com/paritytech/polkadot-sdk/actions/runs/11835548683/job/32978647159?pr=6419#step:5:988

Looks like the build runtime tests are failing, so yes, the derive and the linker crates should also be bumped.

@koute, @athei

I posted this on Tuesday:

#6419 (comment)

I.e. still not knowing what to do (day three).

@athei
Copy link
Copy Markdown
Member

athei commented Nov 14, 2024

Answered here: #6419 (comment)

@jarkkojs jarkkojs enabled auto-merge November 14, 2024 17:13
@jarkkojs jarkkojs disabled auto-merge November 14, 2024 17:14
@jarkkojs jarkkojs enabled auto-merge November 14, 2024 17:14
@jarkkojs jarkkojs disabled auto-merge December 3, 2024 15:21
@jarkkojs jarkkojs enabled auto-merge December 3, 2024 15:21
@jarkkojs jarkkojs added this pull request to the merge queue Dec 4, 2024
Merged via the queue into paritytech:master with commit 82117ad Dec 4, 2024
@jarkkojs jarkkojs deleted the wasm-builder-riscv32emac branch December 4, 2024 18:49
sylvaincormier pushed a commit to sylvaincormier/polkadot-sdk that referenced this pull request Dec 4, 2024
…ytech#6419)

# Description

Closes paritytech#6335.

## Integration

N/A

## Review Notes

`RuntimeTarget` is converted to return path to the custom target JSON
file

---------

Signed-off-by: Jarkko Sakkinen <jarkko@parity.io>
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Co-authored-by: Koute <koute@users.noreply.github.com>
Krayt78 pushed a commit to Krayt78/polkadot-sdk that referenced this pull request Dec 18, 2024
…ytech#6419)

# Description

Closes paritytech#6335.

## Integration

N/A

## Review Notes

`RuntimeTarget` is converted to return path to the custom target JSON
file

---------

Signed-off-by: Jarkko Sakkinen <jarkko@parity.io>
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Co-authored-by: Koute <koute@users.noreply.github.com>
// This is a nightly-only flag.
let arg = match self {
RuntimeTarget::Wasm => "build-std",
RuntimeTarget::Riscv => "build-std=core,alloc",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@bkchr @jarkkojs What is the point of doing -Zbuild-std for target riscv32emac-unknown-none-polkavm? It increases runtime build time for non-wasm targets.

Copy link
Copy Markdown
Member

@athei athei Dec 26, 2024

Choose a reason for hiding this comment

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

It is required for both targets but for different reasons:

wasm: The rustup provided standard library is compiled with wasm extensions enabled we dont support. Hence we need to recompile it.

riscv: Rustup doesn't contain any prebuilt binaries for this target. Hence we need to build it ourselves.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, so I guessed, i.e. it should be official target in rustc upstream to have prebuilt binaries, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[not actually at work yet, my keyboard just writes by itself]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@athei Thanks for the clarification! I think Parity could add to the pre-compiled libraries to riscv32emac-unknown-none-polkavm target in rlib format like the wasm32-unknown-unknown target does (located in ./lib/rustlib/wasm32-unknown-unknown/lib/libcore-192e883b194683b6.rlib).

dudo50 pushed a commit to paraspell-research/polkadot-sdk that referenced this pull request Jan 4, 2025
…ytech#6419)

# Description

Closes paritytech#6335.

## Integration

N/A

## Review Notes

`RuntimeTarget` is converted to return path to the custom target JSON
file

---------

Signed-off-by: Jarkko Sakkinen <jarkko@parity.io>
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Co-authored-by: Koute <koute@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T0-node This PR/Issue is related to the topic “node”.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Update polkavm to the upstream toolchain version

5 participants