[PATCH] D114425: [clang] Add __builtin_bswap128

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In D114425#3217478 <https://reviews.llvm.org/D114425#3217478>, @Quuxplusone wrote:

> In D114425#3216802 <https://reviews.llvm.org/D114425#3216802>, @majnemer wrote:
>
>> OOC, how hard would it be to generalize this builtin a little? It is nice that we have builtins like `__builtin_add_overflow` which do the right thing regardless of their input.
>>
>> It seems like it would be nice if we started to expose more intrinsics which did the right thing regardless of operand width; another bonus is that it composes well with language features like `_BitInt`.
>
> IMHO such builtins are nice iff the programmer can be 100% sure that the compiler will interpret them the same way as a human reader. `__builtin_add_overflow` is easy because its first two arguments are "mathematical integers" (where integer promotion doesn't matter) and its third argument is a pointer (where integer promotion can't happen). So you can really throw any combination of types at it, and it'll do "the right thing" https://godbolt.org/z/sa7b894oa (although I admit I was surprised that this worked).
> For a hypothetical `__builtin_bswap`, you would probably need a similar pointer-based interface like
>
>   short s16 = 0xFEDC;
>   __builtin_bswap(&s16);  // hypothetically
>   assert(s16 == 0xDCFE);
>   
>   assert(__builtin_bswap16(s16) == 0x0000DCFE);
>   assert(__builtin_bswap32(s16) == 0xDCFEFFFF);  // the problem to solve: s16 eagerly promotes to int, which changes the result
>
> The downside is that the pointer-based interface is less ergonomic than today's value-based signatures, and probably worse codegen at `-O0` (because the programmer has to materialize the operand into a named variable, and then the compiler won't remove that variable because you might want to debug it). The upside (as you said) is that a generic builtin could work with `_ExtInt` types and so on.

I think one way of side stepping that problem would be to say that `__builtin_bswap` only works with unsigned types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425