Don't include <bit> in <compare>

[frederick-vs-ja]

Separated from #3623. Towards #3599.

Once <compare> doesn't include <bit>, <format> needs to include <bit> directly. I think it's better to use the standard std::bit_cast in <format>.

I have no good idea how to move std::bit_cast to another header. So currently this PR uses the __builtin_bit_cast intrinsic.

The current decision is moving _Bit_cast from <xutility> to <type_traits> and using it in <compare>.

[strega-nil]

However, I'm not convinced by this change at all; <bit> is one of the "most core" headers possible, and I don't necessarily believe it to be a good idea to try and remove it as a dependency.

[StephanTLavavej]

We talked about this at the weekly maintainer meeting and would prefer to see a different approach here, although we don't immediately know what would be best. Possible approaches to consider (in rough order of preference, from most preferred to least):

• Disconnect <bit> from <limits>, because <limits> is needed only for numeric_limits::digits which is trivially easy to compute for integral types (it's just sizeof(T) * 8). Then <bit> would include only <type_traits> (which we pay the cost for basically everywhere) and <isa_availability.h> which is core-compatible and very small, so we wouldn't need to worry about including <bit> in other headers.
  • We should consider doing this anyways.
• _Bit_cast is currently defined in <xutility> but it could be moved up even higher (e.g. up to <type_traits> or similar). Its whole purpose is to be widely available (even without C++20) and using it even in C++20 mode to avoid dragging in <bit> is reasonable.
• Move Standard bit_cast up higher (possibly <type_traits>). This is perhaps the least clean solution, as it exposes a Standard name to more TUs that weren't asking for it, encouraging non-Standard code to be written.

[frederick-vs-ja]

I'm afraid that <bit> is (far) heavier in STL than it should be. I've tried to reduce inclusion dependency on <cstdlib> (via <xstddef> in #3623), but it seems the current implementation strategy of <bit> needs <stdlib.h> due to _byteswap_*, _rotr, etc., and <stdlib.h> already drags in declarations of more than 200 functions.

Additionally, in MSVC STL we generally includes <cstdlib> now, which currently needs to include <math.h> due to LWG-2192.

• Disconnect <bit> from <limits>, because <limits> is needed only for numeric_limits::digits which is trivially easy to compute for integral types (it's just sizeof(T) * 8). Then <bit> would include only <type_traits> (which we pay the cost for basically everywhere) and <isa_availability.h> which is core-compatible and very small, so we wouldn't need to worry about including <bit> in other headers.

There are other dependencies on <limits> - <bit> currently needs _Countl_zero_fallback, _Countr_zero, _Popcount, and so on, from <limits>. And these internal functions are also needed by other headers (e.g. <bitset>).

• It seems intended that <limits> provides some internal bit operation functions for other headers in STL.

• _Bit_cast is currently defined in <xutility> but it could be moved up even higher (e.g. up to <type_traits> or similar). Its whole purpose is to be widely available (even without C++20) and using it even in C++20 mode to avoid dragging in <bit> is reasonable.

In most cases, _Bit_cast is used for coversion between unsigned integer representations and FP numbers in STL. The exceptions are in <system_error> and <format>, both of which already include <xutility>.

How about defining FP <-> UInt conversion functions in <type_traits> that wrap __builtin_bit_cast?

[StephanTLavavej]

the current implementation strategy of <bit> needs <stdlib.h> due to _byteswap_*, _rotr, etc.

Thanks, I had forgotten that dependency.

and <stdlib.h> already drags in declarations of more than 200 functions.

I think of that as fairly lightweight. Granted, I haven't done detailed profiling of throughput recently, but my expectation is that function declarations are fairly cheap. It's class definitions that can start to be expensive as they add up, and especially function definitions (including function template instantiations).

There are other dependencies on <limits> - <bit> currently needs _Countl_zero_fallback, _Countr_zero, _Popcount, and so on, from <limits>.

Oh yeah, thanks.

How about defining FP <-> UInt conversion functions in <type_traits> that wrap __builtin_bit_cast?

That might be worth it, if it allows us to avoid dragging in significant machinery. However, we want to be careful about pursuing throughput too aggressively and ending up with replicating various kinds of helpers when we could have just one.

What do you think about lifting just _Bit_cast up to <type_traits>? The generic one, not anything dedicated to FP-to-integer and vice versa.

[StephanTLavavej]

Thanks, I think this change is simple and safe now. It affects only C++20 mode, so there is virtually zero concern about source-breaking changes. It's also nice that _Bit_cast is now available for use in core headers, as it needs no runtime support.

[StephanTLavavej]

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

[StephanTLavavej]

Thanks for improving throughput one bit at a time! 😹 🎉 🚀