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

const operands in asm! should use inline consts rather than const promotion #83169

Closed
bstrie opened this issue Mar 15, 2021 · 6 comments · Fixed by #83916
Closed

const operands in asm! should use inline consts rather than const promotion #83169

bstrie opened this issue Mar 15, 2021 · 6 comments · Fixed by #83916
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) F-asm `#![feature(asm)]` (not `llvm_asm`) F-inline_const Inline constants (aka: const blocks, const expressions, anonymous constants)

Comments

@bstrie
Copy link
Contributor

bstrie commented Mar 15, 2021

Elevating an item from the asm! progress report from a comment to an issue so that it can be tracked:


const operands should switch to using the new inline consts instead of explicit promotion. In theory this should have no impact on existing code, but it requires inline consts to support references to generic paramters of the surrounding function, which is not supported yet.

One aspect of asm! that I think needs further scrutiny before stabilization is const arguments, which currently are one of the few syntactic constructs that trigger promotion. Promotion is about turning a part of a function into an anonymous const (the "promoted") and using that instead of the ripped-out part of the function body. This has all sorts of "interesting" consequences (see e.g. rust-lang/rfcs#3027).

Similar to the situation for array repeat expressions in #49147, there is an alternative: we could avoid having to deal with promotion here by requiring that the const argument be either a literal (4), a named constant (CONST), or an explicit inline const (const { arbitrary_const_expr }). If that would be acceptable to y'all, I think it would be better to only support this subset upon initial stabilization. This is forward compatible with supporting more expressions by performing promotion later. The only difference this would make is one of convenience -- one could avoid having to type const { ... }.

@bstrie bstrie added A-inline-assembly Area: Inline assembly (`asm!(…)`) F-inline_const Inline constants (aka: const blocks, const expressions, anonymous constants) labels Mar 15, 2021
@Amanieu Amanieu added the F-asm `#![feature(asm)]` (not `llvm_asm`) label Mar 15, 2021
@RalfJung
Copy link
Member

RalfJung commented Mar 16, 2021

(The syntax I suggested in the quote above is up for bikeshedding of course; the key point is that the const expression is treated separately from the start and we don't have to hot-fix the MIR later via promotion.)

@bstrie
Copy link
Contributor Author

bstrie commented Mar 16, 2021

The syntax I suggested in the quote above is up for bikeshedding of course

Surely it would just use the same syntax as inline const in general, yes? #76001 suggests that const { $expr } is the chosen syntax for this feature, but is there any reason to suspect that this is incompatible with the syntax of the asm! macro?

@RalfJung
Copy link
Member

asm! being a macro can do almost whatever it wants. ;)
So we'd end up with something like this?

    asm!(
        "mov {0}, {1}",
        "add {0}, {number}",
        out(reg) o,
        in(reg) i,
        number = const { 5 },
    );

@bstrie
Copy link
Contributor Author

bstrie commented Mar 16, 2021

Sure, although just as you say in your comment it should also be possible to elide the explicit const for the case of literals and named consts. I believe this exactly mirrors your plan for const array repeat expressions, yes? I.e. [5; 2], [FOO; 2], [const { foo() }; 2]?

@RalfJung
Copy link
Member

RalfJung commented Mar 16, 2021

Well, the asm! RFC says const is a required keyword here, it just does not use curly braces.

For arrays, there's not much of a "plan", but [5; 2] already works so we'll certainly continue supporting it. In [expr; COUNT], expr is a normal expression syntactically, so the const keyword here is nothing sepcific to arrays but just the same keyword as in let x = const { 1+2 };. In contrast, asm! has a const keyword built-in (in the RFC).

@Amanieu
Copy link
Member

Amanieu commented Apr 6, 2021

I made an attempt at switching const operands to use inline consts (well, AnonConst to be precise) in #83916.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 7, 2021
Use AnonConst for asm! constants

This replaces the old system which used explicit promotion. See rust-lang#83169 for more background.

The syntax for `const` operands is still the same as before: `const <expr>`.

Fixes rust-lang#83169

Because the implementation is heavily based on inline consts, we suffer from the same issues:
- We lose the ability to use expressions derived from generics. See the deleted tests in `src/test/ui/asm/const.rs`.
- We are hitting the same ICEs as inline consts, for example rust-lang#78174. It is unlikely that we will be able to stabilize this before inline consts are stabilized.
@bors bors closed this as completed in b81c6cd Apr 7, 2021
flip1995 pushed a commit to flip1995/rust that referenced this issue Apr 8, 2021
Use AnonConst for asm! constants

This replaces the old system which used explicit promotion. See rust-lang#83169 for more background.

The syntax for `const` operands is still the same as before: `const <expr>`.

Fixes rust-lang#83169

Because the implementation is heavily based on inline consts, we suffer from the same issues:
- We lose the ability to use expressions derived from generics. See the deleted tests in `src/test/ui/asm/const.rs`.
- We are hitting the same ICEs as inline consts, for example rust-lang#78174. It is unlikely that we will be able to stabilize this before inline consts are stabilized.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) F-asm `#![feature(asm)]` (not `llvm_asm`) F-inline_const Inline constants (aka: const blocks, const expressions, anonymous constants)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants