On Tue, 11 Nov 2008, Andrew Morton wrote:
>>>>> +#define DIV_ROUND_CLOSEST(x, divisor)( \
>>>>> +{ \
>>>>> + typeof(divisor) __divisor = divisor; \
>>>>> + (((x) + ((__divisor) / 2)) / (__divisor)); \
>>>>> +} \
>>>>> +)
>>>>
>>>> Maybe you can do away with the statement-expression extension? I've seen
>>>> cases where it cases gcc to generate worse code. It seems like it
>>>> shouldn't, but it does. I know DIV_ROUND_CLOSEST (maybe DIV_ROUND_NEAR?)
>>>> uses divisor twice, but all the also divide macros do that too, so why does
>>>> this one need to be different?
>>>
>>> The others need fixing too.
>>
>> Is it worth generating worse code for these simple macros?
>
> Well that's an interesting question.
>
> The risks with the current code are
>
> a) It will introduce straightforward bugs, where pointers are
> incremented twice, etc.
>
> Hopefully these things will be apparent during testing and we'll
> fix them up in the usual fashion.
>
> b) It will introduce subtle slowdowns due to needlessly executing
> code more than once, as in the hugepage case which I identified.
> These problems will hang around for long periods.
>
> So they're good reasons to fix the macros. If these fixes cause the
> compiler to generate worse code then we should quantify and understand
> that. Perhaps it is only certain compiler versions. Perhaps we can
> find a test case (should be easy?) and send it over to the gcc guys to
> fix. Perhaps we can find some C-level construct which prevents the
> compiler from going into stupid mode without reintroducing the existing
> problems.
My question was more along the lines of is it worth it to even have macros for
something as simple rounding up when dividing?
For an example of statement expression problems, I noticed something with
swab16(), addressed in commit 8e2c20023f34b652605a5fb7c68bb843d2b100a8
#define ___swab16(x) \
({ \
__u16 __x = (x); \
((__u16)( \
(((__u16)(__x) & (__u16)0x00ffU) << 8) | \
(((__u16)(__x) & (__u16)0xff00U) >> 8) )); \
})
Produces this code:
movzwl %ax, %eax
movl %eax, %edx
shrl $8, %eax
sall $8, %edx
orl %eax, %edx
While this:
static __inline__ __attribute_const__ __u16 ___swab16(__u16 x)
{
return x<<8 | x>>8;
}
Produces this code:
rolw $8, %ax
