pallet-revive: Use custom target to build test fixtures #6266
Conversation
athei
requested review from
koute and
pgherveou
and removed request for
koute
|
can we get rid of the riscv features as well now that we have these changes? |
|
Yes we don't need the feature anymore. But this can be a follow up. Not sure why the prdoc bot doesn't work. |
| "data-layout": "e-m:e-p:32:32-i64:64-n32-S32", | ||
| "eh-frame-header": false, | ||
| "emit-debug-gdb-scripts": false, | ||
| "features": "+e,+m,+a,+c,+lui-addi-fusion,+fast-unaligned-access,+xtheadcondmov", |
There was a problem hiding this comment.
| "features": "+e,+m,+a,+c,+lui-addi-fusion,+fast-unaligned-access,+xtheadcondmov", | |
| "features": "+e,+m,+a,+c,+lui-addi-fusion,+unaligned-scalar-mem,+xtheadcondmov", |
There was a problem hiding this comment.
Why? I copied the file from here:
https://github.com/paritytech/polkavm/blob/master/guest-programs/riscv32emac-unknown-none-polkavm.json
There was a problem hiding this comment.
+fast-unaligned-access isn't a feature for the target and it won't have any effect. +fast-unaligned-access seems to be an older artifact.
There was a problem hiding this comment.
Ah okay it still called +fast-unaligned-access in LLVM 18.X. Rust 1.82.0 (latest stable) however uses LLVM 19:
rustc -vV
rustc 1.82.0 (f6e511eec 2024-10-15)
binary: rustc
commit-hash: f6e511eec7342f59a25f7c0534f1dbea00d01b14
commit-date: 2024-10-15
host: x86_64-unknown-linux-gnu
release: 1.82.0
LLVM version: 19.1.1
What toolchain do we expect?
There was a problem hiding this comment.
It uses whatever you build the repository with unless you override it. Our CI is on 1.81.
Co-authored-by: Cyrill Leutwiler <[email protected]>
|
/cmd prdoc --audience runtime_dev --bump major --clean |
|
Command "prdoc --audience runtime_dev --bump major --clean" has failed ❌! See logs here |
|
bot fmt |
|
@athei https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7662925 was started for your command Comment |
|
@athei Command |
|
So this will break for newer toolchains? Cause |
|
No, it'll just be a (suppressed) warning. You can set anything there, if it's not known to the target it's just ignored.
I mean unless of we'd rely on that (corresponding) feature always set which shouldn't be the case anyways. |
) This removes the need to use a custom toolchain to build the contract test fixtures. Instead, we supply a custom target and use the currently in use upstream toolchain. --------- Co-authored-by: Jan Bujak <[email protected]> Co-authored-by: Cyrill Leutwiler <[email protected]> Co-authored-by: command-bot <>
) This removes the need to use a custom toolchain to build the contract test fixtures. Instead, we supply a custom target and use the currently in use upstream toolchain. --------- Co-authored-by: Jan Bujak <[email protected]> Co-authored-by: Cyrill Leutwiler <[email protected]> Co-authored-by: command-bot <>
|
Resolved my conversation :-) Sorry for confusion was not meant as arguments. More like notes of my findings what can be done with just The link from Google's blog showed that what they are doing is just passing |
…tytech#6266)" This reverts commit db40a66.
Since #6266 we no longer require a custom toolchain to build the `pallet-revive-fixtures`. Hence we no longer have to guard the build behind a feature flag. --------- Co-authored-by: GitHub Action <[email protected]>
This removes the need to use a custom toolchain to build the contract test fixtures. Instead, we supply a custom target and use the currently in use upstream toolchain.