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

Rollup of 6 pull requests #114673

Merged
merged 18 commits into from
Aug 10, 2023
Merged

Rollup of 6 pull requests #114673

merged 18 commits into from
Aug 10, 2023

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Enselic and others added 18 commits August 2, 2023 20:57
For better failure messages.
```
error[E0599]: no method named `x` found for struct `Pin<&S>` in the current scope
  --> $DIR/arbitrary_self_type_mut_difference.rs:11:18
   |
LL |     Pin::new(&S).x();
   |                  ^ help: there is a method with a similar name: `y`
   |
note: method is available for `Pin<&mut S>`
  --> $DIR/arbitrary_self_type_mut_difference.rs:6:5
   |
LL |     fn x(self: Pin<&mut Self>) {}
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
```

Related to rust-lang#57994, as one of the presented cases can lead to code like
this.
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
These new interrupt calling conventions are not themselves stabilized,
but there are other unstable calling conventions present in the SMIR
mapping (e.g. AVR interrupts) and the mapping appears to be "complete"
so far, with no obvious way to represent unstable conventions separately
from the stable ones.
The change in 07f855d introduced a
trailing numeral of some kind after the `extern crate
compiler_builtins`, which appears to have caused at least two false
negatives (654b924 and 657fd24). Instead, this change normalizes the
test output to ignore the number (of symbols rustc recognizes?) to avoid
needing to re-`--bless` these two tests for unrelated changes.
…r=GuillaumeGomez

rustdoc-json: Add test for field ordering.

Inspired by [this on twitter](https://twitter.com/PredragGruevski/status/1647705616650043392), the ordering of fields really matters, so we should test we preserve it through json.

r? rustdoc
…ckh726

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

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
…tf-8, r=thomcc

test_get_dbpath_for_term(): handle non-utf8 paths (fix FIXME)

Removes a FIXME for rust-lang#9639

Part of rust-lang#44366 which is E-help-wanted

The remaining two FIXMEs for rust-lang#9639 are considerably more complicated, so I will create separate PRs for them.
…diff, r=davidtwco

Detect method not found on arbitrary self type with different mutability

```
error[E0599]: no method named `x` found for struct `Pin<&S>` in the current scope
  --> $DIR/arbitrary_self_type_mut_difference.rs:11:18
   |
LL |     Pin::new(&S).x();
   |                  ^ help: there is a method with a similar name: `y`
   |
note: method is available for `Pin<&mut S>`
  --> $DIR/arbitrary_self_type_mut_difference.rs:6:5
   |
LL |     fn x(self: Pin<&mut Self>) {}
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
```

Related to rust-lang#57994, as one of the presented cases can lead to code like this.
Convert Const to Allocation in smir

Continuation of previous pr rust-lang#114466

cc rust-lang/project-stable-mir#15

r? `@oli-obk`
…gillot

Don't use `type_of` to determine if item has intrinsic shim

When we're calling `resolve_instance` on an inline const, we were previously looking at the `type_of` for that const, seeing that it was an `extern "intrinsic"` fn def, and treating it as if we were computing the instance of that intrinsic itself. This is incorrect.

Instead, we should be using the def-id of the item we're computing to determine if it's an intrinsic.

Fixes rust-lang#114660
@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Aug 9, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=6

@bors
Copy link
Contributor

bors commented Aug 9, 2023

📌 Commit a87dda3 has been approved by matthiaskrgr

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 Aug 9, 2023
@bors
Copy link
Contributor

bors commented Aug 9, 2023

⌛ Testing commit a87dda3 with merge 832db2f...

@bors
Copy link
Contributor

bors commented Aug 10, 2023

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 832db2f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 10, 2023
@bors bors merged commit 832db2f into rust-lang:master Aug 10, 2023
@rustbot rustbot added this to the 1.73.0 milestone Aug 10, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#110435 rustdoc-json: Add test for field ordering. 40a18d58687ab93c53a341d3e163cb7db60a6e48 (link)
#111891 feat: riscv-interrupt-{m,s} calling conventions aaf0e6b8278d88e0660f1c4d6b5221cea5ca3bdd (link)
#114377 test_get_dbpath_for_term(): handle non-utf8 paths (fix FIXM… 9b268f52bf6d6c4cf04a6be13026a899f1b2a0f0 (link)
#114469 Detect method not found on arbitrary self type with differe… 80ef68b8564d92da8e6f4a0ebf719ac5c502354d (link)
#114587 Convert Const to Allocation in smir a2bba95921521045cdfff4df3e2fb16530f36437 (link)
#114670 Don't use type_of to determine if item has intrinsic shim d8bf62ca0fb817e1d1fd50fafc9b19562ad559a2 (link)

previous master: 08d00b40ae

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (832db2f): comparison URL.

Overall result: ✅ improvements - 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
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.2% [-0.2%, -0.2%] 1
All ❌✅ (primary) - - 0

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)
- - 0
Improvements ✅
(primary)
-0.5% [-0.5%, -0.5%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.5% [-0.5%, -0.5%] 1

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: 632.868s -> 633.758s (0.14%)

bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Sep 6, 2023
…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
@matthiaskrgr matthiaskrgr deleted the rollup-9kroqpp branch March 16, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants