Add FileCheck annotations to mir-opt tests. #116810

cjgillot · 2023-10-16T18:01:02Z

This PR makes compiletest run LLVM FileCheck tool on mir-opt tests.

The check is run by default, except if disabled using // skip-filecheck comment. This ensures that we do not have a silently broken test. For now, the check is only run on the output of --emit=mir, ie. on PreCodegen MIR.

I give an example on reference_prop.rs.

r? @oli-obk
cc @RalfJung

Fixes #85180

rustbot · 2023-10-16T18:01:14Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

rustbot · 2023-10-16T18:09:29Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

src/tools/compiletest/src/runtest.rs

RalfJung · 2023-10-16T18:21:14Z

tests/mir-opt/reference_prop.rs

    // Propagation through a reference.
    {
+        // CHECK: bb0: {


Are these the raw underlying MIR bb numbers or also somehow renumbered?

Raw bb numbers. I haven't bothered renumbering them.

RalfJung · 2023-10-16T18:23:02Z

Wow, that's great. :) I'm surprised this works, how does FileCheck understand our function headers as "labels"? I guess I have no idea what CHECK-LABEL actually does.

Can you extend the mir-opt/README.md with a brief explanation of what one has to know, probably mostly linking to the LLVM FileCheck docs? I have no idea what [[a:_.*]] means. Might be worth documenting that pattern since it seems to be some clever way to deal with the MIR variable numbers?

We probably only want to filecheck tests that have // unit-test:? Otherwise it becomes rather random I feel. So that might also be worth putting into the readme.

bors · 2023-10-18T04:50:40Z

☔ The latest upstream changes (presumably #116505) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk

r=me after a rebase

cjgillot · 2023-10-18T16:58:08Z

Wow, that's great. :) I'm surprised this works, how does FileCheck understand our function headers as "labels"? I guess I have no idea what CHECK-LABEL actually does.

CHECK-LABEL merely looks for the following string. As the output MIR contains a fn foobar(, it works.

Can you extend the mir-opt/README.md with a brief explanation of what one has to know, probably mostly linking to the LLVM FileCheck docs? I have no idea what [[a:_.*]] means. Might be worth documenting that pattern since it seems to be some clever way to deal with the MIR variable numbers?

Done.

We probably only want to filecheck tests that have // unit-test:? Otherwise it becomes rather random I feel. So that might also be worth putting into the readme.

I'm going through all files in the mir-opt suite to add the directives. All should have them, except for the pre-codegen directory which goal is to track the quality of the output MIR. I have already found files that have stopped testing anything. (To be finished in another PR.)

@bors r=oli-obk

bors · 2023-10-18T16:58:10Z

📌 Commit 804aa6a has been approved by oli-obk

It is now in the queue for this repository.

Add FileCheck annotations to mir-opt tests. This PR makes compiletest run LLVM `FileCheck` tool on mir-opt tests. The check is *run by default*, except if disabled using `// skip-filecheck` comment. This ensures that we do not have a silently broken test. For now, the check is only run on the output of `--emit=mir`, ie. on PreCodegen MIR. I give an example on `reference_prop.rs`. r? `@oli-obk` cc `@RalfJung` Fixes rust-lang#85180

Rollup of 4 pull requests Successful merges: - rust-lang#116037 (Add `-Zstack-protector` test for Windows targets) - rust-lang#116132 (Make TCP connect handle EINTR correctly) - rust-lang#116810 (Add FileCheck annotations to mir-opt tests.) - rust-lang#116896 (Only check in a single place if a pass is enabled.) r? `@ghost` `@rustbot` modify labels: rollup

RalfJung · 2023-10-19T06:26:15Z

tests/mir-opt/README.md

+
+`[[name:regex]]` syntax allows to bind `name` to a string matching `regex`, and refer to it
+as `[[name]]` in later directives, `regex` should be written not to match a leading space.
+Use `[[my_local:_.*]]` to name a local, and `[[my_bb:bb.*]]` to name a block.


Would [[my_local:_[0-9]+]] be better to avoid matching random other things that start with _? Same for basic blocks.

That would be more precise yes. However, I don't think there is any risk of matching the wrong thing. Basic-blocks can only appear in very few places. Likewise, I don't know anything that could be mistaken for a local.

fmease · 2023-10-19T12:17:41Z

#116924 (comment)
@bors r-

cjgillot · 2023-10-19T16:33:10Z

@bors r=oli-obk

bors · 2023-10-19T16:33:12Z

📌 Commit 328192b has been approved by oli-obk

It is now in the queue for this repository.

bors · 2023-10-19T16:53:55Z

⌛ Testing commit 328192b with merge e76cb8c...

bors · 2023-10-19T18:45:28Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing e76cb8c to master...

rust-timer · 2023-10-19T20:06:21Z

Finished benchmarking commit (e76cb8c): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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)	3.1%	[2.7%, 3.5%]	2
Regressions ❌ (secondary)	4.8%	[4.8%, 4.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.1%	[2.7%, 3.5%]	2

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.785s -> 629.154s (0.06%)
Artifact size: 304.02 MiB -> 304.03 MiB (0.00%)

rustbot assigned oli-obk Oct 16, 2023

This comment has been minimized.

Sign in to view

cjgillot force-pushed the mir-opt-check branch from 2d8fed2 to e5d4075 Compare October 16, 2023 18:09

RalfJung reviewed Oct 16, 2023

View reviewed changes

src/tools/compiletest/src/runtest.rs Outdated Show resolved Hide resolved

RalfJung reviewed Oct 16, 2023

View reviewed changes

cjgillot mentioned this pull request Oct 16, 2023

Normalize alloc-id in tests. #116767

Merged

cjgillot force-pushed the mir-opt-check branch 2 times, most recently from f287381 to 1560e33 Compare October 17, 2023 18:54

oli-obk approved these changes Oct 18, 2023

View reviewed changes

cjgillot force-pushed the mir-opt-check branch 2 times, most recently from 3ae74ec to 804aa6a Compare October 18, 2023 16:20

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 18, 2023

fmease mentioned this pull request Oct 19, 2023

Rollup of 4 pull requests #116924

Closed

RalfJung reviewed Oct 19, 2023

View reviewed changes

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 19, 2023

cjgillot added 13 commits October 19, 2023 15:51

FileCheck bool_compare.

5453a4f

FileCheck combine_clone_of_primitives.

ddc328c

FileCheck intrinsic_asserts.

f856247

FileCheck duplicate_switch_targets.

386fff3

FileCheck combine_transmutes.

68c409f

FileCheck casts.

d8cffda

FileCheck lower_intrinsics.

4bae847

FileCheck lower_array_len.

ad057d4

FileCheck lower_slice_len.

d24e44a

Mention skip in README.

c679b20

FileCheck issue_106141.

1368355

FileCheck inline_shims.

69d9369

FileCheck transmute.

328192b

cjgillot force-pushed the mir-opt-check branch from 804aa6a to 328192b Compare October 19, 2023 15:52

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 19, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 19, 2023

bors merged commit e76cb8c into rust-lang:master Oct 19, 2023
11 checks passed

rustbot added this to the 1.75.0 milestone Oct 19, 2023

bors mentioned this pull request Oct 19, 2023

Never consider raw pointer casts to be trival #113262

Merged

cjgillot deleted the mir-opt-check branch October 19, 2023 23:52

cjgillot mentioned this pull request Oct 20, 2023

Add FileCheck annotations to MIR-opt tests #116971

Open

github-actions bot mentioned this pull request Oct 31, 2023

Automated pull from upstream master ferrocene/ferrocene#74

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add FileCheck annotations to mir-opt tests. #116810

Add FileCheck annotations to mir-opt tests. #116810

cjgillot commented Oct 16, 2023

rustbot commented Oct 16, 2023

This comment has been minimized.

rustbot commented Oct 16, 2023

RalfJung Oct 16, 2023

cjgillot Oct 16, 2023

RalfJung commented Oct 16, 2023 •

edited

Loading

bors commented Oct 18, 2023

oli-obk left a comment

cjgillot commented Oct 18, 2023

bors commented Oct 18, 2023

RalfJung Oct 19, 2023

cjgillot Oct 19, 2023

fmease commented Oct 19, 2023

cjgillot commented Oct 19, 2023

bors commented Oct 19, 2023

bors commented Oct 19, 2023

bors commented Oct 19, 2023

rust-timer commented Oct 19, 2023

Add FileCheck annotations to mir-opt tests. #116810

Add FileCheck annotations to mir-opt tests. #116810

Conversation

cjgillot commented Oct 16, 2023

rustbot commented Oct 16, 2023

This comment has been minimized.

rustbot commented Oct 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung commented Oct 16, 2023 • edited Loading

bors commented Oct 18, 2023

oli-obk left a comment

Choose a reason for hiding this comment

cjgillot commented Oct 18, 2023

bors commented Oct 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmease commented Oct 19, 2023

cjgillot commented Oct 19, 2023

bors commented Oct 19, 2023

bors commented Oct 19, 2023

bors commented Oct 19, 2023

rust-timer commented Oct 19, 2023

Overall result: no relevant changes - no action needed

RalfJung commented Oct 16, 2023 •

edited

Loading