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

Mark Location::caller() as #[inline] #95619

Merged
merged 1 commit into from
Apr 4, 2022

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Apr 3, 2022

This function gets compiled to a single register move as it actually gets it's return value passed in as argument.

This function gets compiled to a single register move as it actually
gets it's return value passed in as argument.
@rust-highfive
Copy link
Collaborator

r? @scottmcm

(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 Apr 3, 2022
@joshtriplett
Copy link
Member

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 3, 2022
@bors
Copy link
Contributor

bors commented Apr 3, 2022

⌛ Trying commit 6d0b61e with merge da29afd45000f8726fab74f84fed8e1d0fab5ed6...

@bors
Copy link
Contributor

bors commented Apr 3, 2022

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

@rust-timer
Copy link
Collaborator

Queued da29afd45000f8726fab74f84fed8e1d0fab5ed6 with parent 168a020, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (da29afd45000f8726fab74f84fed8e1d0fab5ed6): comparison url.

Summary:

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: no relevant changes found. 2 results were found to be statistically significant but too small to be relevant.
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 0 2 0 0 0
mean2 N/A 0.3% N/A N/A N/A
max N/A 0.3% N/A N/A N/A

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Footnotes

  1. number of relevant changes

  2. the arithmetic mean of the percent change

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 3, 2022
@scottmcm
Copy link
Member

scottmcm commented Apr 3, 2022

Inlining a function with a non-monomorphized-looking signature seems perfectly reasonable to me.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 3, 2022

📌 Commit 6d0b61e has been approved by scottmcm

@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 Apr 3, 2022
@bors
Copy link
Contributor

bors commented Apr 3, 2022

⌛ Testing commit 6d0b61e with merge 596dece...

@bors
Copy link
Contributor

bors commented Apr 4, 2022

☀️ Test successful - checks-actions
Approved by: scottmcm
Pushing 596dece to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 4, 2022
@bors bors merged commit 596dece into rust-lang:master Apr 4, 2022
@rustbot rustbot added this to the 1.61.0 milestone Apr 4, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (596dece): comparison url.

Summary:

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: no relevant changes found. 1 results were found to be statistically significant but too small to be relevant.
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 0 0 0 1 0
mean2 N/A N/A N/A -0.3% N/A
max N/A N/A N/A -0.3% N/A

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. number of relevant changes

  2. the arithmetic mean of the percent change

@bjorn3 bjorn3 deleted the inline_location_caller branch April 4, 2022 08:16
@bjorn3
Copy link
Member Author

bjorn3 commented Apr 5, 2022

I think this broke the ui/rfc-2091-track-caller/intrinsic-wrapper.rs test with cg_clif for some reason:

command: "/home/runner/work/rustc_codegen_cranelift/rustc_codegen_cranelift/rust/build/x86_64-unknown-linux-gnu/test/ui/rfc-2091-track-caller/intrinsic-wrapper.mir-opt/a"
stdout: none
--- stderr -------------------------------
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `"/home/runner/work/rustc_codegen_cranelift/rustc_codegen_cranelift/build_sysroot/sysroot_src/library/core/src/panic/location.rs"`,
 right: `"/home/runner/work/rustc_codegen_cranelift/rustc_codegen_cranelift/rust/src/test/ui/rfc-2091-track-caller/intrinsic-wrapper.rs"`', /home/runner/work/rustc_codegen_cranelift/rustc_codegen_cranelift/rust/src/test/ui/rfc-2091-track-caller/intrinsic-wrapper.rs:11:5

https://github.com/bjorn3/rustc_codegen_cranelift/runs/5831236628?check_suite_focus=true

@bjorn3 bjorn3 restored the inline_location_caller branch April 6, 2022 16:43
@bjorn3 bjorn3 deleted the inline_location_caller branch April 6, 2022 16:46
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 6, 2022
…r, r=compiler-errors

Revert "Mark Location::caller() as #[inline]"

This reverts rust-lang#95619. As noted in rust-lang#95619 (comment) this seems to break several tests with cg_clif.
workingjubilee pushed a commit to tcdi/postgrestd that referenced this pull request Sep 15, 2022
…iler-errors

Revert "Mark Location::caller() as #[inline]"

This reverts rust-lang/rust#95619. As noted in rust-lang/rust#95619 (comment) this seems to break several tests with cg_clif.
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. 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.

7 participants