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

Add test for #98294 #101232

Merged
merged 1 commit into from
Sep 7, 2022
Merged

Add test for #98294 #101232

merged 1 commit into from
Sep 7, 2022

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Aug 31, 2022

Add a test to make that the failure condition for this pattern is optimized away.

Fixes #98294.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 31, 2022
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=iffy

@bors
Copy link
Contributor

bors commented Sep 1, 2022

📌 Commit 8a527ee has been approved by Mark-Simulacrum

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 Sep 1, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 3, 2022
Add test for rust-lang#98294

Add a test to make that the failure condition for this pattern is optimized away.

Fixes rust-lang#98294.
@matthiaskrgr
Copy link
Member

@bors r- failed in a rollup
#101372 (comment)

@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. labels Sep 3, 2022
@nikic
Copy link
Contributor Author

nikic commented Sep 5, 2022

Weird, this test does work fine in a local ./x.py test src/test/codegen run (also on x86_64, so there should be not platform difference).

I wasn't able to run x86_64-gnu locally due to #101438.

Anyone know what the relevant difference between x86_64 and a local run might be?

@Mark-Simulacrum
Copy link
Member

Beyond sanitizers and profilers being enabled, we also use a downloaded LLVM rather than building from scratch. That should be up to date, but maybe we're missing some conditional? You could try enabling downloaded LLVM locally; that should work the same (and you can compare to CI log for the chosen hash).

Add a test to make that the failure condition for this pattern is
optimized away.

Fixes rust-lang#98294.
@nikic
Copy link
Contributor Author

nikic commented Sep 5, 2022

It turns out the issue is debug assertion in std. I added an ignore-debug line, which is also used by many other codegen tests for vec/slice functionality.

@nikic
Copy link
Contributor Author

nikic commented Sep 6, 2022

@bors r=Mark-Simulacrum rollup=iffy

@bors
Copy link
Contributor

bors commented Sep 6, 2022

📌 Commit cbf3b24 has been approved by Mark-Simulacrum

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 Sep 6, 2022
@bors
Copy link
Contributor

bors commented Sep 7, 2022

⌛ Testing commit cbf3b24 with merge 0568b0a...

@bors
Copy link
Contributor

bors commented Sep 7, 2022

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 0568b0a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 7, 2022
@bors bors merged commit 0568b0a into rust-lang:master Sep 7, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 7, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0568b0a): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

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

mean1 range count2
Regressions ❌
(primary)
0.8% [0.7%, 0.9%] 6
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.1% [-1.1%, -1.1%] 1
All ❌✅ (primary) 0.8% [0.7%, 0.9%] 6

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.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.4% [-2.4%, -2.4%] 1
All ❌✅ (primary) - - 0

Cycles

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.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.4% [3.4%, 3.4%] 1
Improvements ✅
(primary)
-3.0% [-3.4%, -2.6%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -3.0% [-3.4%, -2.6%] 2

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@rustbot rustbot added the perf-regression Performance regression. label Sep 7, 2022
@lqd
Copy link
Member

lqd commented Sep 7, 2022

this is noise, @rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

slice::get_mut() followed by slice::copy_from_slice generates unreachable panic branch
8 participants