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

Add inline assembly to the reference #1105

Merged
merged 13 commits into from
Dec 31, 2021
Merged

Add inline assembly to the reference #1105

merged 13 commits into from
Dec 31, 2021

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Nov 9, 2021

This PR adds inline assembly to the reference, pending stabilization in rust-lang/rust#72016.

I have included all features currently supported by inline assembly, including the ones that are not being stabilized immediately (sym & const operands, support for additional architectures). This is intentional since it keeps all of the documentation about inline assembly in a single place.

@JohnTitor JohnTitor added the S-waiting-on-stabilization Waiting for a stabilization PR to be merged in the main Rust repository label Nov 9, 2021
- MIPS
- WebAssembly
- BPF
- SPIR-V
Copy link
Member

Choose a reason for hiding this comment

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

Maybe clarify that for many it is currently unstable?

- You cannot exit an `asm!` block that has not been entered. Neither can you exit an `asm!` block that has already been exited.
- You are responsible for switching any target-specific state (e.g. thread-local storage, stack bounds).
- The set of memory locations that you may access is the intersection of those allowed by the `asm!` blocks you entered and exited.
- You cannot assume that an `asm!` block will appear exactly once in the output binary. The compiler is allowed to instantiate multiple copies of the `asm!` block, for example when the function containing it is inlined in multiple places.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe explain that the implication of this is that only numbered labels are allowed and not named labels?

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thank you very much for preparing this!

I am very reluctant to add unstable documentation here. The reference very explicitly tries to avoid adding any unstable documentation, and I don't know why we would add an exception here. Would it be possible to relegate any unstable documentation to the unstable book? Or leave unstable parts commented out? I understand the desire to have some level of consolidation of documentation, but going by that logic, everything from the unstable book would end up here.

Can you add a newline after each sentence? That is the style used in the reference.

Would it be possible to sprinkle a few examples in? I think it is very helpful to have a few illustrative examples, and we try to do that throughout the reference.

I notice that the unstable book has more in-depth documentation. Is there a plan for how that documentation will live on?

@@ -0,0 +1,462 @@
# Inline assembly

Rust provides support for inline assembly via the `asm!` and `global_asm!` macros.
Copy link
Contributor

Choose a reason for hiding this comment

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

We generally try to avoid using "in Rust" style phrases, as almost everything in the reference is about Rust.

I think it would also be good to match these to the standard library docs.

Suggested change
Rust provides support for inline assembly via the `asm!` and `global_asm!` macros.
Support for inline assembly is provided via the [`asm!`] and [`global_asm!`] macros.
[`asm!`]: ../core/arch/macro.asm.html
[`global_asm!`]: ../core/arch/macro.global_asm.html

- BPF
- SPIR-V

Support for more targets may be added in the future. The compiler will emit an error if `asm!` is used on an unsupported target.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it goes without saying that the language can be extended in the future.

Suggested change
Support for more targets may be added in the future. The compiler will emit an error if `asm!` is used on an unsupported target.
The compiler will emit an error if `asm!` is used on an unsupported target.


Currently, all supported targets follow the assembly code syntax used by LLVM's internal assembler which usually corresponds to that of the GNU assembler (GAS). On x86, the `.intel_syntax noprefix` mode of GAS is used by default. On ARM, the `.syntax unified` mode is used. These targets impose an additional restriction on the assembly code: any assembler state (e.g. the current section which can be changed with `.section`) must be restored to its original value at the end of the asm string. Assembly code that does not conform to the GAS syntax will result in assembler-specific behavior.

[format-syntax]: https://doc.rust-lang.org/std/fmt/#syntax
Copy link
Contributor

Choose a reason for hiding this comment

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

The reference uses relative links to other books so that they work offline, and so that the link checker is able to validate them.

Suggested change
[format-syntax]: https://doc.rust-lang.org/std/fmt/#syntax
[format-syntax]: ../std/fmt/#syntax


