[cfe-commits] [PATCH][review request] Add support for LLVM intrinsic bswap16

[Eli Friedman]

On Sat, Mar 19, 2011 at 8:02 PM, James Orr <jorr at apple.com> wrote:
>
> On Mar 19, 2011, at 7:25 PM, Chris Lattner wrote:
>
>>
>> On Mar 19, 2011, at 3:24 PM, Eli Friedman wrote:
>>
>>> On Sat, Mar 19, 2011 at 3:15 PM, James Orr <jorr at apple.com> wrote:
>>>> Add support for built in 16bit byte swap.
>>>
>>> Why?
>>
>> Yes, why?  GCC doesn't support this either IIRC.
>>
>> -Chris
>
> I want to convert a big endian short to little endian (ie Cortex-M0 reading over I2C a 16bit ADC value returned in big endian).
>
> Clang does not recognize the following as a byte swap: (uint16_t)( a >> 8) | (uint16_t)( a << 8); (llvm-gcc does)

Hmm, interesting... that issue actually appears to be ARM-specific.
Hopefully it shouldn't be too hard to fix up InstCombiner::MatchBSwap
to do the right thing here.

> In the ARM architecture this can be accomplished with single instruction (REV16, or in the case I really care about REVSH)
>
> Even with this patch LLVM generates two instructions: REV (which is 32 bit byte swap) and LSRS.

That's the default LegalizeTypes promotion of a bswap to a wider type;
it should be possible to override in ARMISelLowering.cpp.

> I agree this should not be necessary if Clang be taught to recognize this itself, but I could not figure that out (yet). In the meantime this patch enables me to look into the LLVM half of generating the instructions.
>
> I realize GCC only supports 32, and 64, but LLVM supports 16 as well and I did not see the harm in exposing it.

We prefer to expose as few builtins as possible, so people don't start
using them and expect them to work in other compilers...

-Eli