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

ci: run mir-opt tests on PR CI also as 32-bit (for EMIT_MIR_FOR_EACH_BIT_WIDTH). #70989

Merged
merged 5 commits into from
Apr 13, 2020
Merged
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
19 changes: 18 additions & 1 deletion src/ci/docker/x86_64-gnu-llvm-7/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ FROM ubuntu:18.04

RUN apt-get update && apt-get install -y --no-install-recommends \
g++ \
g++-arm-linux-gnueabi \
make \
file \
curl \
Expand Down Expand Up @@ -29,7 +30,23 @@ ENV RUST_CONFIGURE_ARGS \
--enable-llvm-link-shared \
--set rust.thin-lto-import-instr-limit=10

ENV SCRIPT python2.7 ../x.py test --exclude src/tools/tidy && python2.7 ../x.py test src/tools/tidy
ENV SCRIPT python2.7 ../x.py test --exclude src/tools/tidy && \
# Run the `mir-opt` tests again but this time for a 32-bit target.
# This enforces that tests using `// EMIT_MIR_FOR_EACH_BIT_WIDTH` have
# both 32-bit and 64-bit outputs updated by the PR author, before
# the PR is approved and tested for merging.
# It will also detect tests lacking `// EMIT_MIR_FOR_EACH_BIT_WIDTH`,
# despite having different output on 32-bit vs 64-bit targets.
#
# HACK(eddyb) `armv5te` is used (not `i686`) to support older LLVM than LLVM 9:
# https://github.com/rust-lang/compiler-builtins/pull/311#issuecomment-612270089.
# This also requires `--pass=build` because we can't execute the tests
# on the `x86_64` host when they're built as `armv5te` binaries.
# (we're only interested in the MIR output, so this doesn't matter)
python2.7 ../x.py test src/test/mir-opt --pass=build \
--target=armv5te-unknown-linux-gnueabi && \
# Run tidy at the very end, after all the other tests.
python2.7 ../x.py test src/tools/tidy

# The purpose of this container isn't to test with debug assertions and
# this is run on all PRs, so let's get speedier builds by disabling these extra
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/inline/inline-into-box-place.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// ignore-wasm32-bare compiled with panic=abort by default
// compile-flags: -Z mir-opt-level=3
// only-64bit FIXME: the mir representation of RawVec depends on ptr size
// EMIT_MIR_FOR_EACH_BIT_WIDTH
Copy link
Member

Choose a reason for hiding this comment

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

Is this already implemented somewhere in a prior PR?

Can we also avoid shouting and use the same snake-case as is used for other compiletest options?

Copy link
Member Author

Choose a reason for hiding this comment

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

The annotation is from #69916 which was copying the style of the old mir-opt tests.

I've only added it here to get rid of the stray only-64bit (all other mir-opt tests that have platform differences use // EMIT_MIR_FOR_EACH_BIT_WIDTH already)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, okay, then fine by me. I would like to see the shouting removed though, seems odd :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I agree, I think the compiletest precedent should be stronger than the old mir-opt testing (which #70721 already removed). Maybe someone from @rust-lang/wg-mir-opt wants to play around with this.

(The implementation is also ad-hoc, it doesn't use the standard compiletest directives, we'd want to fix that too).

Copy link
Member

Choose a reason for hiding this comment

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

Also is there documentation somewhere on the mir-opt format? I checked rustc-dev-guide, did not find anything.

#![feature(box_syntax)]

// EMIT_MIR rustc.main.Inline.diff
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
- // MIR for `main` before Inline
+ // MIR for `main` after Inline

fn main() -> () {
let mut _0: (); // return place in scope 0 at $DIR/inline-into-box-place.rs:7:11: 7:11
let _1: std::boxed::Box<std::vec::Vec<u32>> as UserTypeProjection { base: UserType(0), projs: [] }; // in scope 0 at $DIR/inline-into-box-place.rs:8:9: 8:11
let mut _2: std::boxed::Box<std::vec::Vec<u32>>; // in scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43
let mut _3: (); // in scope 0 at $DIR/inline-into-box-place.rs:8:42: 8:43
+ let mut _4: &mut std::vec::Vec<u32>; // in scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
scope 1 {
debug _x => _1; // in scope 1 at $DIR/inline-into-box-place.rs:8:9: 8:11
}
+ scope 2 {
+ }

bb0: {
StorageLive(_1); // bb0[0]: scope 0 at $DIR/inline-into-box-place.rs:8:9: 8:11
StorageLive(_2); // bb0[1]: scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43
_2 = Box(std::vec::Vec<u32>); // bb0[2]: scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43
- (*_2) = const std::vec::Vec::<u32>::new() -> [return: bb2, unwind: bb4]; // bb0[3]: scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
+ _4 = &mut (*_2); // bb0[3]: scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
+ ((*_4).0: alloc::raw_vec::RawVec<u32>) = const ByRef { alloc: Allocation { bytes: [4, 0, 0, 0, 0, 0, 0, 0], relocations: Relocations(SortedMap { data: [] }), undef_mask: UndefMask { blocks: [255], len: Size { raw: 8 } }, size: Size { raw: 8 }, align: Align { pow2: 2 }, mutability: Not, extra: () }, offset: Size { raw: 0 } }: alloc::raw_vec::RawVec::<u32>; // bb0[4]: scope 2 at $SRC_DIR/liballoc/vec.rs:LL:COL
// ty::Const
- // + ty: fn() -> std::vec::Vec<u32> {std::vec::Vec::<u32>::new}
- // + val: Value(Scalar(<ZST>))
+ // + ty: alloc::raw_vec::RawVec<u32>
+ // + val: Value(ByRef { alloc: Allocation { bytes: [4, 0, 0, 0, 0, 0, 0, 0], relocations: Relocations(SortedMap { data: [] }), undef_mask: UndefMask { blocks: [255], len: Size { raw: 8 } }, size: Size { raw: 8 }, align: Align { pow2: 2 }, mutability: Not, extra: () }, offset: Size { raw: 0 } })
// mir::Constant
- // + span: $DIR/inline-into-box-place.rs:8:33: 8:41
- // + user_ty: UserType(1)
- // + literal: Const { ty: fn() -> std::vec::Vec<u32> {std::vec::Vec::<u32>::new}, val: Value(Scalar(<ZST>)) }
+ // + span: $SRC_DIR/liballoc/vec.rs:LL:COL
+ // + user_ty: UserType(0)
+ // + literal: Const { ty: alloc::raw_vec::RawVec<u32>, val: Value(ByRef { alloc: Allocation { bytes: [4, 0, 0, 0, 0, 0, 0, 0], relocations: Relocations(SortedMap { data: [] }), undef_mask: UndefMask { blocks: [255], len: Size { raw: 8 } }, size: Size { raw: 8 }, align: Align { pow2: 2 }, mutability: Not, extra: () }, offset: Size { raw: 0 } }) }
+ ((*_4).1: usize) = const 0usize; // bb0[5]: scope 2 at $SRC_DIR/liballoc/vec.rs:LL:COL
+ // ty::Const
+ // + ty: usize
+ // + val: Value(Scalar(0x00000000))
+ // mir::Constant
+ // + span: $SRC_DIR/liballoc/vec.rs:LL:COL
+ // + literal: Const { ty: usize, val: Value(Scalar(0x00000000)) }
+ _1 = move _2; // bb0[6]: scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43
+ StorageDead(_2); // bb0[7]: scope 0 at $DIR/inline-into-box-place.rs:8:42: 8:43
+ _0 = (); // bb0[8]: scope 0 at $DIR/inline-into-box-place.rs:7:11: 9:2
+ drop(_1) -> [return: bb2, unwind: bb1]; // bb0[9]: scope 0 at $DIR/inline-into-box-place.rs:9:1: 9:2
}

bb1 (cleanup): {
resume; // bb1[0]: scope 0 at $DIR/inline-into-box-place.rs:7:1: 9:2
}

bb2: {
- _1 = move _2; // bb2[0]: scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43
- StorageDead(_2); // bb2[1]: scope 0 at $DIR/inline-into-box-place.rs:8:42: 8:43
- _0 = (); // bb2[2]: scope 0 at $DIR/inline-into-box-place.rs:7:11: 9:2
- drop(_1) -> [return: bb3, unwind: bb1]; // bb2[3]: scope 0 at $DIR/inline-into-box-place.rs:9:1: 9:2
- }
-
- bb3: {
- StorageDead(_1); // bb3[0]: scope 0 at $DIR/inline-into-box-place.rs:9:1: 9:2
- return; // bb3[1]: scope 0 at $DIR/inline-into-box-place.rs:9:2: 9:2
- }
-
- bb4 (cleanup): {
- _3 = const alloc::alloc::box_free::<std::vec::Vec<u32>>(move (_2.0: std::ptr::Unique<std::vec::Vec<u32>>)) -> bb1; // bb4[0]: scope 0 at $DIR/inline-into-box-place.rs:8:42: 8:43
- // ty::Const
- // + ty: unsafe fn(std::ptr::Unique<std::vec::Vec<u32>>) {alloc::alloc::box_free::<std::vec::Vec<u32>>}
- // + val: Value(Scalar(<ZST>))
- // mir::Constant
- // + span: $DIR/inline-into-box-place.rs:8:42: 8:43
- // + literal: Const { ty: unsafe fn(std::ptr::Unique<std::vec::Vec<u32>>) {alloc::alloc::box_free::<std::vec::Vec<u32>>}, val: Value(Scalar(<ZST>)) }
+ StorageDead(_1); // bb2[0]: scope 0 at $DIR/inline-into-box-place.rs:9:1: 9:2
+ return; // bb2[1]: scope 0 at $DIR/inline-into-box-place.rs:9:2: 9:2
}
}

