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

Inline strlen_rt in CStr::from_ptr #114950

Merged
merged 1 commit into from
Aug 20, 2023

Conversation

KamilaBorowska
Copy link
Contributor

@KamilaBorowska KamilaBorowska commented Aug 17, 2023

This enables LLVM to optimize this function as if it was strlen (LLVM knows what it does, and can avoid calling it in certain situations) without having to enable std-aware LTO. This is essentially doing what #90007 did, except updated for this function being const.

Pretty sure it's safe to roll-up, considering last time I did make this change it didn't affect performance (CStr::from_ptr isn't really used all that often in Rust code that is checked by rust-perf).

@rustbot
Copy link
Collaborator

rustbot commented Aug 17, 2023

r? @cuviper

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 17, 2023
@cuviper
Copy link
Member

cuviper commented Aug 18, 2023

Can you add comments on this and the outer inline, that these are important for codegen visibility?

This enables LLVM to optimize this function as if it was strlen
without having to enable std-aware LTO.
@KamilaBorowska
Copy link
Contributor Author

Done.

@cuviper
Copy link
Member

cuviper commented Aug 18, 2023

Thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 18, 2023

📌 Commit e94ba4a has been approved by cuviper

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 18, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 18, 2023
Inline strlen_rt in CStr::from_ptr

This enables LLVM to optimize this function as if it was strlen (LLVM knows what it does, and can avoid calling it in certain situations) without having to enable std-aware LTO. This is essentially doing what rust-lang#90007 did, except updated for this function being `const`.

Pretty sure it's safe to roll-up, considering last time I did make this change it didn't affect performance (`CStr::from_ptr` isn't really used all that often in Rust code that is checked by rust-perf).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 19, 2023
Inline strlen_rt in CStr::from_ptr

This enables LLVM to optimize this function as if it was strlen (LLVM knows what it does, and can avoid calling it in certain situations) without having to enable std-aware LTO. This is essentially doing what rust-lang#90007 did, except updated for this function being `const`.

Pretty sure it's safe to roll-up, considering last time I did make this change it didn't affect performance (`CStr::from_ptr` isn't really used all that often in Rust code that is checked by rust-perf).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 19, 2023
Inline strlen_rt in CStr::from_ptr

This enables LLVM to optimize this function as if it was strlen (LLVM knows what it does, and can avoid calling it in certain situations) without having to enable std-aware LTO. This is essentially doing what rust-lang#90007 did, except updated for this function being `const`.

Pretty sure it's safe to roll-up, considering last time I did make this change it didn't affect performance (`CStr::from_ptr` isn't really used all that often in Rust code that is checked by rust-perf).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 19, 2023
Inline strlen_rt in CStr::from_ptr

This enables LLVM to optimize this function as if it was strlen (LLVM knows what it does, and can avoid calling it in certain situations) without having to enable std-aware LTO. This is essentially doing what rust-lang#90007 did, except updated for this function being `const`.

Pretty sure it's safe to roll-up, considering last time I did make this change it didn't affect performance (`CStr::from_ptr` isn't really used all that often in Rust code that is checked by rust-perf).
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 19, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#114605 (Increase clarity about Hash - Eq consistency in HashMap and HashSet docs)
 - rust-lang#114934 (instantiate response: no unnecessary new universe)
 - rust-lang#114950 (Inline strlen_rt in CStr::from_ptr)
 - rust-lang#114973 (Expose core::error::request_value in std)
 - rust-lang#114983 (Usage zero as language id for `FormatMessageW()`)
 - rust-lang#114991 (remove redundant var rebindings)
 - rust-lang#114992 (const-eval: ensure we never const-execute a function marked rustc_do_not_const_check)
 - rust-lang#115001 (clippy::perf stuff)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cad8f8c into rust-lang:master Aug 20, 2023
11 checks passed
@rustbot rustbot added this to the 1.73.0 milestone Aug 20, 2023
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-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants