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

<format> use _MSVC_EXECUTION_CHARACTER_SET #2493

Merged

Conversation

barcharcraz
Copy link
Member

@barcharcraz barcharcraz commented Jan 20, 2022

This should drastically increase format's performance in the "default" case (still need to rerun my toy benchmarks though)

For clang use __clang_literal_encoding, clang 14 will learn
_MSVC_EXECUTION_CHARACTER_SET, but __clang_literal_encoding
is in clang 13.

Additionally for the "slow path" get the "format codepage" using
these new macros (at compile-time) rather than assuming the
active codepage is the same as the execution character set.

ABI Impact:

This changes which versions of some templates get used, and could cause ODR violations, but in general should make people's ABI situation better rather than worse.

In particular windows 1251 will now use the "fast path" on msvc, so if you link clang code (which always used the fast path) to
non-clang code you no longer have an ODR violation in std::foramt (although your codepages differ, so there are probably still relatively benign ODR violations elsewhere).

fixes: #2342

For clang use ____clang_literal_encoding__, clang 14 will learn
_MSVC_EXECUTION_CHARACTER_SET, but ____clang_literal_encoding__
is in clang 13.

Additionally for the "slow path" get the "format codepage" using
these new macros (at compile-time) rather than assuming the
active codepage is the same as the execution character set.
@barcharcraz barcharcraz requested a review from a team as a code owner January 20, 2022 01:17
@StephanTLavavej

This comment has been minimized.

stl/inc/format Outdated Show resolved Hide resolved
stl/inc/format Outdated Show resolved Hide resolved
stl/inc/format Outdated Show resolved Hide resolved
Remember to actually write code that compiles under clang :/
stl/inc/format Outdated Show resolved Hide resolved
stl/inc/format Outdated Show resolved Hide resolved
stl/inc/format Outdated Show resolved Hide resolved
stl/inc/format Outdated Show resolved Hide resolved
stl/inc/format Outdated Show resolved Hide resolved
stl/inc/format Show resolved Hide resolved
stl/inc/format Outdated Show resolved Hide resolved
stl/inc/format Outdated Show resolved Hide resolved
stl/inc/format Show resolved Hide resolved
stl/inc/format Show resolved Hide resolved
stl/inc/format Outdated Show resolved Hide resolved
stl/inc/format Outdated Show resolved Hide resolved
stl/inc/format Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter self-assigned this Jan 28, 2022
@CaseyCarter CaseyCarter removed their assignment Jan 29, 2022
@StephanTLavavej StephanTLavavej self-assigned this Jan 31, 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 e4303b5 into microsoft:main Feb 1, 2022
@StephanTLavavej
Copy link
Member

Thanks for this significant performance improvement! 🚀 🚀 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format C++20/23 format performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<format>: format should statically detect the default latin-1252 character set
5 participants
  翻译: