feat: riscv-interrupt-{m,s} calling conventions

[sethp]

Similar to prior support added for the mips430, avr, and x86 targets this change implements the rough equivalent of clang's __attribute__((interrupt)) for riscv targets, enabling e.g.

static mut CNT: usize = 0;

pub extern "riscv-interrupt-m" fn isr_m() {
    unsafe {
        CNT += 1;
    }
}

to produce highly effective assembly like:

pub extern "riscv-interrupt-m" fn isr_m() {
420003a0:       1141                    addi    sp,sp,-16
    unsafe {
        CNT += 1;
420003a2:       c62a                    sw      a0,12(sp)
420003a4:       c42e                    sw      a1,8(sp)
420003a6:       3fc80537                lui     a0,0x3fc80
420003aa:       63c52583                lw      a1,1596(a0) # 3fc8063c <_ZN12esp_riscv_rt3CNT17hcec3e3a214887d53E.0>
420003ae:       0585                    addi    a1,a1,1
420003b0:       62b52e23                sw      a1,1596(a0)
    }
}
420003b4:       4532                    lw      a0,12(sp)
420003b6:       45a2                    lw      a1,8(sp)
420003b8:       0141                    addi    sp,sp,16
420003ba:       30200073                mret

(disassembly via riscv64-unknown-elf-objdump -C -S --disassemble ./esp32c3-hal/target/riscv32imc-unknown-none-elf/release/examples/gpio_interrupt)

This outcome is superior to hand-coded interrupt routines which, lacking visibility into any non-assembly body of the interrupt handler, have to be very conservative and save the entire CPU state to the stack frame. By instead asking LLVM to only save the registers that it uses, we defer the decision to the tool with the best context: it can more accurately account for the cost of spills if it knows that every additional register used is already at the cost of an implicit spill.

At the LLVM level, this is apparently implemented by marking every register as "callee-save," matching the semantics of an interrupt handler nicely (it has to leave the CPU state just as it found it after its {m|s}ret).

This approach is not suitable for every interrupt handler, as it makes no attempt to e.g. save the state in a user-accessible stack frame. For a full discussion of those challenges and tradeoffs, please refer to the interrupt calling conventions RFC.

Inside rustc, this implementation differs from prior art because LLVM does not expose the "all-saved" function flavor as a calling convention directly, instead preferring to use an attribute that allows for differentiating between "machine-mode" and "superivsor-mode" interrupts.

Finally, some effort has been made to guide those who may not yet be aware of the differences between machine-mode and supervisor-mode interrupts as to why no riscv-interrupt calling convention is exposed through rustc, and similarly for why riscv-interrupt-u makes no appearance (as it would complicate future LLVM upgrades).

[rustbot]

r? @jackh726

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

[rustbot]

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in src/tools/rust-analyzer

cc @rust-lang/rust-analyzer

These commits modify compiler targets.
(See the Target Tier Policy.)

[sethp]

So the riscv UI test failures have taken some debugging. It seems that these are the first riscv-specific UI tests, which have been subtly broken since #110884: on any version of LLVM that does not recognize that feature (i.e. any before rust-lang/llvm-project@da7c712, which appears to have landed in ~15.0.0), the act of filtering down the total list of features known to rustc attempts to apply each feature to a short-lived feature set, causing LLVM to emit this error as a side effect:

+ '+unaligned-scalar-mem' is not a recognized feature for this target (ignoring feature)

Since this goes directly to stderr, the UI tests pick it up and observe "hey, on LLVM 14 there's this extra output that's not in the .stderr file": and on versions that do support the feature, no such message is printed. (I suspect, but have not confirmed, that this means every feature listed in that file is supported across every tested version of LLVM, except apparently for this one).

As a potential fix, in 70b2188 I've modified the C++ side of the binding to search the feature table directly rather than delegating to the side-effecting class member, but this requires access to the feature table. Accessors were added to the Rust fork in rust-lang/llvm-project@da7c712 , but the failing test is being built against Ubuntu's system LLVM that lacks any rust-specific patches, so that accessor doesn't exist. Instead, I've had to resort to, uh, cleverness to access the private field in a way that the internet claims ought to be portable across LLVM and language versions, and C++ compilers. Feedback gladly accepted on where else there might be some leverage here that I'm missing, either within the test suite or elsewhere.

[jackh726]

This likely needs some lang and/or compiler team okay

[sethp]

Thanks for taking a look, @jackh726! Is this something the lang and/or compiler teams are going to see, or is there an action I should be taking to discuss it with them?

[jackh726]

I'm going to start by tagging @rust-lang/lang. I guess adding a new calling convention needs at least someone on lang to okay (it's behind a feature gate, so likely doesn't need full FCP). For compiler, not sure - maybe an MCP is warranted, but probably not.

For lang team: Is it okay to add a feature gate for new risvc-interrupt-{m,s} calling conventions?

[jackh726]

Lang nominating. See above comment.

[nikomatsakis]

@jackh726 this sounds like an experimental feature gate, although I think it would ultimately be covered under rust-lang/rfcs#3246. We discussed in meeting and are happy.

@tmandry can serve as liaison.

[jackh726]

Can you squash your commits?

[sethp]

Can you squash your commits?

Sure, I did a little bit of editing down to cut out the stuff that's only useful for test usability. Let me know if you want me to do any more!

[jackh726]

Is this change nice-to-have, or required? If it's just nice-to-have, I'd rather it be split into a separate PR, since I'm not personally comfortable reviewing this alone.

[sethp]

I wrote more about this in #111891 (comment) and 7161c5e : prior to this change, it was impossible to introduce riscv UI tests to the compiler; I'm not super confident either in whatever is happening here, but it does seem important to me to include in this context. Could we ask someone else to help us take a look here?

[jackh726]

Let's tag @nikic and @rust-lang/wg-llvm

It might still be worth to split this into a separate PR that can have a separate reviewer assigned.

[nikic]

This is obviously not acceptable. You probably no longer need this because LLVM 14 is no longer supported.

[sethp]

Ah! Dropping LLVM 14 support may indeed decrease the urgency here, though I'm not totally comfortable abandoning it entirely. The side-effect-ful filter call will cause any architecture's UI tests to fail any time any supported version of LLVM doesn't recognize a feature that rustc knows about, thereby seeming to defeat the purpose of the feature filter (at least as far as UI tests go)?

[jackh726]

Thanks for removing this change here. If you feel strongly about this, a separate PR is likely the right way to go about this.

[sethp]

Well, the tests pass, so the good news is that means there's currently no features at the LLVM level that exist in any newer versions, which does indeed mean these changes can be decoupled. I did want to leave it up to @nikic what the resolution ought to be: I searched for a few hours before coming up with the approach here, and this was the only one I found that worked against system-provided LLVM. I'm open to an alternative approach, but if y'all would rather do nothing, then perhaps an issue documenting the problem would be a more fitting outcome than a PR with this change in it. What do you think?

[jackh726]

Opening an issue to document the problem is a reasonable approach to move forward with this.

[bors]

☔ The latest upstream changes (presumably #114569) made this pull request unmergeable. Please resolve the merge conflicts.

[jackh726]

Please remove the llvm changes/revert from the commit history, and squash the final test change commit into the previous ones. After that, I think this should be good to go.

[jackh726]

Thanks for your patience on this review!

@bors r+

[bors]

📌 Commit 26bd86d has been approved by jackh726

It is now in the queue for this repository.

[sethp]

Thanks @jackh726 for your help and guidance! star wars voice almooooost there, stay on target