lkml.org 
[lkml]   [2008]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [lm-sensors] [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding
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



\
 
 \ /
  Last update: 2008-11-14 22:59    [W:0.078 / U:0.160 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site