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

Sync rustc_codegen_cranelift #115616

Merged
merged 24 commits into from
Sep 7, 2023
Merged

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Sep 6, 2023

Not much changed this time. Mostly doing this sync to make it easier to run the entire test suite on the in-tree version.

r? @ghost

@rustbot label +A-codegen +A-cranelift +T-compiler

sethp and others added 24 commits August 8, 2023 18:09
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
Removes a piece of mutable state.
Follow up to rust-lang#114578.
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#110435 (rustdoc-json: Add test for field ordering.)
 - rust-lang#111891 (feat: `riscv-interrupt-{m,s}` calling conventions)
 - rust-lang#114377 (test_get_dbpath_for_term(): handle non-utf8 paths (fix FIXME))
 - rust-lang#114469 (Detect method not found on arbitrary self type with different mutability)
 - rust-lang#114587 (Convert Const to Allocation in smir)
 - rust-lang#114670 (Don't use `type_of` to determine if item has intrinsic shim)

Failed merges:

 - rust-lang#114599 (Add impl trait declarations to SMIR)

r? `@ghost`
`@rustbot` modify labels: rollup
rustc: Move `crate_types` and `stable_crate_id` from `Session` to `GlobalCtxt`

Removes two pieces of mutable state.
Follow up to rust-lang#114578.
As experimentation in 115242 has shown looks better than `coldcc`.

And *don't* use a different convention for cold on Windows, because that actually ends up making things worse.

cc tracking issue 97544
Don't ICE on layout computation failure

Fixes rust-lang#111176 regression.

r? `@oli-obk`
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-codegen Area: Code generation A-cranelift Things relevant to the [future] cranelift backend labels Sep 6, 2023
@bjorn3
Copy link
Member Author

bjorn3 commented Sep 6, 2023

@bors r+ subtree sync

@bors
Copy link
Contributor

bors commented Sep 6, 2023

📌 Commit 521ed6e has been approved by bjorn3

It is now in the queue for this repository.

@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 Sep 6, 2023
@bors
Copy link
Contributor

bors commented Sep 7, 2023

⌛ Testing commit 521ed6e with merge fece303...

@bors
Copy link
Contributor

bors commented Sep 7, 2023

☀️ Test successful - checks-actions
Approved by: bjorn3
Pushing fece303 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 7, 2023
@bors bors merged commit fece303 into rust-lang:master Sep 7, 2023
12 checks passed
@rustbot rustbot added this to the 1.74.0 milestone Sep 7, 2023
@bjorn3 bjorn3 deleted the sync_cg_clif-2023-09-06 branch September 7, 2023 08:00
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (fece303): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.2% [0.2%, 0.2%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% [0.2%, 0.2%] 1

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.2% [2.2%, 2.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 628.823s -> 628.138s (-0.11%)
Artifact size: 317.79 MiB -> 317.88 MiB (0.03%)

bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Oct 9, 2023
…orn3

Sync rustc_codegen_cranelift

Not much changed this time. Mostly doing this sync to make it easier to run the entire test suite on the in-tree version.

r? `@ghost`

`@rustbot` label +A-codegen +A-cranelift +T-compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-cranelift Things relevant to the [future] cranelift backend merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.