[PATCH] D142305: [ADT] llvm::bit_cast - use __builtin_bit_cast if available

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D142305#4082091 <https://reviews.llvm.org/D142305#4082091>, @RKSimon wrote:

> In D142305#4078205 <https://reviews.llvm.org/D142305#4078205>, @dblaikie wrote:
>
>> In D142305#4077555 <https://reviews.llvm.org/D142305#4077555>, @RKSimon wrote:
>>
>>> In D142305#4077361 <https://reviews.llvm.org/D142305#4077361>, @dblaikie wrote:
>>>
>>>>> If the compiler supports __builtin_bit_cast we should try to use it instead of std::memcpy (and avoid including the cstring header).
>>>>
>>>> Why? Maintaining non-portable code has some cost, if we can get similar/sufficient/the same performance without special casing, that seems better. Do we not get adequate performance with `std::memcpy`? Is the header especially expensive to include?
>>>
>>> No strong preference tbh
>>
>> Did something motivate proposing this change?
>
> This was part of a general attempt to get our bit.h as close to c++ <bit> as possible - especially when I realised that all latest supported compilers already implement '__builtin_bit_cast' : https://godbolt.org/z/YWM4GcceE

I'd say unless there's some significant gain - maintaining #ifdef'd non-portable(to all supported compilers) codepaths isn't worth the maintenance burden. But I don't feel strongly enough to insist on a revert - up to you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142305/new/

https://reviews.llvm.org/D142305