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

internal: Sync from downstream #15494

Merged
merged 13 commits into from Aug 22, 2023
Merged

internal: Sync from downstream #15494

merged 13 commits into from Aug 22, 2023

Conversation

lnicola
Copy link
Member

@lnicola lnicola commented Aug 21, 2023

No description provided.

Trolldemorted and others added 11 commits August 7, 2023 14:11
stabilize abi_thiscall

Closes rust-lang/rust#42202, stabilizing the use of the "thiscall" ABI.

FCP was substituted by a poll, and the poll has been accepted.
Similar to prior support added for the mips430, avr, and x86 targets
this change implements the rough equivalent of clang's
[`__attribute__((interrupt))`][clang-attr] for riscv targets, enabling
e.g.

```rust
static mut CNT: usize = 0;

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

to produce highly effective assembly like:

```asm
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][full-frame-save]. 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][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).

[clang-attr]: https://clang.llvm.org/docs/AttributeReference.html#interrupt-risc-v
[full-frame-save]: https://github.com/esp-rs/esp-riscv-rt/blob/9281af2ecffe13e40992917316f36920c26acaf3/src/lib.rs#L440-L469
[implemented by]: https://github.com/llvm/llvm-project/blob/b7fb2a3fec7c187d58a6d338ab512d9173bca987/llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp#L61-L67
[callee-save]: https://github.com/llvm/llvm-project/blob/973f1fe7a8591c7af148e573491ab68cc15b6ecf/llvm/lib/Target/RISCV/RISCVCallingConv.td#L30-L37
[rfc]: rust-lang/rfcs#3246
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 21, 2023
@lnicola
Copy link
Member Author

lnicola commented Aug 21, 2023

Not sure what's happening here. The two tests (resolve_proc_macro and out_dirs_check) also fail for me on master, when running CI=1 cargo +nightly test --features sysroot-abi. CI passes because we don't run them most of the time.

@lnicola
Copy link
Member Author

lnicola commented Aug 21, 2023

Huh, it's now working for me after upgrading to the latest nightly.

@lnicola
Copy link
Member Author

lnicola commented Aug 21, 2023

@bors try

@bors
Copy link
Collaborator

bors commented Aug 21, 2023

⌛ Trying commit a4cf405 with merge 7293b17...

bors added a commit that referenced this pull request Aug 21, 2023
@bors
Copy link
Collaborator

bors commented Aug 21, 2023

💔 Test failed - checks-actions

@Veykril
Copy link
Member

Veykril commented Aug 21, 2023

It looks like internal_features was set to deny by default now, so we need to allow that on the test fixture's builds script part of the failing test?

@Veykril
Copy link
Member

Veykril commented Aug 21, 2023

This is the one part thats annoying about upstream CI not running r-a tests, because this is the one integration test we do want to run ...

@lnicola
Copy link
Member Author

lnicola commented Aug 21, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 21, 2023

📌 Commit 6caf79c has been approved by lnicola

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Aug 21, 2023

⌛ Testing commit 6caf79c with merge 8011137...

bors added a commit that referenced this pull request Aug 21, 2023
@bors
Copy link
Collaborator

bors commented Aug 21, 2023

💥 Test timed out

@lnicola
Copy link
Member Author

lnicola commented Aug 22, 2023

I can reproduce the hang on nightly (switch to master, then run analysis-stats on the RA repo). I think we didn't spot this because we usually run it on stable on CI.

It's in const eval, looks like it's not a real hang, but each const takes about 4s.

CC @HKalbasi

@lnicola
Copy link
Member Author

lnicola commented Aug 22, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 22, 2023

📌 Commit b4576b5 has been approved by lnicola

It is now in the queue for this repository.

@lnicola lnicola changed the title minor: Sync from downstream internal: Sync from downstream Aug 22, 2023
@bors
Copy link
Collaborator

bors commented Aug 22, 2023

⌛ Testing commit b4576b5 with merge 7f659a3...

@bors
Copy link
Collaborator

bors commented Aug 22, 2023

☀️ Test successful - checks-actions
Approved by: lnicola
Pushing 7f659a3 to master...

@bors bors merged commit 7f659a3 into rust-lang:master Aug 22, 2023
10 checks passed
@lnicola lnicola deleted the sync-from-rust branch August 22, 2023 08:29
@@ -86,6 +86,12 @@ jobs:
- name: Test
run: cargo test ${{ env.USE_SYSROOT_ABI }} -- --nocapture --quiet

- name: Switch to stable toolchain
Copy link
Member Author

Choose a reason for hiding this comment

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

@Veykril heads-up, just so you're aware of this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants