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 reference for asm-goto #1693

Merged
merged 7 commits into from
Mar 18, 2025
Merged

Add reference for asm-goto #1693

merged 7 commits into from
Mar 18, 2025

Conversation

nbdd0121
Copy link
Contributor

@nbdd0121 nbdd0121 commented Dec 4, 2024

The asm-goto-with-outputs is still unstable, so in the reference it's still mentioned as forbidden by a check rule.

Stabilization PR: rust-lang/rust#133870

@ehuss ehuss added S-waiting-on-stabilization Waiting for a stabilization PR to be merged in the main Rust repository S-waiting-on-review Status: The marked PR is awaiting review from a maintainer labels Dec 5, 2024
@traviscross
Copy link
Contributor

@Amanieu: What do you think?

@ehuss
Copy link
Contributor

ehuss commented Dec 12, 2024

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: The marked PR is awaiting review from a maintainer labels Dec 12, 2024
@traviscross
Copy link
Contributor

We reviewed this in the lang call today. Everyone read through it. It looked right. Nobody had any notes.

@traviscross
Copy link
Contributor

In reading this more carefully (especially the lines not changed), I find myself wanting to double check that we're being precise enough now by what we mean by "the asm! block". Previously, when speaking about what happened during execution of "the asm! block", that meant only the execution of the assembly code. But now, given that the label blocks are lexically within the asm! block and may be executed during the execution of the asm! block, it would be reasonable to read "the asm! block" as to include that code also.

So when we say, e.g., that...

  • pure: The asm! block has no side effects, must eventually return, and its outputs depend only on its direct inputs (i.e. the values themselves, not what they point to) or values read from memory (unless the nomem options is also set).

...or...

  • nomem: The asm! block does not read from or write to any memory accessible outside of the asm! block.

...or...

  • readonly: The asm! block does not write to any memory accessible outside of the asm! block.

...or even...

  • nostack: The asm! block does not push data to the stack...

...do we mean for these to apply to the label blocks also? I wouldn't think so, but it's rather unclear in reading it.

@nbdd0121
Copy link
Contributor Author

In reading this more carefully (especially the lines not changed), I find myself wanting to double check that we're being precise enough now by what we mean by "the asm! block". Previously, when speaking about what happened during execution of "the asm! block", that meant only the execution of the assembly code. But now, given that the label blocks are lexically within the asm! block and may be executed during the execution of the asm! block, it would be reasonable to read "the asm! block" as to include that code also.

So when we say, e.g., that...

  • pure: The asm! block has no side effects, must eventually return, and its outputs depend only on its direct inputs (i.e. the values themselves, not what they point to) or values read from memory (unless the nomem options is also set).

GCC manual says "Also note that an asm goto statement is always implicitly considered volatile.", although AFAIU it's possible to represent a asm-goto in LLVM without the sideeffect modifier. Clang generates LLVM IR differently for asm goto and asm goto volatile, however LLVM doesn't seem to handle it specially (e.g. function containing a "pure" asm-goto block isn't treated as pure and is not optimised out).

It's not clear what exactly is pure for an asm goto block, since the control flow change is also one of its output. We probably should just forbid pure + label from being used together -- however note that since pure require an output operand, this is currently forbidden without a stable asm_goto_with_outputs feature.

...or...

  • nomem: The asm! block does not read from or write to any memory accessible outside of the asm! block.

...or...

  • readonly: The asm! block does not write to any memory accessible outside of the asm! block.

...or even...

  • nostack: The asm! block does not push data to the stack...

...do we mean for these to apply to the label blocks also? I wouldn't think so, but it's rather unclear in reading it.

These would just mean the assembly themselves.

@nbdd0121
Copy link
Contributor Author

I wouldn't think so, but it's rather unclear in reading it.

How about replacing "asm! block" with "assembly code"?

@traviscross
Copy link
Contributor

How about replacing "asm! block" with "assembly code"?

Sounds about right, assuming that's correct in each use, which of course should be carefully checked. Everywhere that the word "block" is used should be reviewed during this rename.

@rustbot

This comment has been minimized.

@nikomatsakis nikomatsakis self-assigned this Jan 30, 2025
@nbdd0121 nbdd0121 force-pushed the master branch 2 times, most recently from cc990ed to 040fc58 Compare February 10, 2025 13:09
nbdd0121 and others added 3 commits February 10, 2025 13:14
The asm-goto-with-outputs is still unstable, so in the reference it's
still mentioned as forbidden by a check rule.
The latter now only means the entire block, while the former means the
assembly code specified within the block (thus excluding `label` blocks).
@ehuss
Copy link
Contributor

ehuss commented Mar 13, 2025

@joshtriplett Indicated they could give a review over the chapter to make sure everything still fits together.

@joshtriplett
Copy link
Member

I reviewed the entire chapter, and wrote nbdd0121#1 as a PR to this PR branch, since there were changes in areas outside the context of the patch, which github doesn't allow suggestions in.

I added some more disambiguations in cases of possible ambiguity between the entire asm! block (including label blocks) and the assembly code within it.

The second commit, which I've kept separate, attempts to clarify an interaction between noreturn and label blocks; however, for that commit I could use some confirmation on accuracy. The phrasing as previously written said that "A noreturn asm block behaves just like a function which doesn't return", but that isn't accurate in the presence of label blocks, since the asm block can return, from the label blocks. I changed it to apply to "A noreturn asm block with no label blocks". However, this assumes there are not more semantic connotations of noreturn here that do apply to asm blocks with label blocks.

With the first commit applied, and either the second commit (if accurate) or something like it (if another phrasing would be more accurate) applied, the chapter looks fully consistent to me.

joshtriplett and others added 3 commits March 13, 2025 17:55
Update more cases where the phrasing could potentially have been
interpreted as affecting a `label` block.
Let's make some clarifications of substance and editorial tweaks to
the changes for `asm_goto`.  We add an example, we change various
wording to be more clear -- in particular, we significantly revise the
section on `noreturn` to unify the statement of the rules -- and we
replace remaining uses of "asm code" with "assembly code" to match our
verbiage elsewhere.

The main clarification of substance is that for requirements that must
be upheld when falling through the assembly, such as he requirement to
restore the stack pointer, we must also note that the requirement must
be satisfied before jumping to any `label` blocks.
Copy link
Contributor

@traviscross traviscross left a comment

Choose a reason for hiding this comment

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

Looks good to me. Ready to go.

@traviscross traviscross removed the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Mar 17, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 17, 2025
…rcote

Stabilize `asm_goto` feature gate

Stabilize `asm_goto` feature (tracked by rust-lang#119364). The issue will remain open and be updated to track `asm_goto_with_outputs`.

Reference PR: rust-lang/reference#1693

# Stabilization Report

This feature adds a `label <block>` operand type to `asm!`. `<block>` must be a block expression with type unit or never. The address of the block is substituted and the assembly may jump to the block. When block completes the `asm!` block returns and continues execution.

The block starts a new safety context and unsafe operations within must have additional `unsafe`s; the effect of `unsafe` that surrounds `asm!` block is cancelled. See rust-lang#119364 (comment) and rust-lang#131544.

It's currently forbidden to use `asm_goto` with output operands; that is still unstable under `asm_goto_with_outputs`.

Example:

```rust
unsafe {
    asm!(
        "jmp {}",
        label {
            println!("Jumped from asm!");
        }
    );
}
```

Tests:
- tests/ui/asm/x86_64/goto.rs
- tests/ui/asm/x86_64/goto-block-safe.stderr
- tests/ui/asm/x86_64/bad-options.rs
- tests/codegen/asm/goto.rs
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 17, 2025
Rollup merge of rust-lang#133870 - nbdd0121:asm, r=traviscross,nnethercote

Stabilize `asm_goto` feature gate

Stabilize `asm_goto` feature (tracked by rust-lang#119364). The issue will remain open and be updated to track `asm_goto_with_outputs`.

Reference PR: rust-lang/reference#1693

# Stabilization Report

This feature adds a `label <block>` operand type to `asm!`. `<block>` must be a block expression with type unit or never. The address of the block is substituted and the assembly may jump to the block. When block completes the `asm!` block returns and continues execution.

The block starts a new safety context and unsafe operations within must have additional `unsafe`s; the effect of `unsafe` that surrounds `asm!` block is cancelled. See rust-lang#119364 (comment) and rust-lang#131544.

It's currently forbidden to use `asm_goto` with output operands; that is still unstable under `asm_goto_with_outputs`.

Example:

```rust
unsafe {
    asm!(
        "jmp {}",
        label {
            println!("Jumped from asm!");
        }
    );
}
```

Tests:
- tests/ui/asm/x86_64/goto.rs
- tests/ui/asm/x86_64/goto-block-safe.stderr
- tests/ui/asm/x86_64/bad-options.rs
- tests/codegen/asm/goto.rs
@ehuss ehuss added this pull request to the merge queue Mar 18, 2025
Merged via the queue into rust-lang:master with commit 074bbab Mar 18, 2025
5 checks passed
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Mar 20, 2025
Stabilize `asm_goto` feature gate

Stabilize `asm_goto` feature (tracked by #119364). The issue will remain open and be updated to track `asm_goto_with_outputs`.

Reference PR: rust-lang/reference#1693

# Stabilization Report

This feature adds a `label <block>` operand type to `asm!`. `<block>` must be a block expression with type unit or never. The address of the block is substituted and the assembly may jump to the block. When block completes the `asm!` block returns and continues execution.

The block starts a new safety context and unsafe operations within must have additional `unsafe`s; the effect of `unsafe` that surrounds `asm!` block is cancelled. See rust-lang/rust#119364 (comment) and rust-lang/rust#131544.

It's currently forbidden to use `asm_goto` with output operands; that is still unstable under `asm_goto_with_outputs`.

Example:

```rust
unsafe {
    asm!(
        "jmp {}",
        label {
            println!("Jumped from asm!");
        }
    );
}
```

Tests:
- tests/ui/asm/x86_64/goto.rs
- tests/ui/asm/x86_64/goto-block-safe.stderr
- tests/ui/asm/x86_64/bad-options.rs
- tests/codegen/asm/goto.rs
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.

8 participants