Skip to content

Stabilize asm! and global_asm!#91728

Merged
bors merged 9 commits intorust-lang:masterfrom
Amanieu:stable_asm
Dec 15, 2021
Merged

Stabilize asm! and global_asm!#91728
bors merged 9 commits intorust-lang:masterfrom
Amanieu:stable_asm

Conversation

@Amanieu
Copy link
Member

@Amanieu Amanieu commented Dec 10, 2021

Tracking issue: #72016

It's been almost 2 years since the original RFC was posted and we're finally ready to stabilize this feature!

The main changes in this PR are:

  • Removing asm! and global_asm! from the prelude as per the decision in Decide whether asm! and/or global_asm! should be exported from the prelude.  #87228.
  • Stabilizing the asm and global_asm features.
  • Removing the unstable book pages for asm and global_asm. The contents are moved to the reference and rust by example.
    • All links to these pages have been removed to satisfy the link checker. In a later PR these will be replaced with links to the reference or rust by example.
  • Removing the automatic suggestion for using llvm_asm! instead of asm! if you're still using the old syntax, since it doesn't work anymore with asm! no longer being in the prelude. This only affects code that predates the old LLVM-style asm! being renamed to llvm_asm!.
  • Updating stdarch and compiler-builtins.
  • Updating all the tests.

r? @joshtriplett

@rust-highfive
Copy link
Contributor

Some changes occured to rustc_codegen_gcc

cc @antoyo

@rust-highfive
Copy link
Contributor

⚠️ 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 Dec 10, 2021
Cargo.lock Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

In case anyone's wondering: yes, I made a typo when publishing a new version of compiler-builtins.

@nbdd0121
Copy link
Member

Would it make sense to leave the macro in the prelude unstably for easier migration?

@camelid camelid added needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. T-lang Relevant to the language team labels Dec 10, 2021
@camelid

This comment has been minimized.

@nbdd0121

This comment has been minimized.

@camelid

This comment has been minimized.

@camelid camelid added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. labels Dec 10, 2021
@joshtriplett
Copy link
Member

I just posted a set of review comments; with all of those addressed, r=me.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Dec 11, 2021

☔ The latest upstream changes (presumably #91799) made this pull request unmergeable. Please resolve the merge conflicts.

@Amanieu
Copy link
Member Author

Amanieu commented Dec 11, 2021

@bors r=joshtriplett

@bors
Copy link
Collaborator

bors commented Dec 11, 2021

📌 Commit 9af1f7d550300b5459fa6c4b8d6213f3003e15cf has been approved by joshtriplett

@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 Dec 11, 2021
@bors
Copy link
Collaborator

bors commented Dec 12, 2021

☔ The latest upstream changes (presumably #91813) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 12, 2021
yvt added a commit to r3-os/rust-riscv-rt that referenced this pull request Dec 19, 2021
`asm!` was removed from the prelude when it was stabilized.
<rust-lang/rust#91728>
yvt added a commit to r3-os/r3 that referenced this pull request Dec 19, 2021
The `asm` feature has been stabilized by
<rust-lang/rust#91728>, which also removes
`[global_]asm!` from the prelude.
yvt added a commit to r3-os/r3 that referenced this pull request Dec 19, 2021
The `asm` feature has been stabilized by
<rust-lang/rust#91728>, which also removed
`[global_]asm!` from the prelude.
bors bot added a commit to rust-embedded/cortex-m that referenced this pull request Dec 19, 2021
372: asm/inline: explicitly use asm macro r=adamgreig a=jordens

`asm!()` removed from prelude in current nightly
rust-lang/rust#91728

close #371

This is also a good candidate for the cortex-m v0.7 series.

Co-authored-by: Robert Jördens <rj@quartiq.de>
Co-authored-by: Adam Greig <adam@adamgreig.com>
linkmauve added a commit to linkmauve/luma that referenced this pull request Dec 19, 2021
These features are now stable, as of the 19th of December! See
rust-lang/rust#91728
Undin added a commit to intellij-rust/intellij-rust that referenced this pull request Dec 20, 2021
`asm` macro was removed from stdlib prelude in 1.59
See rust-lang/rust#91728
@rylev
Copy link
Member

rylev commented Dec 21, 2021

This seems likely to be noise as a result of the issue tracked in rust-lang/rustc-perf#1126 -- marking as triaged, not something we should address directly.

@rustbot label +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Dec 21, 2021
@bradjc bradjc mentioned this pull request Feb 9, 2022
2 tasks
@FCLC
Copy link

FCLC commented Feb 26, 2022

[x86_64, AVX512]
I might be missing something in the docs, but AFAICT as of now the only supported avx512 instruction sets are the foundation set (avx512F) and the Byte and Word set (avx512BW), the implication being that the V(ector) L(ength) and D(ual) Q(uad) sets are not currently supported.

Reason I bring this up is mainly to do with the exclusion of AVX512VL. The VL set extends the foundation set (and most other sets) to be able to operate on both XMM and YMM registers, allowing for much cleaner implementations as well as faster overall code.

The reason for wanting this functionality is due the lower latency of certain AVX and AVX2 instructions that can only operate on xmm registers, but wanting the more modern and advanced instructions available under AVX512.

For example, as of now if we wanted to do a fuse multiply add on a group of 4 packed singles without avx512VL we'd have to do something along the lines of

xor/zero out all your registers
vmov zmm1, {mem pointer to 4 PS}
vfmadd132 zmm1, zmm1, zmm1; do a fuse multiply add on each of the 4 packed singles on themselves
vextractf32x4 xmm1, zmm1, 0
whatever you need to do in xmm registers
vinsertf32x4 zmm1, zmm1, xmm1, 0 ; we're back in ZMM registers for any avx512 instruction work we need to do

The VL set of extensions allows us to use AVX512 instructions directly on xmm and ymm registers, removing the need for copying back and forth between AVX1/2 registers and AVX512 registers.

This is also relevant not only because of the latency cost, but also because they may find themselves on different ports of the CPU core itself, slowing us down further.

The above would become:

vmov xmm1, {mem pointer to 4 PS}
vfmadd132 xmm1, xmm1, xmm1; do a fuse multiply add on each of the 4 packed singles on themselves
{other SIMD instructions from AVX1/2 512 or SSE all operating on xmm without extra copies}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.