1 change: 1 addition & 0 deletions src/test/ui/issues/issue-69841.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// LLVM bug which needed a fix to be backported.

// run-pass
// no-system-llvm

fn main() {
let buffer = [49u8, 10];
Expand Down
23 changes: 15 additions & 8 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,14 +357,15 @@ impl<'test> TestCx<'test> {
Ui if pm == Some(PassMode::Run) || self.props.fail_mode == Some(FailMode::Run) => {
WillExecute::Yes
}
Ui => WillExecute::No,
MirOpt if pm == Some(PassMode::Run) => WillExecute::Yes,
Ui | MirOpt => WillExecute::No,
mode => panic!("unimplemented for mode {:?}", mode),
}
}

fn should_run_successfully(&self, pm: Option<PassMode>) -> bool {
match self.config.mode {
Ui => pm == Some(PassMode::Run),
Ui | MirOpt => pm == Some(PassMode::Run),
mode => panic!("unimplemented for mode {:?}", mode),
}
}
Expand Down Expand Up @@ -3057,18 +3058,24 @@ impl<'test> TestCx<'test> {
}

fn run_mir_opt_test(&self) {
let proc_res = self.compile_test(WillExecute::Yes, EmitMetadata::No);
let pm = self.pass_mode();
let should_run = self.should_run(pm);
let emit_metadata = self.should_emit_metadata(pm);
let proc_res = self.compile_test(should_run, emit_metadata);

if !proc_res.status.success() {
self.fatal_proc_rec("compilation failed!", &proc_res);
}

let proc_res = self.exec_compiled_test();
self.check_mir_dump();

if !proc_res.status.success() {
self.fatal_proc_rec("test run failed!", &proc_res);
if let WillExecute::Yes = should_run {
let proc_res = self.exec_compiled_test();

if !proc_res.status.success() {
self.fatal_proc_rec("test run failed!", &proc_res);
}
}
self.check_mir_dump();
}

fn check_mir_dump(&self) {
Expand Down Expand Up @@ -3146,7 +3153,7 @@ impl<'test> TestCx<'test> {
}
let expected_string = fs::read_to_string(&expected_file).unwrap();
if dumped_string != expected_string {
print_diff(&dumped_string, &expected_string, 3);
print_diff(&expected_string, &dumped_string, 3);
panic!(
"Actual MIR output differs from expected MIR output {}",
expected_file.display()
Expand Down