Lang proposal: Allow #[cfg(...)] within asm!

[tgross35]

Background

Currently there isn't an easy way to enable or disable portions of assembly based on configuration. This can be a annoyance for asm! that can be worked around with multiple assembly blocks, but it is especially a problem in global_asm! and naked_asm! because small config-based changes mean the entire assembly block needs to be duplicated.

Another workaround is possible by defining multiple macro_rules! of the same name that are enabled/disabled based on the desired config and expand to strings. This option is inconvenient and fragments the assembly.

Proposal

Allow #[cfg(...)] in all assembly macros:

asm!( // or global_asm! or naked_asm!
    "nop",
    #[cfg(target_feature = "sse2")]
    "nop",
    // ...
    #[cfg(target_feature = "sse2")]
    a = const 123, // only used on sse2
);

The configuration applies to a single comma-separated segment.

We may want to also support blocks in order to allow grouping instructions without merging strings, or grouping directives:

asm!(
    "nop",
    #[cfg(target_feature = "sse2")] {
        "nop",
        "nop",
        "nop",
    } // does allowing/requiring a comma here make sense?
    "nop",
    #[cfg(target_feature = "sse2")] {
        a = const 123,
        a = const 123,
    }
);

This design has some concerns and there may be a better option, see discussion in the thread.

Zulip discussion: #project-inline-asm > `cfg` in assembly blocks

[tgross35]

Unfortunately I won't have the chance to work on this (at least for a while), but this been mentioned in a few different places so I wrote this up to keep track.

I'll probably add help wanted labels in case somebody interested wants to pick this up before I do, but @rust-lang/lang would anybody mind giving this a vibe check first? I'll nominate as well, feel free to remove if that doesn't make sense.

@rustbot label +I-lang-nominated +I-lang-easy-decision

[tgross35]

Cc @folkertdev in case you have any thoughts here, since you have done a lot of work on assembly internals recently.

[joshtriplett]

General thoughts: I like the concept, and it seems reasonable, but it also seems likely to become very verbose and redundant if you have many lines tagged with the same complex cfg. In C you can wrap multiple lines in the same ifdef, and I think this proposed syntax for cfg in asm! will lead people to combine multiple assembly lines into one string (e.g. with \n or with dialect-specific separators like ;), just to be able to use one cfg.

I'm wondering if we should proactively address that use case, to avoid encouraging suboptimal formatting like that.

For instance, perhaps we should allow:

asm!(
    "always",
    #[cfg(complicated)]
    {
        "sometimes",
        "perhaps",
    }
);

Also, ideally cfg_match! should work, to avoid having to write both cfg(complicated) and cfg(not(complicated)) (where complicated here is a stand-in for some very long compound cfg).

[tgross35]

I mentioned blocks on the Zulip thread as well, and think they would be a nice to have because it also allows applying the same cfg to multiple operands. Updated the description to include this.

@adamgreig linked to an example at https://github.com/rust-embedded/cortex-m/blob/c3d664bba1148cc2d0f963ebeb788aa347ba81f7/cortex-m-rt/src/lib.rs#L528-L636 that uses merged strings and looks pretty good, for comparison.

One downside of blocks is that we don't have any cases (that I can think of) where {...} can be used to group a subset of comma-delimited items within a larger list, like

let x = [1, 2, #[cfg(unix)] { 3, 4 }, 5];

Which may make cfg_match support more difficult. Agreed that this would great to have though (cfg_if as well).

Unfortunately with grouping, this got less easy :)

@rustbot label -I-lang-easy-decision

