Skip to content

[libc++] Ensure that passing predicates through reference_wrapper doesn't kill desugars_to optimizations #129312

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.

Sign up for GitHub

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

Jump to bottom
Closed
ldionne opened this issue Feb 28, 2025 · 1 comment · Fixed by #132092
Closed

[libc++] Ensure that passing predicates through reference_wrapper doesn't kill desugars_to optimizations #129312

ldionne opened this issue Feb 28, 2025 · 1 comment · Fixed by #132092
Assignees
@ldionne
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. performance

Comments

@ldionne
Copy link
Member

ldionne commented Feb 28, 2025

We do that in e.g. ranges::contains where we pass a reference_wrapper to the projection to ranges::find. Does that turn off optimizations?
@ldionne ldionne added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. performance labels Feb 28, 2025
@ldionne ldionne mentioned this issue Mar 19, 2025
Open
@ldionne
Copy link
Member Author

ldionne commented Mar 19, 2025

I just confirmed that this actually breaks our optimizations. Calling std::mismatch with a reference_wrapper prevents it from being optimized properly.

@ldionne ldionne self-assigned this Mar 19, 2025
ldionne added a commit to ldionne/llvm-project that referenced this issue Mar 19, 2025
@ldionne
[libc++] Make sure that __desugars_to isn't tripped up by reference_w…
72990d4 
…rapper and cv-refs

Previously, any cv-ref qualification on an operation would cause
__desugars_to to report false, which would lead to unnecessary
pessimizations. The same holds for reference_wrapper.

In practice, cv-ref qualifications on the operation itself are not
relevant to determining whether an operation desugars to something
else or not.

Fixes llvm#129312
@ldionne ldionne mentioned this issue Mar 19, 2025
Merged
ldionne added a commit to ldionne/llvm-project that referenced this issue Mar 24, 2025
@ldionne
[libc++] Make sure that __desugars_to isn't tripped up by reference_w…
3641b46 
…rapper and cv-refs

Previously, any cv-ref qualification on an operation would cause
__desugars_to to report false, which would lead to unnecessary
pessimizations. The same holds for reference_wrapper.

In practice, cv-ref qualifications on the operation itself are not
relevant to determining whether an operation desugars to something
else or not.

Fixes llvm#129312
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ldionne ldionne

Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. performance
Projects
None yet
Milestone
No milestone
1 participant
@ldionne