An `asm!` invocation may have one or more template string arguments; an `asm!` with multiple template string arguments is treated as if all the strings were concatenated with a `\n` between them. The expected usage is for each template string argument to correspond to a line of assembly code. All template string arguments must appear before any other arguments.

As with format strings, named arguments must appear after positional arguments. Explicit register operands must appear at the end of the operand list, after named arguments if any.
Copy link
Contributor

Choose a reason for hiding this comment

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

This term is used before it is defined. It can help to link to the section where it is defined to provide some context.

Suggested change
As with format strings, named arguments must appear after positional arguments. Explicit register operands must appear at the end of the operand list, after named arguments if any.
As with format strings, named arguments must appear after positional arguments. Explicit [register operands](#register-operands) must appear at the end of the operand list, after named arguments if any.

>
> - Some register classes are marked as "Only clobbers" which means that they cannot be used for inputs or outputs, only clobbers of the form `out("reg") _` or `lateout("reg") _`.

Additional register classes may be added in the future based on demand (e.g. MMX, x87, etc).
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is necessary to mention what might happen in the future. If something is added in the future, it will get added to this list, and almost all changes to the language are based on demand and need.

Suggested change
Additional register classes may be added in the future based on demand (e.g. MMX, x87, etc).

@Amanieu
Copy link
Member Author

Amanieu commented Nov 16, 2021

I can move the unstable parts (unstable arch registers, sym & const) to the unstable book.

The guide-level explanation from the unstable book (which is from the RFC) will most likely go into Rust by example.

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 14, 2021
Stabilize asm! and global_asm!

Tracking issue: rust-lang#72016

It's been almost 2 years since the original [RFC](rust-lang/rfcs#2850) 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 rust-lang#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](rust-lang/reference#1105) and [rust by example](rust-lang/rust-by-example#1483).
  - 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`
@Amanieu
Copy link
Member Author

Amanieu commented Dec 16, 2021

asm! and global_asm! are now stable on the latest nightly, so the tests should pass.

flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Dec 17, 2021
Stabilize asm! and global_asm!

Tracking issue: #72016

It's been almost 2 years since the original [RFC](rust-lang/rfcs#2850) 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 #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](rust-lang/reference#1105) and [rust by example](rust-lang/rust-by-example#1483).
  - 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`
@Amanieu
Copy link
Member Author

Amanieu commented Dec 27, 2021

Ping! I'd like to get this merged before the beta branch in 2 weeks which include stable inline assembly.

@ehuss
Copy link
Contributor

ehuss commented Dec 28, 2021

Thanks for the updates!

If I'm understanding this correctly, this seems to introduce a new set of undefined behaviors to the language? Would it make sense to add an entry to the bullet list at https://github.com/rust-lang/reference/blob/master/src/behavior-considered-undefined.md that links to this chapter?

Otherwise, this looks good to me, though I can't vouch for the accuracy of all the information as I have no experience with it. I did at least scanned over the parser and it seems to match.

I don't know if anyone else would want to comment. Maybe @joshtriplett would be willing to chime in?

@Amanieu
Copy link
Member Author

Amanieu commented Dec 29, 2021

Thanks for the feedback! I've applied all the suggestions, so it should be good to go.

@joshtriplett
Copy link
Member

I remain saddened by the restrictions on assuming a single instance of an asm!, and I think people are highly likely to end up ignoring that and getting the results they expect with some careful use of #[inline(never)] and further care. I am not suggesting at this time that we know all the steps necessary to make that work reliably, but people will figure it out in practice when they need it, and I think we should be prepared to consider documenting those findings.

But I don't consider this a blocking concern, insofar as we can always loosen a restriction.

@joshtriplett
Copy link
Member

I think this is ready to merge.

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thank you!!

I'll update the books early next week before the branch.

@ehuss ehuss merged commit cdb6520 into rust-lang:master Dec 31, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 4, 2022
Update books

## reference

3 commits in 06f9e61931bcf58b91dfe6c924057e42ce273ee1..f8ba2f12df60ee19b96de24ae5b73af3de8a446b
2021-12-17 07:31:40 -0800 to 2022-01-03 11:02:08 -0800
- Switch the default edition for examples to 2021 (rust-lang/reference#1125)
- Clarify behavior of x87 FP registers in inline assembly (rust-lang/reference#1126)
- Add inline assembly to the reference (rust-lang/reference#1105)

## book

36 commits in 8a0bb3c96e71927b80fa2286d7a5a5f2547c6aa4..d3740fb7aad0ea4a80ae20f64dee3a8cfc0c5c3c
2021-12-22 20:54:27 -0500 to 2022-01-03 21:46:04 -0500
- Add a concrete example of an optional value. Fixes rust-lang/book#2848.
- match isn't really an operator. Fixes rust-lang/book#2859
- Edits to edits of chapter 6
- Make fixes recommended by shellcheck
- Use shellcheck
- SIGH fix all the typos that were missed while spellcheck was broken
- SIGH add all the words to the dictionary that were missed while spellcheck was broken
- Remove test_harness from the dictionary
- sigh, the xkcd sandwich one
- Install aspell in CI
- set -eu in all bash scripts
- typo: assignement -> assignment
- Fix quotes
- Snapshot of ch12 for nostarch
- Use 'static lifetime earlier because that's more correct. Fixes rust-lang/book#2864.
- Add does_not_compile annotation to intermediate steps that don't compile
- Sidestep who provides output streams. Fixes rust-lang/book#2933.
- Remove note about primitive obsession. Fixes rust-lang/book#2863.
- Remove sentence encouraging writing tests on your own. Fixes rust-lang/book#2223.
- Bump mdBook version to 0.4.14 in workflow main.yml
- Past tense make better sense
- Past tense makes better sense
- Update the edition in all the listings' Cargo.toml
- Update the book to either say 2021 edition or not talk about editions
- Remove most of the 2018 edition text from the title page. Fixes rust-lang/book#2852.
- Fix word wrapping
- Emphasize return type is mandatory
- fix title capitalization
- Further edits to mention of --include-ignored, propagate to src
- feat: mention `cargo test -- --include-ignored`
- wording: get rid of "to from"
- interchanged position of `binary` and `library`
- Fix wrong word typo
- Further edits in rust-analyzer text
- appendix-04 IDE integration: Replaced rls by rust-analyzer
- Update link to Italian translation. Connects to rust-lang/book#2484.

## rustc-dev-guide

3 commits in 9bf0028b557798ddd07a6f652e4d0c635d3d6620..875464457c4104686faf667f47848aa7b0f0a744
2021-12-20 21:53:57 +0900 to 2021-12-28 22:17:49 -0600
- Update link to moved section (rust-lang/rustc-dev-guide#1282)
- Fix link in contributing.md (rust-lang/rustc-dev-guide#1280)
- Streamline "Getting Started" (rust-lang/rustc-dev-guide#1279)
Comment on lines +425 to +429
- The set of memory locations that assembly code is allowed to read and write are the same as those allowed for an FFI function.
- Refer to the unsafe code guidelines for the exact rules.
- If the `readonly` option is set, then only memory reads are allowed.
- If the `nomem` option is set then no reads or writes to memory are allowed.
- These rules do not apply to memory which is private to the asm code, such as stack space allocated within the asm block.
Copy link

Choose a reason for hiding this comment

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

Do the unsafe code guidelines actually have any information about this? I can't find anything in the published reference and I also can't find anything in the unsafe code guidelines' github issues. While not mentioned here, I also couldn't find anything in the Rustonomicon. Did I miss something or is there some other place that has information about this?

Besides that, isn't the whole point of the unsafe code guidelines to be a place for discussion before being moved into the proper reference? It seems a bit weird to have the official reference point to an unofficial reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-stabilization Waiting for a stabilization PR to be merged in the main Rust repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants