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

tests: -Copt-level=3 instead of -O in codegen tests #136761

Merged

Conversation

workingjubilee
Copy link
Member

@workingjubilee workingjubilee commented Feb 9, 2025

An effective blocker for redefining the meaning of -O is to stop reusing this somewhat ambiguous alias in our own codegen test suite. The choice between -Copt-level=2 and -Copt-level=3 is arbitrary for most of our tests. In most cases it makes no difference, so I set most of them to -Copt-level=3, as it will lead to slightly more "normalized" codegen.

try-job: test-various
try-job: arm-android
try-job: armhf-gnu
try-job: i686-gnu-1
try-job: i686-gnu-2
try-job: i686-mingw
try-job: i686-msvc-1
try-job: i686-msvc-2
try-job: aarch64-apple
try-job: aarch64-gnu

@rustbot rustbot added 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. labels Feb 9, 2025
@workingjubilee
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2025
…-codegen-tests, r=<try>

tests: `-Copt-level=3` instead of `-O` in codegen tests

An effective blocker for rust-lang#135439

r? `@ghost`

try-job: test-various
try-job: aarch64-gnu
try-job: aarch64-gnu-debug
try-job: i686-gnu-1
try-job: i686-gnu-2
try-job: i686-mingw
try-job: i686-msvc-1
try-job: i686-msvc-2
try-job: x86_64-msvc-1
try-job: x86_64-msvc-2
@bors
Copy link
Contributor

bors commented Feb 9, 2025

⌛ Trying commit ba88cad with merge 88aac11...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 9, 2025

💔 Test failed - checks-actions

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 9, 2025
@workingjubilee
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Feb 9, 2025

⌛ Trying commit bba98a6 with merge 19ec3e2...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2025
…-codegen-tests, r=<try>

tests: `-Copt-level=3` instead of `-O` in codegen tests

An effective blocker for rust-lang#135439

r? `@ghost`

try-job: test-various
try-job: aarch64-gnu
try-job: aarch64-gnu-debug
try-job: i686-gnu-1
try-job: i686-gnu-2
try-job: i686-mingw
try-job: i686-msvc-1
try-job: i686-msvc-2
try-job: x86_64-msvc-1
try-job: x86_64-msvc-2
@rust-log-analyzer

This comment has been minimized.

@workingjubilee
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Feb 9, 2025

⌛ Trying commit daf1ffc with merge 8c43bfa...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2025
…-codegen-tests, r=<try>

tests: `-Copt-level=3` instead of `-O` in codegen tests

An effective blocker for rust-lang#135439

r? `@ghost`

try-job: test-various
try-job: aarch64-gnu
try-job: aarch64-gnu-debug
try-job: i686-gnu-1
try-job: i686-gnu-2
try-job: i686-mingw
try-job: i686-msvc-1
try-job: i686-msvc-2
try-job: x86_64-msvc-1
try-job: x86_64-msvc-2
@rust-log-analyzer

This comment has been minimized.

@workingjubilee
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Feb 9, 2025

⌛ Trying commit dd1ee8c with merge f1e2ad8...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2025
…-codegen-tests, r=<try>

tests: `-Copt-level=3` instead of `-O` in codegen tests

An effective blocker for rust-lang#135439

r? `@ghost`

try-job: test-various
try-job: aarch64-gnu
try-job: aarch64-gnu-debug
try-job: i686-gnu-1
try-job: i686-gnu-2
try-job: i686-mingw
try-job: i686-msvc-1
try-job: i686-msvc-2
try-job: x86_64-msvc-1
try-job: x86_64-msvc-2
@bors
Copy link
Contributor

bors commented Feb 9, 2025

☀️ Try build successful - checks-actions
Build commit: f1e2ad8 (f1e2ad852d6aa6412ad042f508d0ecf19500bd26)

@workingjubilee workingjubilee force-pushed the specify-opt-level-for-codegen-tests branch 2 times, most recently from 0b91173 to 2ea7953 Compare February 10, 2025 01:02
@workingjubilee workingjubilee marked this pull request as ready for review February 10, 2025 01:03
@@ -0,0 +1,36 @@
//@ revisions: OPT2 OPT3
//@[OPT2] compile-flags: -Copt-level=2
Copy link
Member

