Skip to content

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 assembly 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" assembly.

@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
…-tests, r=<try>

tests: Specify `-Copt-level=3` instead of `-O` in 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
Collaborator

bors commented Feb 9, 2025

⌛ Trying commit ee111b2 with merge c912816...

@workingjubilee workingjubilee changed the title tests: Specify -Copt-level=3 instead of -O in tests tests: Specify -Copt-level=3 instead of -O in assembly tests Feb 9, 2025
@workingjubilee workingjubilee changed the title tests: Specify -Copt-level=3 instead of -O in assembly tests tests: -Copt-level=3 instead of -O in assembly tests Feb 9, 2025
@dianqk
Copy link
Member

dianqk commented Feb 9, 2025

Why not -Copt-level=2? The intention of some test cases might be this.

@workingjubilee
Copy link
Member Author

Even experienced Rust contributors thought it meant -Copt-level=3.

Authors that do not document the intention of their test have already cast it to the winds.

@workingjubilee
Copy link
Member Author

For instance, it was easy to tell that the intention was in fact -Copt-level=2 so I did set it thusly over here: 5a0032c

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

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

and there are good reasons to decide to not use -Copt-level=3 sometimes, see a7c7f39

@workingjubilee
Copy link
Member Author

@bors try

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

tests: `-Copt-level=3` instead of `-O` in assembly 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
Collaborator

bors commented Feb 9, 2025

⌛ Trying commit a7c7f39 with merge 088e0cc...

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Feb 9, 2025

💔 Test failed - checks-actions

@workingjubilee
Copy link
Member Author

@bors try

@bors
Copy link
Collaborator

bors commented Feb 9, 2025

⌛ Trying commit 984605d with merge d0a1bf8...

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

tests: `-Copt-level=3` instead of `-O` in assembly 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
Collaborator

bors commented Feb 9, 2025

☀️ Try build successful - checks-actions
Build commit: d0a1bf8 (d0a1bf858b9d60c1cca3e366c262aac76ce59c3c)

We choose to test for Linux and Windows instead of random other targets.
@workingjubilee workingjubilee force-pushed the specify-opt-level-for-tests branch from 984605d to 833f070 Compare February 10, 2025 00:55
@bors
Copy link
Collaborator

bors commented Feb 10, 2025

📌 Commit 833f070 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 10, 2025
@workingjubilee
Copy link
Member Author

will collide with #135408

@bors r-
@rustbot label: S-blocked

@rustbot rustbot added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Feb 10, 2025
@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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Feb 10, 2025
@workingjubilee
Copy link
Member Author

Well that one needs to be fixed so I guess not.

@bors r=saethlin

@bors
Copy link
Collaborator

bors commented Feb 10, 2025

📌 Commit 833f070 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 10, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Feb 11, 2025
…or-tests, r=saethlin

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

An effective blocker for redefining the meaning of `-O` is to stop reusing this somewhat ambiguous alias in our own assembly 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" assembly.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 11, 2025
Rollup of 6 pull requests

Successful merges:

 - rust-lang#134999 (Add cygwin target.)
 - rust-lang#135677 (Small `rustc_resolve` cleanups)
 - 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#136833 (compiler: die immediately instead of handling unknown target codegen)

Failed merges:

 - rust-lang#136808 (Try to recover from path sep error in type parsing)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 11, 2025
Rollup of 7 pull requests

Successful merges:

 - rust-lang#134090 (Stabilize target_feature_11)
 - rust-lang#134999 (Add cygwin target.)
 - rust-lang#135025 (Cast allocas to default address space)
 - rust-lang#135408 (x86: make SSE2 required for i686 hardfloat targets and use it to pass SIMD types)
 - rust-lang#135549 (Document some safety constraints and use more safe wrappers)
 - rust-lang#136193 (Implement pattern type ffi checks)
 - rust-lang#136699 (std: replace the `FromInner` implementation for addresses with private conversion functions)

Failed merges:

 - rust-lang#136758 (tests: `-Copt-level=3` instead of `-O` in assembly tests)

r? `@ghost`
`@rustbot` modify labels: rollup
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 12, 2025
…or-tests, r=saethlin

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

An effective blocker for redefining the meaning of `-O` is to stop reusing this somewhat ambiguous alias in our own assembly 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" assembly.
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

probably not, replied here: #136761 (comment)

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 a3f76ad 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#136758 - workingjubilee:specify-opt-level-for-tests, r=saethlin

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

An effective blocker for redefining the meaning of `-O` is to stop reusing this somewhat ambiguous alias in our own assembly 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" assembly.
@workingjubilee workingjubilee deleted the specify-opt-level-for-tests branch February 14, 2025 09:06
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.

7 participants