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.

Sign up for GitHub

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

Jump to bottom

Update stdarch submodule (to before it switched to const generics) #83776

Merged
merged 1 commit into from
Apr 12, 2021
Merged

Update stdarch submodule (to before it switched to const generics) #83776

merged 1 commit into from
Apr 12, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Apr 2, 2021 •
edited
Loading

#83278 (comment): This unblocks #82539.

Major changes:

  • More AVX-512 intrinsics.
  • More ARM & AArch64 NEON intrinsics.
  • Updated unstable WASM intrinsics to latest draft standards.
  • std_detect is now a separate crate instead of a submodule of std.

I double-checked and the first use of const generics looks like rust-lang/stdarch@8d50178, which isn't included in this PR.

r? @Amanieu

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • These commits modify submodules.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jyn514
Copy link
Member Author

jyn514 commented Apr 2, 2021

@Amanieu do you recognize the build failure? I don't see any changes to rustc-dep-of-std even looking at the full git history for stdarch :/ all the other changes are directly from #83278.

@Amanieu
Copy link
Member

Amanieu commented Apr 2, 2021

I think you'll need to cherry-pick rust-lang/stdarch#1089 which was added after the port to #[rustc_legacy_const_generics] started.

If you create a branch with that cherry pick then I can push it to rust-lang/stdarch so that you can use it for the submodule.

@jyn514
Copy link
Member Author

jyn514 commented Apr 2, 2021

Thanks! https://github.com/jyn514/stdarch/tree/doc-include-fixed has the cherry-picked fix.

@Amanieu
Copy link
Member

Amanieu commented Apr 2, 2021

Pushed to https://github.com/rust-lang/stdarch/tree/rustc-2021-04-02

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jyn514
Copy link
Member Author

jyn514 commented Apr 4, 2021 •
edited
Loading

It looks like this error is also present in the other PR: #83278 (comment). I think this may actually be a breaking change and libstd needs to re-export the macro in the crate root? I'll try that.

@jyn514
Copy link
Member Author

jyn514 commented Apr 4, 2021

Ok, I think that should have worked.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jyn514
Copy link
Member Author

jyn514 commented Apr 4, 2021

Oh, I guess all the macros need the same treatment. @Amanieu is there an easy way to see all macros that are declared by stdarch? Is searching for @MACRO_NAME enough?

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jyn514
Copy link
Member Author

jyn514 commented Apr 4, 2021 •
edited
Loading

Apparently there are two different tracking issues for simd (??) #48556 #27731 https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/Why.20are.20there.20two.20tracking.20issues.20for.20SIMD.3F

@Amanieu
Copy link
Member

Amanieu commented Apr 5, 2021

I'm not sure what's up with the two separate tracking issues, but this can be resolved separately.

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Apr 5, 2021

📌 Commit f297a4072cba08bab1f3fcf589247291fff79add has been approved by Amanieu

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 5, 2021
@bors
Copy link
Contributor

bors commented Apr 5, 2021

⌛ Testing commit f297a4072cba08bab1f3fcf589247291fff79add with merge 59e207563090269860362bc18eea72adbe563a7e...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Amanieu
Copy link
Member

Amanieu commented Apr 5, 2021

@bors retry

@jyn514
Copy link
Member Author

jyn514 commented Apr 5, 2021

So fixing the error is easy enough, but I'm having trouble replicating it to make sure the fix is right. It looks like os::linux is only compiled when there's a non-x86 target? I tried cargo check --target aarch64-unknown-linux-gnu -p std_detect --features libc,std_detect_file_io but it didn't show any warnings.

@Amanieu
Copy link
Member

Amanieu commented Apr 5, 2021

The elided_lifetimes_in_paths lint is allow by default.

@Amanieu
Copy link
Member

Amanieu commented Apr 5, 2021

It's part of the rust_2018_idioms lint group, which really should be enabled by default at this point...

@jyn514 jyn514 mentioned this pull request Apr 5, 2021
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 5, 2021
@Dylan-DPC
Rollup merge of rust-lang#83878 - the8472:fix-concurrent-tidy-access,…
d856a26 
… r=Mark-Simulacrum

Fix racing file access in tidy

