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

Add minicore test auxiliary and support //@ use-minicore directive in ui/assembly/codegen tests #130693

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1725,6 +1725,11 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
cmd.arg("--run-lib-path").arg(builder.sysroot_libdir(compiler, target));
cmd.arg("--rustc-path").arg(builder.rustc(compiler));

// Minicore auxiliary lib for `no_core` tests that need `core` stubs in cross-compilation
// scenarios.
cmd.arg("--minicore-path")
.arg(builder.src.join("tests").join("auxiliary").join("minicore.rs"));

let is_rustdoc = suite.ends_with("rustdoc-ui") || suite.ends_with("rustdoc-js");

if mode == "run-make" {
Expand Down
1 change: 1 addition & 0 deletions src/tools/compiletest/src/command-list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,5 +228,6 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"unset-rustc-env",
// Used by the tidy check `unknown_revision`.
"unused-revision-names",
"use-minicore",
// tidy-alphabetical-end
];
5 changes: 5 additions & 0 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,11 @@ pub struct Config {
/// True if the profiler runtime is enabled for this target.
/// Used by the "needs-profiler-runtime" directive in test files.
pub profiler_runtime: bool,

/// Path to minicore aux library, used for `no_core` tests that need `core` stubs in
/// cross-compilation scenarios that do not otherwise want/need to `-Zbuild-std`. Used in e.g.
/// ABI tests.
pub minicore_path: PathBuf,
}

impl Config {
Expand Down
26 changes: 26 additions & 0 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,9 @@ pub struct TestProps {
pub no_auto_check_cfg: bool,
/// Run tests which require enzyme being build
pub has_enzyme: bool,
/// Build and use `minicore` as `core` stub for `no_core` tests in cross-compilation scenarios
/// that don't otherwise want/need `-Z build-std`.
pub use_minicore: bool,
}

mod directives {
Expand Down Expand Up @@ -242,6 +245,7 @@ mod directives {
pub const LLVM_COV_FLAGS: &'static str = "llvm-cov-flags";
pub const FILECHECK_FLAGS: &'static str = "filecheck-flags";
pub const NO_AUTO_CHECK_CFG: &'static str = "no-auto-check-cfg";
pub const USE_MINICORE: &'static str = "use-minicore";
// This isn't a real directive, just one that is probably mistyped often
pub const INCORRECT_COMPILER_FLAGS: &'static str = "compiler-flags";
}
Expand Down Expand Up @@ -299,6 +303,7 @@ impl TestProps {
filecheck_flags: vec![],
no_auto_check_cfg: false,
has_enzyme: false,
use_minicore: false,
}
}

Expand Down Expand Up @@ -563,6 +568,8 @@ impl TestProps {
}

config.set_name_directive(ln, NO_AUTO_CHECK_CFG, &mut self.no_auto_check_cfg);

self.update_use_minicore(ln, config);
},
);

Expand Down Expand Up @@ -676,6 +683,25 @@ impl TestProps {
pub fn local_pass_mode(&self) -> Option<PassMode> {
self.pass_mode
}

pub fn update_use_minicore(&mut self, ln: &str, config: &Config) {
let use_minicore = config.parse_name_directive(ln, directives::USE_MINICORE);
if use_minicore {
if !matches!(config.mode, Mode::Ui | Mode::Codegen | Mode::Assembly) {
panic!("`use-minicore` is only supported for ui, codegen and assembly test modes");
}

// FIXME(jieyouxu): this check is currently order-dependent, but we should probably
// collect all directives in one go then perform a validation pass after that.
if self.local_pass_mode().is_some_and(|pm| pm == PassMode::Run) {
// `minicore` can only be used with non-run modes, because it's `core` prelude stubs
// and can't run.
panic!("`use-minicore` core stubs cannot be used to run the test binary");
}

self.use_minicore = use_minicore;
}
}
}

/// Extract an `(Option<line_revision>, directive)` directive from a line if comment is present.
Expand Down
1 change: 1 addition & 0 deletions src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ impl ConfigBuilder {
"--git-repository=",
"--nightly-branch=",
"--git-merge-commit-email=",
"--minicore-path=",
];
let mut args: Vec<String> = args.iter().map(ToString::to_string).collect();

Expand Down
12 changes: 11 additions & 1 deletion src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ pub fn parse_config(args: Vec<String>) -> Config {
"git-merge-commit-email",
"email address used for finding merge commits",
"EMAIL",
);
)
.reqopt("", "minicore-path", "path to minicore aux library", "PATH");

let (argv0, args_) = args.split_first().unwrap();
if args.len() == 1 || args[1] == "-h" || args[1] == "--help" {
Expand Down Expand Up @@ -356,6 +357,8 @@ pub fn parse_config(args: Vec<String>) -> Config {
git_merge_commit_email: matches.opt_str("git-merge-commit-email").unwrap(),

profiler_runtime: matches.opt_present("profiler-runtime"),

minicore_path: opt_path(matches, "minicore-path"),
}
}

