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

asm: Allow multiple template string arguments; interpret them as newline-separated #73364

Merged
merged 4 commits into from
Jun 20, 2020

Conversation

joshtriplett
Copy link
Member

@joshtriplett joshtriplett commented Jun 15, 2020

Allow the asm! macro to accept a series of template arguments, and interpret them as if they were concatenated with a '\n' between them. This allows writing an asm! where each line of assembly appears in a separate template string argument.

This syntax makes it possible for rustfmt to reliably format and indent each line of assembly, without risking changes to the inside of a template string. It also avoids the complexity of having the user carefully format and indent a multi-line string (including where to put the surrounding quotes), and avoids the extra indentation and lines of a call to concat!.

For example, rewriting the second example from the blog post on the new inline assembly syntax using multiple template strings:

fn main() {
    let mut bits = [0u8; 64];
    for value in 0..=1024u64 {
        let popcnt;
        unsafe {
            asm!(
                "    popcnt {popcnt}, {v}",
                "2:",
                "    blsi rax, {v}",
                "    jz 1f",
                "    xor {v}, rax",
                "    tzcnt rax, rax",
                "    stosb",
                "    jmp 2b",
                "1:",
                v = inout(reg) value => _,
                popcnt = out(reg) popcnt,
                out("rax") _, // scratch
                inout("rdi") bits.as_mut_ptr() => _,
            );
        }
        println!("bits of {}: {:?}", value, &bits[0..popcnt]);
    }
}

Note that all the template strings must appear before all other arguments; you cannot, for instance, provide a series of template strings intermixed with the corresponding operands.

…xpect`

Currently, `asm!` parsing uses an `expect` for the last parsed
pseudo-keyword (`sym`), which makes it difficult to extend without
simultaneously refactoring. Use `eat` for the last pseudo-keyword, and
then add an `else` that fails parsing. No change to error output.
pprust uses `print_string` to write out the template string, and
`print_string` already calls `escape_debug`, so `impl fmt::Display for
InlineAsmTemplatePiece` shouldn't do an additional `escape_debug`.

This fixes a pretty-printing bug that translated
`asm!("...\n...")`
to
`asm!("...\\n...")`
@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 15, 2020
@joshtriplett
Copy link
Member Author

r? @Amanieu

@rust-highfive rust-highfive assigned Amanieu and unassigned eddyb Jun 15, 2020
@joshtriplett

This comment has been minimized.

Copy link
Member

@Amanieu Amanieu left a comment

Choose a reason for hiding this comment

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

Have you checked that this is correctly handling implicitly number arguments in the template over multiple template strings? For example asm!("{}", "{}", in(reg) a, in(reg) b).

src/librustc_ast/ast.rs Outdated Show resolved Hide resolved
src/librustc_builtin_macros/asm.rs Outdated Show resolved Hide resolved
src/librustc_builtin_macros/asm.rs Show resolved Hide resolved
@joshtriplett

This comment has been minimized.

@petrochenkov petrochenkov self-assigned this Jun 15, 2020
@rust-highfive

This comment has been minimized.

src/librustc_builtin_macros/asm.rs Outdated Show resolved Hide resolved
src/librustc_builtin_macros/asm.rs Outdated Show resolved Hide resolved
src/librustc_parse_format/lib.rs Outdated Show resolved Hide resolved
src/librustc_parse_format/lib.rs Outdated Show resolved Hide resolved
src/librustc_builtin_macros/asm.rs Show resolved Hide resolved
…ated

Allow the `asm!` macro to accept a series of template arguments, and
interpret them as if they were concatenated with a '\n' between them.
This allows writing an `asm!` where each line of assembly appears in a
separate template string argument.

This syntax makes it possible for rustfmt to reliably format and indent
each line of assembly, without risking changes to the inside of a
template string. It also avoids the complexity of having the user
carefully format and indent a multi-line string (including where to put
the surrounding quotes), and avoids the extra indentation and lines of a
call to `concat!`.

For example, rewriting the second example from the [blog post on the new
inline assembly
syntax](https://blog.rust-lang.org/inside-rust/2020/06/08/new-inline-asm.html)
using multiple template strings:

```rust

