Skip to content

Rollup of 7 pull requests #129060

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

Merged
merged 21 commits into from
Aug 14, 2024
Merged

Rollup of 7 pull requests #129060

merged 21 commits into from
Aug 14, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

mzabaluev and others added 21 commits July 11, 2024 23:46
The branch at the end of the `pow` implementations is redundant
with multiplication code already present in the loop. By rotating
the exit check, this branch can be largely removed, improving code size
and instruction cache coherence.
The newly optimized loop has introduced a regression in the case
when pow is called with a small constant exponent. LLVM is no longer
able to unroll the loop and the generated code is larger and slower
than what's expected in tests.

Match and handle small exponent values separately by branching out
to an explicit multiplication sequence for that exponent.
Powers larger than 6 need more than three multiplications, so these
cases are less likely to benefit from this optimization, also such
constant exponents are less likely to be used in practice.
For uses with a non-constant exponent, this might also provide
a performance benefit if the exponent is small and does not vary
between successive calls, so the same match arm tends to be taken as
a predicted branch.
In the dynamic exponent case, it's preferred to not increase code size,
so use solely the loop-based implementation there.
This shows about 4% penalty in the variable exponent benchmarks
on x86_64.
Give LLVM the for original, optimizable loop in pow and wrapped_pow
functions in the case when the exponent is statically known.
Relevant for the deprecation of `CommandExt::before_exit` in rust-lang#125970.
…er error

This doesn't work for translated compiler error messages.
This function is called for both compiler and non-compiler output, so if the
line isn't recognized as JSON from the compiler then just print it as-is.
The JSON messages parsed by this file are from the _compiler_, not from
libtest.
… is used with other output formats than HTML
…r=Amanieu

Optimize integer `pow` by removing the exit branch

The branch at the end of the `pow` implementations is redundant with multiplication code already present in the loop. By rotating the exit check, this branch can be largely removed, improving code size and reducing instruction cache misses.

Testing on my machine (`x86_64`, 11th Gen Intel Core i5-1135G7 @ 2.40GHz), the `num::int_pow` benchmarks improve by some 40% for the unchecked operations and show some slight improvement for the checked operations as well.
…trochenkov

Allow to customize `// TODO:` comment for deprecated safe autofix

Relevant for the deprecation of `CommandExt::before_exit` in rust-lang#125970.

Tracking:
- rust-lang#124866
…joboet

Add `#[must_use]` attribute to `Coroutine` trait