Expand Down Expand Up @@ -393,6 +396,7 @@ pub fn log_config(config: &Config) {
logv(c, format!("host-linker: {:?}", config.host_linker));
logv(c, format!("verbose: {}", config.verbose));
logv(c, format!("format: {:?}", config.format));
logv(c, format!("minicore_path: {:?}", config.minicore_path.display()));
logv(c, "\n".to_string());
}

Expand Down Expand Up @@ -874,6 +878,12 @@ fn files_related_to_test(
related.push(path);
}

// `minicore.rs` test auxiliary: we need to make sure tests get rerun if this changes.
//
// FIXME(jieyouxu): untangle these paths, we should provide both a path to root `tests/` or
// `tests/auxiliary/` and the test suite in question. `src_base` is also a terrible name.
related.push(config.src_base.parent().unwrap().join("auxiliary").join("minicore.rs"));

related
}

Expand Down
78 changes: 48 additions & 30 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1106,14 +1106,20 @@ impl<'test> TestCx<'test> {
}
}

/// `root_testpaths` refers to the path of the original test.
/// the auxiliary and the test with an aux-build have the same `root_testpaths`.
/// `root_testpaths` refers to the path of the original test. the auxiliary and the test with an
/// aux-build have the same `root_testpaths`.
fn compose_and_run_compiler(
&self,
mut rustc: Command,
input: Option<String>,
root_testpaths: &TestPaths,
) -> ProcRes {
if self.props.use_minicore {
let minicore_path = self.build_minicore();
rustc.arg("--extern");
rustc.arg(&format!("minicore={}", minicore_path.to_str().unwrap()));
}

let aux_dir = self.aux_output_dir();
self.build_all_auxiliary(root_testpaths, &aux_dir, &mut rustc);

Expand All @@ -1127,6 +1133,37 @@ impl<'test> TestCx<'test> {
)
}

/// Builds `minicore`. Returns the path to the minicore rlib within the base test output
/// directory.
fn build_minicore(&self) -> PathBuf {
Copy link
Member Author

Choose a reason for hiding this comment

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

Remark: I don't like how convoluted the top-level runtest logic is, but intended for future compiletest cleanup PRs instead of this PR.

let output_file_path = self.output_base_dir().join("libminicore.rlib");
let mut rustc = self.make_compile_args(
&self.config.minicore_path,
TargetLocation::ThisFile(output_file_path.clone()),
Emit::None,
AllowUnused::Yes,
LinkToAux::No,
vec![],
);

rustc.args(&["--crate-type", "rlib"]);
rustc.arg("-Cpanic=abort");

let res =
self.compose_and_run(rustc, self.config.compile_lib_path.to_str().unwrap(), None, None);
if !res.status.success() {
self.fatal_proc_rec(
&format!(
"auxiliary build of {:?} failed to compile: ",
self.config.minicore_path.display()
),
&res,
);
}

output_file_path
}