Choose a reason for hiding this comment

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

It's a bit odd to not have OPT2 filecheck comments, but I guess the idea here is to check for the lack of a shufflevector at level 2, and check that we get the slick bswap codegen at level 3?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, we're checking for "no shufflevector plz" at both levels and then for the bswap at OPT3

Copy link
Member Author

Choose a reason for hiding this comment

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

for OPT3 the only way it could have a shufflevector instruction is if it smuggled itself in right between start: and the actual checked lines, but, idk, might as well?

@saethlin
Copy link
Member

@bors r+

In spite of this touching every codegen test I bet it's actually not going to conflict. Except for one test, this is just changing the flags line.

@bors
Copy link
Contributor

bors commented Feb 10, 2025

📌 Commit 2ea7953 has been approved by saethlin

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 10, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 11, 2025
…-codegen-tests, r=<try>

tests: `-Copt-level=3` instead of `-O` in codegen tests

An effective blocker for redefining the meaning of `-O` is to stop reusing this somewhat ambiguous alias in our own codegen test suite. The choice between `-Copt-level=2` and `-Copt-level=3` is arbitrary for most of our tests. In most cases it makes no difference, so I set most of them to `-Copt-level=3`, as it will lead to slightly more "normalized" codegen.

try-job: test-various
try-job: arm-android
try-job: armhf-gnu
try-job: i686-gnu-1
try-job: i686-gnu-2
try-job: i686-mingw
try-job: i686-msvc-1
try-job: i686-msvc-2
try-job: aarch64-apple
try-job: aarch64-gnu
@bors
Copy link
Contributor

bors commented Feb 11, 2025

⌛ Trying commit d76bcf2 with merge e1f698a...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 11, 2025

💔 Test failed - checks-actions

@workingjubilee workingjubilee force-pushed the specify-opt-level-for-codegen-tests branch from d76bcf2 to 16fb1c4 Compare February 11, 2025 18:47
@workingjubilee
Copy link
Member Author

think I have a better idea...

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 11, 2025
…-codegen-tests, r=<try>

tests: `-Copt-level=3` instead of `-O` in codegen tests

An effective blocker for redefining the meaning of `-O` is to stop reusing this somewhat ambiguous alias in our own codegen test suite. The choice between `-Copt-level=2` and `-Copt-level=3` is arbitrary for most of our tests. In most cases it makes no difference, so I set most of them to `-Copt-level=3`, as it will lead to slightly more "normalized" codegen.

try-job: test-various
try-job: arm-android
try-job: armhf-gnu
try-job: i686-gnu-1
try-job: i686-gnu-2
try-job: i686-mingw
try-job: i686-msvc-1
try-job: i686-msvc-2
try-job: aarch64-apple
try-job: aarch64-gnu
@bors
Copy link
Contributor

bors commented Feb 11, 2025

⌛ Trying commit 16fb1c4 with merge a28e7fa...

@scottmcm
Copy link
Member

Sounds good. I've occasionally intentionally done opt-level=1 for tests that certain optimizations are "easy", but I can't think of a time I've written something that really cared about testing 2 vs 3 specifically. (Though there's probably some vectorization tests that do care.)

@bors
Copy link
Contributor

bors commented Feb 11, 2025

☀️ Try build successful - checks-actions
Build commit: a28e7fa (a28e7fa9603d6c4f39773a02f028cc4c0dc65108)

This should guarantee it tests what we want it to test and no more.
It should probably also run on 64-bit platforms that are not x86-64,
which will often have the vector registers the opt implies.
@workingjubilee workingjubilee force-pushed the specify-opt-level-for-codegen-tests branch from 16fb1c4 to 3c0c9b6 Compare February 11, 2025 21:46
@workingjubilee
Copy link
Member Author

@bors r=saethlin

@bors
Copy link
Contributor

bors commented Feb 11, 2025

📌 Commit 3c0c9b6 has been approved by saethlin

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 11, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 12, 2025
…or-codegen-tests, r=saethlin