(looks like I'm racing with rustbot on the labels)

[joshtriplett]

@tgross35 Yeah, that linked example is a good case study. I hadn't thought about the possibility of just using the newline and continuing the string constant. That doesn't look too bad. We should evaluate how we want to see people write that example, in an ideal world:

• Blocks
• Multi-line string constants
• concat! or similar
• \n or ; or similar
• Some other idea we haven't thought of yet.

[joshtriplett]

Personally, I would somewhat favor blocks, because I think multi-line strings blend together a bit much, and it's harder to see the boundaries between them. In the linked example, there are large comments and blank lines, which help; in the absence of those comments and blank lines, if you just have #[cfg(...)] and multi-line strings and commas, I think it'd be much less readable/skimmable.

[ktnlvr]

It seems like cfg_match! would be better, but it also does not require being exhaustive, which will certainly cause trouble in the ASM. Is there a way to require exhaustiveness from cfg_match! and use #[cfg(...)] for non-exhaustive cases?

[joshtriplett]

@ktnlvr I don't think exhaustiveness would be an issue here; if there's no matching branch, that cfg_match should add no assembly.

[folkertdev]

In general, I'm very much in favor of this idea!

The simple cfg idea already parses with #![feature(stmt_expr_attributes)], though you can't actually disable lines:

error: removing an expression is not supported in this position
  --> /home/folkertdev/rust/rust/tests/ui/asm/cfg.rs:10:9
   |
LL |         #[cfg(reva)]
   |         ^^^^^^^^^^^^

Also, ideally cfg_match! should work

Absolutely! I'm lacking some context here, but the cfg_match! macro currently does not work in expression contexts, so it currently fails to parse.

Could we make this work (without an additional { ... } block for each arm)?

cfg_match! {
    feature = "zero-init-ram" => concat!(
        "ldr r0, =_ram_start",
        "ldr r1, =_ram_end",
    )
    _ => concat!(
        "ldr r0, =__sbss",
        "ldr r1, =__ebss",
    )
}

We should evaluate how we want to see people write that example

This is interesting. Firstly, this example could now be a naked function I believe.

#[unsafe(naked)]
extern "C" fn Reset() { 
    naked_asm!(
        "..."
    )
}

That removes a bunch of ceremony from the start/end.

I see no real advantage to blocks versus concat!, which already works and does not have the complications of expanding to something that is not a valid expression:

asm!(
    #[cfg(feature = "zero-init-ram")]
    {
        "ldr r0, =_ram_start",
        "ldr r1, =_ram_end",
    }

    #[cfg(feature = "zero-init-ram")]
    concat!(
        "ldr r0, =_ram_start",
        "ldr r1, =_ram_end",
    )
)

So after some playing around I end up with this https://gist.github.com/folkertdev/6a5ebe1272b38472310cec3964b6ae44. The formatter removes additional whitespace, but formatting of asm! is sort of undecided (see rust-lang/style-team#152).

---

I'd definitely be up for implementing this if we feel like we have a design that works. Just a plain cfg seems uncontroversial:

asm!(
    #[cfg(target_feature = "sse2")]
    "nop",
)

So we could start there, and see what else makes sense to add as we go?

[traviscross]

We should also probably ask whether it makes sense to special-case asm here, or whether we could find a way to answer this question for all expressions in argument position.

Maybe some version of cfg_match! helps here. Allowing that rather than bare #[cfg] in argument position seems maybe easier, as it could ensure some expression is always produced for the argument.

cc #115585

[tgross35]

@traviscross to clarify, are you suggesting that #[cfg(...)] should continue to be rejected within asm!? @Amanieu pointed out on Zulip, we do allow code like foo(10, #[cfg(unix)] 20, 30);, having consistency with that seems to make things nicely predictable. If you only meant to leverage cfg_match for applying to multiple items in a list, rather than something like blocks, then that makes sense.

[traviscross]

@Amanieu pointed out on Zulip, we do allow code like foo(10, #[cfg(unix)] 20, 30);

fn f(_: u8, _: u8) {}

macro_rules! g {
    ($($es:expr),*) => { f($($es),*) };
}

fn main() {
    f(#[cfg(true)] 0, #[cfg(false)] 0, #[cfg(true)] 0); //~ OK
    g!(#[cfg(true)] 0, #[cfg(false)] 0, #[cfg(true)] 0); //~ OK
}

...so we do -- even through macro calls in the form of g above. Yes, then it seems straightforward that we should be consistent with that.

My personal estimate is that we'd treat doing that for asm as a straightforward extension (i.e. no RFC required), and that a stabilization PR for this would be likely well received upon its nomination for us.

[folkertdev]

I'm digging through the code a bit more (it's not an area I know well, but we'll get there), and the reason for the "removing an expression is not supported in this position" error is the use of expr_to_spanned_string which is applied to the arguments of the asm! macro (to e.g. expand concat! calls). This function enters macro-expansion in sort of a custom way that eventually hits:

trait InvocationCollectorNode: HasAttrs + HasNodeId + Sized {
    // ...

    fn expand_cfg_false(
        &mut self,
        collector: &mut InvocationCollector<'_, '_>,
        _pos: usize,
        span: Span,
    ) {
        collector.cx.dcx().emit_err(RemoveNodeNotSupported { span, descr: Self::descr() });
    }
}

Normal expressions follow a different path, and for them expand_cfg_false is never actually called, the #[cfg(false)]'d node is just removed.

The expr_to_spanned_string function is used for a number of other macros that depend on arguments being a string, for instance format! and env!. So similarly, this currently does not work, but following the logic of consistency with standard expressions, maybe it should?

format!(
    #[cfg(false)]
    "foo",
    #[cfg(true)]
    "bar",
)

Also using the expression logic for expansion clearly saves a lot of code, but I think some specialization is needed to tell the asm! (or println! etc) macro expansion logic that a particular string argument is actually cfg'd out.

[Amanieu]

The asm! macro can directly evaluate the cfg as part of its expansion in the same way cfg! does. Additionally some special handling is going to be needed for operands. Notably we probably don't want to allow positional operands to be cfg'd out since that may impact the numbering of operands.

[folkertdev]

The asm! macro can directly evaluate the cfg as part of its expansion in the same way cfg! does.

Ok yes that is much easier, thanks!

Notably we probably don't want to allow positional operands to be cfg'd out since that may impact the numbering of operands.

Is that restriction needed? I don't see why this sort of thing is useful per se, but it seems kind of arbitrary to disallow e.g. this:

std::arch::asm!(
    "/* {0} */", 
    #[cfg(a)]
    const 4,
    #[cfg(not(a))]
    const 8,
);

Actually, do you have specific examples that you'd want to disallow?

[Amanieu]

My main concern is that the index of operands can vary depending on which cfgs are enabled. We don't have to disallow it, but I feel it makes the asm! less readable and potentially harder for tools to parse.

std::arch::asm!(
    "/* {0} */", 
    #[cfg(a)]
    const 4,
    const 8,  // <-- the index of this operand depends on cfg(a)
);

[tgross35]

If we do that, it may make sense to also disallow use of positional arguments in string literals that are gated by cfg.

[joshtriplett]

That seems, to me, to be the territory of lints rather than hard errors: we could lint if a cfg affects positional argument indexes, and recommend using named arguments. (It should continue to be a hard error if the positional arguments don't match.)

[traviscross]

Agreed. For a lint, perhaps we'd want it to push people toward cfg_match!, since that could likely achieve the person's ends while being clearer about not shifting around arguments. Personally, I wouldn't block shipping this feature on this linting.

One might say, in a way, that we already accept this sort of thing.

unsafe extern "C" {
    fn printf(fmt: *const i8, ...);
}

fn main() {
    unsafe {
        printf(
            c"%s".as_ptr(),
            #[cfg(false)]
            c"Hello world.\n".as_ptr(),
            c"Hello Rust.\n".as_ptr(),
        );
    }
}

Playground link