Re: [boost] Boost.En­dian comments

Subject: Re: [boost] Boost.En­dian comments
From: Giovanni Piero Deretta (gpderetta_at_[hidden])
Date: 2011-09-07 13:37:03

On Wed, Sep 7, 2011 at 6:15 PM, Beman Dawes <bdawes_at_[hidden]> wrote:
> On Mon, Sep 5, 2011 at 2:56 PM, tymofey <tymofey_at_[hidden]> wrote:
>> This is not a full review, but rather a few comments on the implementation of reorder(x) functions.
>> What you are doing is casting pointers around and assigning individual bytes, which might not really effective:
>>
>> inline void reorder(int64_t source, int64_t& target)
>> {
>>    const char* s (reinterpret_cast<const char*>(&source));
>>    char * t (reinterpret_cast<char*>(&target) + sizeof(target) - 1);
>>    *t = *s;
>>    *--t = *++s;
>>    *--t = *++s;
>>    *--t = *++s;
>>    *--t = *++s;
>>    *--t = *++s;
>>    *--t = *++s;
>>    *--t = *++s;
>> }
>>
>> it does eight increments, eight decrements, one addition, one substraction and eight assignments.
>> maybe something like:
>>
>> inline void reorder(int64_t source, int64_t& target)
>> {
>>    target = ((source << 0x38) & 0xFF00000000000000)
>>           | ((source << 0x28) & 0x00FF000000000000)
>>           | ((source << 0x18) & 0x0000FF0000000000)
>>           | ((source << 0x08) & 0x000000FF00000000)
>>           | ((source >> 0x08) & 0x00000000FF000000)
>>           | ((source >> 0x18) & 0x0000000000FF0000)
>>           | ((source >> 0x28) & 0x000000000000FF00)
>>           | ((source >> 0x38) & 0x00000000000000FF);
>> }
>>
>> would be more efficient?
>
> Yes, but if I read the standard correctly, it is relying on undefined behavior.
>
> Both the C and C++ standards say:
>
> The value of E1 << E2 is E1 left-shifted E2 bit positions; vacated
> bits are zero-filled. If E1 has an unsigned type, the value of the
> result is E1 × 2 [to the power] E2, reduced modulo one more than the
> maximum value representable in the result type. Otherwise, if E1 has a
> signed type and non-negative value, and E1×2 [to the power] E2 is
> representable in the result type, then that is the resulting value;
> otherwise, the behavior is undefined.
>
> In practice I don't think this is a problem, but Boost code generally
> doesn't rely on code that has undefined behavior. Perhaps it would be
> OK to #ifdef on the architecture, and use shift-and-mask
> implementations only on architectures that don't do something weird
> (like cause an interrupt) in the case covered by the last sentence.
>

If you use uint64_t instead of int64_t (always the right thing when
doing bit-twidling), there shouldn't be any UB, right?

-- 
gpd