tests: `-Copt-level=3` instead of `-O` in codegen tests

An effective blocker for redefining the meaning of `-O` is to stop reusing this somewhat ambiguous alias in our own codegen test suite. The choice between `-Copt-level=2` and `-Copt-level=3` is arbitrary for most of our tests. In most cases it makes no difference, so I set most of them to `-Copt-level=3`, as it will lead to slightly more "normalized" codegen.

try-job: test-various
try-job: arm-android
try-job: armhf-gnu
try-job: i686-gnu-1
try-job: i686-gnu-2
try-job: i686-mingw
try-job: i686-msvc-1
try-job: i686-msvc-2
try-job: aarch64-apple
try-job: aarch64-gnu
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 12, 2025
…llaumeGomez

Rollup of 9 pull requests

Successful merges:

 - rust-lang#135025 (Cast allocas to default address space)
 - rust-lang#136217 (Mark condition/carry bit as clobbered in C-SKY inline assembly)
 - rust-lang#136699 (std: replace the `FromInner` implementation for addresses with private conversion functions)
 - rust-lang#136758 (tests: `-Copt-level=3` instead of `-O` in assembly tests)
 - rust-lang#136761 (tests: `-Copt-level=3` instead of `-O` in codegen tests)
 - rust-lang#136807 (compiler: internally merge `PtxKernel` into `GpuKernel`)
 - rust-lang#136818 (Implement `read*_exact` for `std:io::repeat`)
 - rust-lang#136831 (Update stdarch)
 - rust-lang#136916 (use cc archiver as default in `cc2ar`)

r? `@ghost`
`@rustbot` modify labels: rollup
@GuillaumeGomez
Copy link
Member

I wonder if this is the one which failed in #136930.

@workingjubilee
Copy link
Member Author

Nah, that error looks like the stdarch update. Looks like cg_clif stopped compiling some code in stdarch due to updates to the Arm intrinsic generator?

@GuillaumeGomez
Copy link
Member

Noted, thanks! Gonna comment there then!

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 12, 2025
…llaumeGomez

Rollup of 10 pull requests

Successful merges:

 - rust-lang#136758 (tests: `-Copt-level=3` instead of `-O` in assembly tests)
 - rust-lang#136761 (tests: `-Copt-level=3` instead of `-O` in codegen tests)
 - rust-lang#136784 (Nuke `Buffer` abstraction from `librustdoc`, take 2 💣)
 - rust-lang#136838 (Check whole `Unsize` predicate for escaping bound vars)
 - rust-lang#136848 (add docs and ut for bootstrap util cache)
 - rust-lang#136871 (dev-guide: Link to `t-lang` procedures for new features)
 - rust-lang#136890 (Change swap_nonoverlapping from lang to library UB)
 - rust-lang#136901 (compiler: give `ExternAbi` truly stable `Hash` and `Ord`)
 - rust-lang#136907 (compiler: Make middle errors `pub(crate)` and bury the dead code)
 - rust-lang#136916 (use cc archiver as default in `cc2ar`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f0af030 into rust-lang:master Feb 12, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 12, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 12, 2025
Rollup merge of rust-lang#136761 - workingjubilee:specify-opt-level-for-codegen-tests, r=saethlin

tests: `-Copt-level=3` instead of `-O` in codegen tests

An effective blocker for redefining the meaning of `-O` is to stop reusing this somewhat ambiguous alias in our own codegen test suite. The choice between `-Copt-level=2` and `-Copt-level=3` is arbitrary for most of our tests. In most cases it makes no difference, so I set most of them to `-Copt-level=3`, as it will lead to slightly more "normalized" codegen.

try-job: test-various
try-job: arm-android
try-job: armhf-gnu
try-job: i686-gnu-1
try-job: i686-gnu-2
try-job: i686-mingw
try-job: i686-msvc-1
try-job: i686-msvc-2
try-job: aarch64-apple
try-job: aarch64-gnu
@workingjubilee workingjubilee deleted the specify-opt-level-for-codegen-tests branch February 13, 2025 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants