[llvm-commits] [PATCH][System] Add SwapByteOrder and move implementation from Support/MathExtras.h.

Michael Spencer bigcheesegs at gmail.com
Fri Oct 1 15:13:15 PDT 2010


On Fri, Oct 1, 2010 at 3:28 PM, Rafael Espindola <espindola at google.com> wrote:
>> I added unittests to verify signed swaps work, and modified the
>> implementation to error out if any type is used other than
>> {signed,unsigned} {8,16,32,64}bit integer types.
>>
>> New patch attached.
>
> Sorry for the ultra long delay.  My only comment on the implementation
> now is that it might be a good idea to just remove ByteSwap_XX and use
> sys::SwapByteOrder directly, but that can be done in another patch.
>
> As for the test, I really like that you added it, but please don't use
> random numbers :-)

K, I removed them and added a deterministic test that tests the same thing.

> Can you also add quick test that x = swap(swap(x)) even for signed x?

These were included in the random test, and are now moved into the the
deterministic tests.

>> - Michael Spencer
>>
>
> Thanks,
> --
> Rafael Ávila de Espíndola
>

Thanks again for the reviews!

- Michael Spencer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: SwapByteOrder.patch
Type: application/octet-stream
Size: 10726 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20101001/7dbc20b0/attachment.obj>


More information about the llvm-commits mailing list