[PATCH] D142274: [ADT] Add llvm::byteswap to bit.h

[Kazu Hirata via Phabricator via llvm-commits]

kazu marked 2 inline comments as done.
kazu added a comment.

Please take a look.  Thanks!



================
Comment at: llvm/include/llvm/ADT/bit.h:62
+    // release build they're replaced with BSWAP instructions anyway.
+    return _byteswap_ushort(UV);
+#else
----------------
RKSimon wrote:
> Looking at SwapByteOrder.h suggests this might need #include <cstdlib>?
I've added

```
#if defined(_MSC_VER) && !defined(_DEBUG)
#include <stdlib.h>  // for _byteswap_uint64                                                                           
#endif
```



================
Comment at: llvm/include/llvm/ADT/bit.h:70
+    uint32_t UV = V;
+#if defined(__llvm__) || (defined(__GNUC__) && !defined(__ICC))
+    return __builtin_bswap32(UV);
----------------
kazu wrote:
> RKSimon wrote:
> > use has_builtin ?
> Shall we stop using the intrinsics and just stick to the ANDs and ORs?  This way, we don't have to worry about the availability of intrinsics, the availability of has_builtin itself, whether we are using ICC, etc.  (I couldn't find out why ICC is excluded here.)  According to compiler explorer, gcc-4.5.3 and clang-10.0.0 generate bswap for the following code snippet:
> 
> ```
> #include <stdint.h>
> 
> uint32_t bs32(uint32_t v) { 
>     uint32_t Byte0 = v & 0x000000FF;
>     uint32_t Byte1 = v & 0x0000FF00;
>     uint32_t Byte2 = v & 0x00FF0000;
>     uint32_t Byte3 = v & 0xFF000000;
>     return (Byte0 << 24) | (Byte1 << 8) | (Byte2 >> 8) | (Byte3 >> 24);
> }
> 
> uint64_t bs64(uint64_t v) {
>     uint64_t Hi = bs32(v);
>     uint32_t Lo = bs32(v >> 32);
>     return (Hi << 32) | Lo;
> }
> ```
> 
> clang prior to 10.0.0 does not generate 64-bit bswap, but we don't attempt to use 64-bit bswap intrinsic in this patch or `SwapByteOrder.h` anyway.
> 
> Note that I mention gcc-4.5.3 above, but we require gcc-7.1.0.
I've removed the dependence on GCC-style `__builtin_xxx`, so I don't use `__has_builtin`,

Both gcc and clang generate good code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142274