That should fix the failure in rust-lang#83776 (comment)

The file is only created for a brief moment during the bins checks in the source directories while other checks may also be visiting the same directory. By skipping it we avoid file not found errors.
@jyn514 jyn514 added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 6, 2021
@Amanieu
Copy link
Member

Amanieu commented Apr 12, 2021

I've updated the rustc-2021-04-02 branch on stdarch with rust-lang/stdarch#1108.

@jyn514
Update stdarch submodule (to before it switched to const generics)
1b0b7e9 
This also includes a cherry-pick of
rust-lang/stdarch@ec14619
and rust-lang/stdarch#1108 to fix a build
failure.

It also adds a re-export of various macros to the crate root of libstd -
previously they would show up automatically because std_detect was defined
in the same crate.
@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Apr 12, 2021
@Amanieu
Copy link
Member

Amanieu commented Apr 12, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Apr 12, 2021

📌 Commit 1b0b7e9 has been approved by Amanieu

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 12, 2021
@bors
Copy link
Contributor

bors commented Apr 12, 2021

⌛ Testing commit 1b0b7e9 with merge d0695c9...

@bors
Copy link
Contributor

bors commented Apr 12, 2021

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing d0695c9 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 12, 2021
@bors bors merged commit d0695c9 into rust-lang:master Apr 12, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 12, 2021
@jyn514 jyn514 deleted the update-stdarch-docs branch April 12, 2021 20:59
@bors bors mentioned this pull request Apr 12, 2021
Closed
@bluss
Copy link
Member

bluss commented Apr 12, 2021

This should also fix some arch-specific detect bugs

@Mark-Simulacrum
Copy link
Member

FWIW, this was a 1.4-1.5% memory usage regression on check, debug, and opt builds. I don't know that we necessarily want to do anything here - this may be entirely mitigated by the const generics rewrites. But thought I'd flag it.

@Amanieu
Copy link
Member

Amanieu commented Apr 13, 2021

Could this be because we are importing std_detect as a separate crate instead of having it as part of std?

@the8472 the8472 mentioned this pull request Apr 14, 2021
Merged
@jyn514 jyn514 mentioned this pull request Apr 16, 2021
Merged
@ehuss ehuss mentioned this pull request Apr 23, 2021
Merged
bors added a commit to rust-lang/cargo that referenced this pull request Apr 23, 2021
@bors
Auto merge of #9393 - ehuss:build-std-updating, r=alexcrichton
02103f9 
Fix build-std updating the index on every build.

rust-lang/rust#83776 has caused a problem where build-std will update the index on every build. That PR added `std_detect` from the `stdarch` submodule as a path dependency of `std`. However, since `stdarch` has a workspace of its own, an exclusion had to be added to `Cargo.toml` so that it does not treat `std_detect` as a workspace member (because nested workspaces are not supported).

The problem is that the std `Cargo.lock` file is built thinking that `std_detect` is *not* a workspace member. This means that its dev-dependencies are not included. However, when cargo resolves the std workspace, it doesn't know that `std_detect` should be excluded, so it considers it a workspace member (because it is a path dependency). This means that it expects the dev-dependencies to be in Cargo.lock. Because they are missing, it ends up poisoning the registry and triggering an update:

> poisoning registry `https://github.com/rust-lang/crates.io-index` because std_detect v0.1.5 (/Users/eric/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/stdarch/crates/std_detect) looks like it changed auxv

The solution here is to skip dev-dependencies if they are not actively being resolved, even if the package is a workspace member.

This has happened before (#8962), so I have updated the test to check for it.

There are some alternative solutions I considered:

* Add support for nested workspaces. 😄
* Use a symlink to `std_detect` in the `rust-lang/rust` repository so that it appears to cargo as-if it is "outside" of the stdarch workspace, and thus can be treated like a normal workspace member (and remove the "exclude"). That seems a little hacky.

Fixes #9390
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Amanieu Amanieu

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.53.0
Development

Successfully merging this pull request may close these issues.
9 participants
@jyn514 @rust-highfive @rust-log-analyzer @Amanieu @bors @the8472 @bluss @Mark-Simulacrum @rustbot