[Coroutines tracking issue](rust-lang#43122)

Like closures (`FnOnce`, `AsyncFn`, etc.), coroutines are lazy and do nothing unless called (resumed). Closure traits like `FnOnce` have `#[must_use = "closures are lazy and do nothing unless called"]` to catch likely bugs for users of APIs that produce them. This PR adds such a `#[must_use]` attribute to `trait Coroutine`.
compiletest: Don't panic on unknown JSON-like output lines

The `json::extract_rendered` function is called for both compiler output and non-compiler output, so it's inappropriate to panic just because a line starting with `{` didn't contain a compiler output message.

It is called from two places: when checking the output of a `ui` test process, and when printing the output of an arbitrary non-passing `ProcRes`. So unfortunately there's currently no easy way to know for sure whether it is seeing compiler output or not. Fortunately, neither call site appears to be relying on this panic; it's just an overzealous internal check.

Fixes rust-lang#126373.
…finition-warning, r=notriddle

Emit a warning instead of an error if `--generate-link-to-definition` is used with other output formats than HTML

Fixes rust-lang/docs.rs#2581.

It's a bit weird to emit an error in this case anyway, a warning is more than enough.

r? ``@notriddle``
…zkan

Fix one usage of target triple in bootstrap

This bug was introduced in rust-lang#128983. In this one case, the `TargetSelection` was also used as `Display` (not just as `Path`), which I did not notice in the original PR. If the target contained a custom file, it would be included in its `Display` formatting, even though only the triple should be used.

Found [here](rust-lang#128983 (comment)).

r? `@onur-ozkan`
@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Aug 13, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Collaborator

bors commented Aug 13, 2024

📌 Commit 24a46c5 has been approved by matthiaskrgr

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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 13, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#122884 (Optimize integer `pow` by removing the exit branch)
 - rust-lang#127857 (Allow to customize `// TODO:` comment for deprecated safe autofix)
 - rust-lang#129034 (Add `#[must_use]` attribute to `Coroutine` trait)
 - rust-lang#129049 (compiletest: Don't panic on unknown JSON-like output lines)
 - rust-lang#129050 (Emit a warning instead of an error if `--generate-link-to-definition` is used with other output formats than HTML)
 - rust-lang#129056 (Fix one usage of target triple in bootstrap)
 - rust-lang#129058 (Add mw back to review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Collaborator

bors commented Aug 13, 2024

⌛ Testing commit 24a46c5 with merge d0bf5f7...

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-fuchsia failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
+ .jiri_root/bin/jiri import -name=integration -revision=1c5b42266fbfefb2337c6b2f0030a91bde15f9e9 -overwrite=true flower https://fuchsia.googlesource.com/integration
+ '[' -d .git ']'
+ .jiri_root/bin/jiri update -autoupdate=false
[20:53:45.292] Updating all projects
[20:55:43.264] Wait for 1m4s before next attempt...: Cloning https://fuchsia.googlesource.com/third_party/github.com/KhronosGroup/OpenCL-Headers

ERROR: 'git clone --no-checkout --filter=blob:none https://fuchsia.googlesource.com/third_party/github.com/KhronosGroup/OpenCL-Headers /checkout/obj/fuchsia/third_party/OpenCL-Headers' failed:

stderr:
Cloning into '/checkout/obj/fuchsia/third_party/OpenCL-Headers'...
error: RPC failed; HTTP 503 curl 22 The requested URL returned error: 503
error: RPC failed; HTTP 503 curl 22 The requested URL returned error: 503
fatal: error reading section header 'shallow-info'

command fail error: exit status 128

[20:55:46.576] Wait for 1m4s before next attempt...: Cloning https://fuchsia.googlesource.com/third_party/inih

ERROR: 'git clone --no-checkout --filter=blob:none https://fuchsia.googlesource.com/third_party/inih /checkout/obj/fuchsia/third_party/inih' failed:

stderr:
Cloning into '/checkout/obj/fuchsia/third_party/inih'...
error: RPC failed; HTTP 503 curl 22 The requested URL returned error: 503
error: RPC failed; HTTP 503 curl 22 The requested URL returned error: 503
fatal: error reading section header 'shallow-info'

command fail error: exit status 128

[20:56:04.727] Wait for 1m4s before next attempt...: Cloning https://fuchsia.googlesource.com/third_party/github.com/jd/tenacity

ERROR: 'git clone --no-checkout --filter=blob:none https://fuchsia.googlesource.com/third_party/github.com/jd/tenacity /checkout/obj/fuchsia/third_party/github.com/jd/tenacity/src' failed:

stderr:
stderr:
Cloning into '/checkout/obj/fuchsia/third_party/github.com/jd/tenacity/src'...
fatal: expected flush after ref listing

command fail error: exit status 128


[20:56:47.265] Attempt 2/3: Cloning https://fuchsia.googlesource.com/third_party/github.com/KhronosGroup/OpenCL-Headers
[20:56:50.579] Attempt 2/3: Cloning https://fuchsia.googlesource.com/third_party/inih


[20:57:08.734] Attempt 2/3: Cloning https://fuchsia.googlesource.com/third_party/github.com/jd/tenacity

[20:57:17.398] Wait for 1m4s before next attempt...: Cloning https://fuchsia.googlesource.com/third_party/github.com/KhronosGroup/OpenCL-Headers

ERROR: 'git clone --no-checkout --filter=blob:none https://fuchsia.googlesource.com/third_party/github.com/KhronosGroup/OpenCL-Headers /checkout/obj/fuchsia/third_party/OpenCL-Headers' failed:

stderr:
Cloning into '/checkout/obj/fuchsia/third_party/OpenCL-Headers'...
error: RPC failed; HTTP 503 curl 22 The requested URL returned error: 503
error: RPC failed; HTTP 503 curl 22 The requested URL returned error: 503
fatal: expected flush after ref listing

command fail error: exit status 128

[20:58:21.400] Attempt 3/3: Cloning https://fuchsia.googlesource.com/third_party/github.com/KhronosGroup/OpenCL-Headers

[21:08:52.516] WARN: Projects with local changes and/or not on JIRI_HEAD:
third_party/OpenCL-Headers (third_party/OpenCL-Headers): (Has changes)
third_party/freetype2 (third_party/freetype2): (Has changes)
third_party/github.com/google/serde_json5 (third_party/rust_crates/mirrors/serde_json5): (Has changes)

To force an update to JIRI_HEAD, you may run 'jiri runp git checkout JIRI_HEAD'
++ git -C integration rev-parse HEAD
+ echo integration commit = 1c5b42266fbfefb2337c6b2f0030a91bde15f9e9
integration commit = 1c5b42266fbfefb2337c6b2f0030a91bde15f9e9
+ bash scripts/rust/build_fuchsia_from_rust_ci.sh

@bors
Copy link
Collaborator

bors commented Aug 13, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 13, 2024
@matthiaskrgr
Copy link
Member Author

@bors retry

@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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 14, 2024
@bors
Copy link
Collaborator

bors commented Aug 14, 2024

⌛ Testing commit 24a46c5 with merge b16185a...

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 14, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#122884 (Optimize integer `pow` by removing the exit branch)
 - rust-lang#127857 (Allow to customize `// TODO:` comment for deprecated safe autofix)
 - rust-lang#129034 (Add `#[must_use]` attribute to `Coroutine` trait)
 - rust-lang#129049 (compiletest: Don't panic on unknown JSON-like output lines)
 - rust-lang#129050 (Emit a warning instead of an error if `--generate-link-to-definition` is used with other output formats than HTML)
 - rust-lang#129056 (Fix one usage of target triple in bootstrap)
 - rust-lang#129058 (Add mw back to review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Verifying status of rust-by-example...
Verifying status of edition-guide...
Verifying status of embedded-book...
Cloning into 'rust-toolstate'...
fatal: unable to access 'https://github.com/rust-lang-nursery/rust-toolstate/': Failed to connect to github.com port 443 after 129382 ms: Connection timed out
Command has failed. Rerun with -v to see more details.
  local time: Wed Aug 14 04:15:17 UTC 2024
  network time: Wed, 14 Aug 2024 04:15:17 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@bors
Copy link
Collaborator

bors commented Aug 14, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 14, 2024
@matthiaskrgr
Copy link
Member Author

@bors retry

@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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 14, 2024
@bors
Copy link
Collaborator

bors commented Aug 14, 2024

⌛ Testing commit 24a46c5 with merge fbce03b...

@bors
Copy link
Collaborator

bors commented Aug 14, 2024

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing fbce03b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 14, 2024
@bors bors merged commit fbce03b into rust-lang:master Aug 14, 2024
7 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 14, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#122884 Optimize integer pow by removing the exit branch f377c6ac1c4267c88932328883f3752482fb1b6b (link)
#127857 Allow to customize // TODO: comment for deprecated safe a… 7f0c89cb4ae881eb68171254bac55c3c709a4024 (link)
#129034 Add #[must_use] attribute to Coroutine trait f61bbc187c51c8f493555c6ad2763a5deb892bfd (link)
#129049 compiletest: Don't panic on unknown JSON-like output lines 290c630fe148a5b334f056a6c4fb79773f53db00 (link)
#129050 Emit a warning instead of an error if `--generate-link-to-d… d55a4c58c4e2492756cefa21f1896cc6298902d5 (link)
#129056 Fix one usage of target triple in bootstrap eca3fc87f22ce93e2ae1c6ec480070b340cadff9 (link)
#129058 Add mw back to review rotation fcb2e9b1f887154f9b437a58f01a7eac3f385c7d (link)

previous master: 9859bf27fd

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (fbce03b): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.3% [0.2%, 0.4%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.3%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary 1.4%, secondary 2.2%)

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% [3.0%, 3.3%] 2
Regressions ❌
(secondary)
2.2% [2.2%, 2.2%] 1
Improvements ✅
(primary)
-2.1% [-2.1%, -2.1%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.4% [-2.1%, 3.3%] 3

Cycles

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

Binary size

Results (primary 0.1%)

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)
0.2% [0.2%, 0.2%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.0% [-0.0%, -0.0%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [-0.0%, 0.2%] 2

Bootstrap: 750.582s -> 751.474s (0.12%)
Artifact size: 341.42 MiB -> 341.45 MiB (0.01%)

@matthiaskrgr matthiaskrgr deleted the rollup-s72gpif branch September 1, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.