fn main() {
    let mut bits = [0u8; 64];
    for value in 0..=1024u64 {
        let popcnt;
        unsafe {
            asm!(
                "    popcnt {popcnt}, {v}",
                "2:",
                "    blsi rax, {v}",
                "    jz 1f",
                "    xor {v}, rax",
                "    tzcnt rax, rax",
                "    stosb",
                "    jmp 2b",
                "1:",
                v = inout(reg) value => _,
                popcnt = out(reg) popcnt,
                out("rax") _, // scratch
                inout("rdi") bits.as_mut_ptr() => _,
            );
        }
        println!("bits of {}: {:?}", value, &bits[0..popcnt]);
    }
}
```

Note that all the template strings must appear before all other
arguments; you cannot, for instance, provide a series of template
strings intermixed with the corresponding operands.

In order to get srcloc mappings right for macros that generate
multi-line string literals, create one line_span for each
line in the string literal, each pointing to the macro.

Make `rustc_parse_format::Parser::curarg` `pub`, so that we can
propagate it from one template string argument to the next.
@Amanieu

This comment has been minimized.

@petrochenkov
Copy link
Contributor

and interpret them as if they were concatenated with a '\n' between them

So it's not just C-style concatenation of neighboring string literals for people accustomed to it.
This certainly makes it more useful.

@petrochenkov petrochenkov removed their assignment Jun 15, 2020
…uments

Update all examples to use the new formatting, and update explanations
to document it.
@joshtriplett
Copy link
Member Author

@Amanieu

Can you update src/doc/unstable-book/src/library-features/asm.md with the same changes you made to the RFC?

Done; thanks!

@joshtriplett joshtriplett changed the title asm: Allow multiple template strings; interpret them as newline-separated asm: Allow multiple template string arguments; interpret them as newline-separated Jun 15, 2020
@Amanieu
Copy link
Member

Amanieu commented Jun 16, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jun 16, 2020

📌 Commit fd9ed30 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 Jun 16, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 19, 2020
…arth

Rollup of 13 pull requests

Successful merges:

 - rust-lang#71568 (Document unsafety in slice/sort.rs)
 - rust-lang#72709 (`#[deny(unsafe_op_in_unsafe_fn)]` in liballoc)
 - rust-lang#73214 (Add asm!() support for hexagon)
 - rust-lang#73248 (save_analysis: improve handling of enum struct variant)
 - rust-lang#73257 (ty: projections in `transparent_newtype_field`)
 - rust-lang#73261 (Suggest `?Sized` when applicable for ADTs)
 - rust-lang#73300 (Implement crate-level-only lints checking.)
 - rust-lang#73334 (Note numeric literals that can never fit in an expected type)
 - rust-lang#73357 (Use `LocalDefId` for import IDs in trait map)
 - rust-lang#73364 (asm: Allow multiple template string arguments; interpret them as newline-separated)
 - rust-lang#73382 (Only display other method receiver candidates if they actually apply)
 - rust-lang#73465 (Add specialization of `ToString for char`)
 - rust-lang#73489 (Refactor hir::Place)

Failed merges:

r? @ghost
@bors bors merged commit 687f929 into rust-lang:master Jun 20, 2020
@joshtriplett joshtriplett deleted the inline-asm branch June 20, 2020 03:49
joshtriplett added a commit to joshtriplett/blog.rust-lang.org that referenced this pull request Jun 20, 2020
rust-lang/rust#73364 implemented support for
providing multiple lines of assembly as separate arguments to `asm!`;
update the blog post to use that new syntax, so that people who find it
will use that style as an example.
XAMPPRocky pushed a commit to rust-lang/blog.rust-lang.org that referenced this pull request Jun 22, 2020
* inline-asm: Update for new style

rust-lang/rust#73364 implemented support for
providing multiple lines of assembly as separate arguments to `asm!`;
update the blog post to use that new syntax, so that people who find it
will use that style as an example.

* inline-asm: Outdent

* inline asm: Update play link
jackh726 pushed a commit to jackh726/blog.rust-lang.org that referenced this pull request Jul 26, 2021
* inline-asm: Update for new style

rust-lang/rust#73364 implemented support for
providing multiple lines of assembly as separate arguments to `asm!`;
update the blog post to use that new syntax, so that people who find it
will use that style as an example.

* inline-asm: Outdent

* inline asm: Update play link
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants