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

wasm-builder: Also set mcpu for c deps #3777

Merged
merged 6 commits into from
Jul 18, 2024
Merged

wasm-builder: Also set mcpu for c deps #3777

merged 6 commits into from
Jul 18, 2024

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Mar 21, 2024

Closes: #3192

@bkchr bkchr added the R0-silent Changes should not be mentioned in any release notes label Mar 21, 2024
@michalkucharczyk michalkucharczyk requested a review from a team March 21, 2024 10:04
Copy link
Contributor

@koute koute left a comment

Choose a reason for hiding this comment

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

Note that this could in theory break builds if someone has non-Rust dependencies in their build deps, in which case it would use this flag to compile them too, and since mvp is only valid for WASM it might break them**.

I don't know of a better way of doing this though.

** - interestingly, if I pass -mcpu=mvp to clang I get this and it compiles just fine (it completely ignores the flags, which, AFAIK, it shouldn't?):

$ clang -mcpu=mvp --target=x86_64-unknown-linux-gnu hello.c 
clang-16: warning: argument unused during compilation: '-mcpu=mvp' [-Wunused-command-line-argument]

but if I change the target to something else then I get this:

$ clang -mcpu=mvp --target=aarch64-unknown-linux-gnu hello.c 
clang-16: error: unsupported argument 'mvp' to option '-mcpu='

...which is a little strange that it's specifically hardcoded to ignore this only on AMD64.

@bkchr bkchr marked this pull request as ready for review March 22, 2024 22:55
@bkchr bkchr enabled auto-merge July 18, 2024 08:16
@bkchr bkchr added this pull request to the merge queue Jul 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 18, 2024
@bkchr bkchr added this pull request to the merge queue Jul 18, 2024
Merged via the queue into master with commit 35fcac7 Jul 18, 2024
158 of 159 checks passed
@bkchr bkchr deleted the bkchr-wasm-builder-mvp branch July 18, 2024 13:32
dnjscksdn98 added a commit to bifrost-platform/polkadot-sdk that referenced this pull request Jul 22, 2024
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
dnjscksdn98 added a commit to bifrost-platform/polkadot-sdk that referenced this pull request Sep 20, 2024
@nazar-pc nazar-pc mentioned this pull request Sep 30, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UnknownOpcode(192) again
6 participants