/// Builds an aux dependency.
fn build_auxiliary(
&self,
Expand Down Expand Up @@ -1614,6 +1651,15 @@ impl<'test> TestCx<'test> {

rustc.args(&self.props.compile_flags);

// FIXME(jieyouxu): we should report a fatal error or warning if user wrote `-Cpanic=` with
// something that's not `abort`, however, by moving this last we should override previous
// `-Cpanic=`s
//
// `minicore` requires `#![no_std]` and `#![no_core]`, which means no unwinding panics.
if self.props.use_minicore {
rustc.arg("-Cpanic=abort");
}

rustc
}

Expand Down Expand Up @@ -1798,34 +1844,6 @@ impl<'test> TestCx<'test> {
(proc_res, output_path)
}

fn compile_test_and_save_assembly(&self) -> (ProcRes, PathBuf) {
// This works with both `--emit asm` (as default output name for the assembly)
// and `ptx-linker` because the latter can write output at requested location.
let output_path = self.output_base_name().with_extension("s");
let input_file = &self.testpaths.file;

// Use the `//@ assembly-output:` directive to determine how to emit assembly.
let emit = match self.props.assembly_output.as_deref() {
Some("emit-asm") => Emit::Asm,
Some("bpf-linker") => Emit::LinkArgsAsm,
Some("ptx-linker") => Emit::None, // No extra flags needed.
Some(other) => self.fatal(&format!("unknown 'assembly-output' directive: {other}")),
None => self.fatal("missing 'assembly-output' directive"),
};

let rustc = self.make_compile_args(
input_file,
TargetLocation::ThisFile(output_path.clone()),
emit,
AllowUnused::No,
LinkToAux::Yes,
Vec::new(),
);

let proc_res = self.compose_and_run_compiler(rustc, None, self.testpaths);
(proc_res, output_path)
}

fn verify_with_filecheck(&self, output: &Path) -> ProcRes {
let mut filecheck = Command::new(self.config.llvm_filecheck.as_ref().unwrap());
filecheck.arg("--input-file").arg(output).arg(&self.testpaths.file);
Expand Down
32 changes: 31 additions & 1 deletion src/tools/compiletest/src/runtest/assembly.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use super::TestCx;
use std::path::PathBuf;

use super::{AllowUnused, Emit, LinkToAux, ProcRes, TargetLocation, TestCx};

impl TestCx<'_> {
pub(super) fn run_assembly_test(&self) {
Expand All @@ -16,4 +18,32 @@ impl TestCx<'_> {
self.fatal_proc_rec("verification with 'FileCheck' failed", &proc_res);
}
}

fn compile_test_and_save_assembly(&self) -> (ProcRes, PathBuf) {
// This works with both `--emit asm` (as default output name for the assembly)
// and `ptx-linker` because the latter can write output at requested location.
let output_path = self.output_base_name().with_extension("s");
let input_file = &self.testpaths.file;

// Use the `//@ assembly-output:` directive to determine how to emit assembly.
let emit = match self.props.assembly_output.as_deref() {
Some("emit-asm") => Emit::Asm,
Some("bpf-linker") => Emit::LinkArgsAsm,
Some("ptx-linker") => Emit::None, // No extra flags needed.
Some(other) => self.fatal(&format!("unknown 'assembly-output' directive: {other}")),
None => self.fatal("missing 'assembly-output' directive"),
};

let rustc = self.make_compile_args(
input_file,
TargetLocation::ThisFile(output_path.clone()),
emit,
AllowUnused::No,
LinkToAux::Yes,
Vec::new(),
);

let proc_res = self.compose_and_run_compiler(rustc, None, self.testpaths);
(proc_res, output_path)
}
}
5 changes: 5 additions & 0 deletions tests/assembly/compiletest-self-test/use-minicore-no-run.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
//! `compiletest` self-test to check that `use-minicore` is incompatible with run pass modes.

//@ use-minicore
//@ run-pass
//@ should-fail
80 changes: 80 additions & 0 deletions tests/auxiliary/minicore.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
//! Auxiliary `minicore` prelude which stubs out `core` items for `no_core` tests that need to work
//! in cross-compilation scenarios where no `core` is available (that don't want nor need to
//! `-Zbuild-std`).
//!
//! # Important notes
//!
//! - `minicore` is **only** intended for `core` items, and the stubs should match the actual `core`
//! items.
//!
//! # References
//!
//! This is partially adapted from `rustc_codegen_cranelift`:
//! <https://github.com/rust-lang/rust/blob/c0b5cc9003f6464c11ae1c0662c6a7e06f6f5cab/compiler/rustc_codegen_cranelift/example/mini_core.rs>.
// ignore-tidy-linelength

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

#[lang = "sized"]
pub trait Sized {}

#[lang = "receiver"]
pub trait Receiver {}
impl<T: ?Sized> Receiver for &T {}
impl<T: ?Sized> Receiver for &mut T {}

#[lang = "copy"]
pub trait Copy {}

impl Copy for bool {}
impl Copy for u8 {}
impl Copy for u16 {}
impl Copy for u32 {}
impl Copy for u64 {}
impl Copy for u128 {}
impl Copy for usize {}
impl Copy for i8 {}
impl Copy for i16 {}
impl Copy for i32 {}
impl Copy for isize {}
impl Copy for f32 {}
impl Copy for f64 {}
impl Copy for char {}
impl<'a, T: ?Sized> Copy for &'a T {}
impl<T: ?Sized> Copy for *const T {}
impl<T: ?Sized> Copy for *mut T {}

#[lang = "phantom_data"]
pub struct PhantomData<T: ?Sized>;
impl<T: ?Sized> Copy for PhantomData<T> {}

pub enum Option<T> {
None,
Some(T),
}
impl<T: Copy> Copy for Option<T> {}

pub enum Result<T, E> {
Ok(T),
Err(E),
}
impl<T: Copy, E: Copy> Copy for Result<T, E> {}

#[lang = "manually_drop"]
#[repr(transparent)]
pub struct ManuallyDrop<T: ?Sized> {
value: T,
}
impl<T: Copy + ?Sized> Copy for ManuallyDrop<T> {}
Copy link
Member

@bjorn3 bjorn3 Sep 22, 2024

Choose a reason for hiding this comment

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

You can derive Copy if you add

#[rustc_builtin_macro]
pub macro Copy($item:item) {
    /* compiler built-in */
}

and the same applies to Clone.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not make this change because I couldn't figure out how to make use of this, just kept manual impl Copy for ... for now.


#[lang = "unsafe_cell"]
#[repr(transparent)]
pub struct UnsafeCell<T: ?Sized> {
value: T,
}

// Trait stub, no `type_id` method.
pub trait Any: 'static {}
Loading
Loading