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 5 pull requests #132399

Closed
wants to merge 23 commits into from

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

koute and others added 23 commits October 30, 2024 20:28
To catch at least one pattern that was miscompiled. The test is a
minimization of the MCVE reported in
<rust-lang#132353>.
The initial `minicore` is intentionally super minimal and contains an
incomplete subset of `core` items, and explicitly not items from `alloc`
or `std`-only items.
…lude when requested via `//@ add-core-stubs` directive

`//@ add-core-stubs` will imply `-Cpanic=abort`.
Check that `minicore` cannot be used with `run-{pass,fail}` tests.
Add `minicore` test auxiliary and support `//@ add-core-stubs` directive in ui/assembly/codegen tests

Context: [Real cross-compiling tests instead of `#![no_core]` silliness rust-lang#130375](rust-lang#130375)
MCP: rust-lang/compiler-team#786
Tracking issue: rust-lang#131485

This prototype PR is subject to further changes based on feedback.

### New `minicore` test auxiliary and `//@ add-core-stubs` compiletest directive

This PR introduces a prototype implementation of a `minicore` auxiliary test helper that provides `core` stubs for `#![no_core]` ui/assembly/codegen tests that need to build but not run on both the host platform and the cross-compiled target platform.

Key summary:

- `tests/auxiliary/minicore.rs` contains stub definitions of `core` items intended for consumption by `check-pass`/`build-pass` tests that want the typical prelude items like `Copy` to be stubbed out under `#![no_core]` scenarios, so that the test can be built (not run) for cross-compiled target platforms. Such tests don't want nor need full `-Z build-std` (e.g. `tests/ui/abi/compatibility.rs`).
- `minicore` is intended for `core` items **only**, not `std`- or `alloc`-exclusive items. If stubs for `alloc` or `std` are wanted, they should be provided by an additional directive and test auxiliary, and not be conflated with `minicore` or `core` stubs. This is because a wider range of tests can benefit from `core`-only stubs.

### Implementation

- The `minicore` auxiliary is a single source file `tests/auxiliary/minicore.rs`.
- The path to `minicore` is made avaiable from bootstrap to compiletest via the `--minicore-path` compiletest flag.
- `minicore` is then built on-demand via the `//@ add-core-stubs` compiletest directive, for each test revision for the given target platform (this distinction is important for when host platform != target platform in cross-compilation scenario).
- `minicore` is then made available to the test as an [extern prelude].

[extern prelude]: https://doc.rust-lang.org/reference/names/preludes.html#extern-prelude

### Example usage

```rs
// tests/ui/abi/my-abi-test.rs

//@ check-pass
//@ add-core-stubs
//@ compile-flags: --target i686-unknown-linux-gnu
//@ needs-llvm-components: x86

#![feature(no_core, lang_items)]
#![no_std]
#![no_core]
#![allow(unused, internal_features)]

extern crate minicore;
use minicore::*;

#[lang = "clone"]
pub trait Clone: Sized {      // `Sized` is provided by `minicore`
    fn clone(&self) -> Self;
}
```

### Implementation steps

- [x] 1. Add an initial `minicore` test auxiliary.
- [x] 2. Build `minicore` in bootstrap.
- [x] 3. Setup a `--minicore-path` compiletest cli flag and pass `minicore` build artifact path from bootstrap to compiletest.
- [x] 4. Assert `add-core-stubs` is mutually incompatible with tests that require to be `run`, as the stubs are only good for tests that only need to be built (i.e. no `run-{pass,fail}`).
- [x] 5. Add some self-tests to sanity check the behavior.
- [x] 6. Ensure that `tests/auxiliary/minicore.rs` is input stamped, i.e. modifying `tests/auxiliary/minicore.rs` should invalidate test cache and force the test to be rerun.

### Known limitations

- The current `minicore` is very minimal, because this PR is intended to focus on supporting the test infrastructure first. Further stubs could be added in follow-up PRs and/or on a as-needed basis.

try-job: aarch64-apple
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: test-various
try-job: dist-various-1
… r=Mark-Simulacrum

CI: use free runners for 3 fast windows jobs

try-job: dist-i686-msvc
try-job: dist-i686-mingw
try-job: dist-x86_64-mingw
try-job: dist-x86_64-msvc-alt
Add `lp64e` RISC-V ABI

This PR adds support for the `lp64e` RISC-V ABI, which is the 64-bit equivalent of the `ilp32e` ABI that is already supported.

For reference, this ABI was originally added to LLVM in [this PR](https://reviews.llvm.org/D70401).
…e_to_copy, r=cjgillot,DianQK

Mark `simplify_aggregate_to_copy` mir-opt as unsound

Mark the `simplify_aggregate_to_copy` mir-opt added in rust-lang#128299 as unsound as it seems to miscompile the MCVE reported in rust-lang#132353. The mir-opt can be re-enabled once this case is fixed.

```rs
fn pop_min(mut score2head: Vec<Option<usize>>) -> Option<usize> {
    loop {
        if let Some(col) = score2head[0] {
            score2head[0] = None;
            return Some(col);
        }
    }
}

fn main() {
    let min = pop_min(vec![Some(1)]);
    println!("min: {:?}", min);
    // panic happens here on beta in release mode
    // but not in debug mode
    min.unwrap();
}
```

This MCVE is included as a `run-pass` ui regression test in the first commit. I built the ui test with a nightly manually, and can reproduce the behavioral difference with `-C opt-level=0` and `-C opt-level=1`. Locally, this ui test will fail unless it was run on a compiler built with the second commit marking the mir-opt as unsound thus disabling it by default.

This PR **partially reverts** commit e7386b3, reversing changes made to 02b1be1. The mir-opt implementation is just marked as unsound but **not** reverted to make reland reviews easier. The test changes are **fully** reverted.

cc `@DianQK` `@cjgillot` (PR author and reviewer of rust-lang#128299)
coverage: Avoid ICE when `coverage_cx` is unexpectedly unavailable

In rust-lang#132124, `coverage_cx()` was changed to panic if the context was unavailable, under the assumption that it would always be available whenever coverage instrumentation is enabled.

However, there have been reports of this change causing ICEs in `polars` CI.

I don't yet understand why this is happening, but for now it seems wisest to revert that part of the change, restoring the two early returns that had been replaced with panics.
@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Oct 31, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Oct 31, 2024

📌 Commit d01f8e2 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 Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc 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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants