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 support for [[msvc::intrinsic]] #3182

Merged
merged 4 commits into from
Oct 27, 2022

Conversation

cdacamar
Copy link
Contributor

This adds support for the detection and application of the [[msvc::intrinsic]] attribute which will allow the compiler to internally implement some functionality of the STL without a function call.

@cdacamar cdacamar requested a review from a team as a code owner October 26, 2022 20:22
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Oct 26, 2022
Copy link
Member

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, but it's failing format validation. You'll need to run the problem files through clang-format 15 either manually, or with the format build target, or by downloading and applying the patch the build generates to check formatting (https://dev.azure.com/vclibs/STL/_build/results?buildId=12759&view=artifacts&pathAsName=false&type=publishedArtifacts).

@CaseyCarter CaseyCarter added the high priority Important! label Oct 26, 2022
stl/inc/type_traits Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej self-assigned this Oct 26, 2022
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 48eb4a4 into microsoft:main Oct 27, 2022
@StephanTLavavej
Copy link
Member

Thanks for making the STL go faster! Must go faster! 🏃 🚗 🦖

@cdacamar
Copy link
Contributor Author

cdacamar commented Dec 7, 2022

It is worth noting that @xiangfan-ms noted since these functions are not instantiated in some cases, it is possible that static_assertions in the body of these functions would fail to fire so these assertions need to be hoisted as constrains to calling the function.

@CaseyCarter
Copy link
Member

@cdacamar that implies a bug in one of the std::forward overloads which needs a static_assert to enforce one of the Standard's requirements. I'm unable to reproduce it, though. This program:

#include <utility>
int main() {
    (void) std::forward<int&>(42);
}

triggers the static_assert even with the latest prod/fe compiler. Do I misunderstand the optimization, or ...?

@cdacamar
Copy link
Contributor Author

cdacamar commented Dec 9, 2022

Did you also compile with /permissive-? When I compile with /permissive- I get:

t.cpp(4): error C2101: '&' on constant

Which is correct if the function were replaced with a cast, but also does not instantiate the function so the static assertion is missing.

@CaseyCarter
Copy link
Member

Did you also compile with /permissive-?

Aha! No, I did not. Do I surmise correctly that the optimization is only enabled in strict mode?

@cdacamar
Copy link
Contributor Author

cdacamar commented Dec 9, 2022

Yes, the optimization is only available when /permissive- is used or implied because two-phase is required for function template definition checking whether the attribute is applicable to the function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved high priority Important!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants