[PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding - Kernel

This is a discussion on [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding - Kernel ; Create a helper macro to divide two numbers and round the result to the nearest whole number. This is a helper macro for hwmon drivers that want to convert incoming sysfs values per standard hwmon practice, though the macro itself ...

+ Reply to Thread
Results 1 to 11 of 11

Thread: [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding

  1. [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding


    Create a helper macro to divide two numbers and round the result to the
    nearest whole number. This is a helper macro for hwmon drivers that
    want to convert incoming sysfs values per standard hwmon practice, though
    the macro itself can be used by anyone.

    Signed-off-by: Darrick J. Wong
    ---

    include/linux/kernel.h | 6 ++++++
    1 files changed, 6 insertions(+), 0 deletions(-)

    diff --git a/include/linux/kernel.h b/include/linux/kernel.h
    index fba141d..fb02266 100644
    --- a/include/linux/kernel.h
    +++ b/include/linux/kernel.h
    @@ -48,6 +48,12 @@ extern const char linux_proc_banner[];
    #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
    #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
    #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
    +#define DIV_ROUND_CLOSEST(x, divisor)( \
    +{ \
    + typeof(divisor) __divisor = divisor; \
    + (((x) + ((__divisor) / 2)) / (__divisor)); \
    +} \
    +)

    #define _RET_IP_ (unsigned long)__builtin_return_address(0)
    #define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; })

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. [PATCH 2/2] adt74{62, 70, 73}: Use DIV_ROUND_CLOSEST for rounded division


    Modify some hwmon drivers to use DIV_ROUND_CLOSEST instead of bloating
    source with (naughty) macros.

    Signed-off-by: Darrick J. Wong
    ---

    drivers/hwmon/adt7462.c | 14 ++++++--------
    drivers/hwmon/adt7470.c | 8 +++-----
    drivers/hwmon/adt7473.c | 10 ++++------
    3 files changed, 13 insertions(+), 19 deletions(-)

    diff --git a/drivers/hwmon/adt7462.c b/drivers/hwmon/adt7462.c
    index 66107b4..1852f27 100644
    --- a/drivers/hwmon/adt7462.c
    +++ b/drivers/hwmon/adt7462.c
    @@ -204,8 +204,6 @@ I2C_CLIENT_INSMOD_1(adt7462);
    #define MASK_AND_SHIFT(value, prefix) \
    (((value) & prefix##_MASK) >> prefix##_SHIFT)

    -#define ROUND_DIV(x, divisor) (((x) + ((divisor) / 2)) / (divisor))
    -
    struct adt7462_data {
    struct device *hwmon_dev;
    struct attribute_group attrs;
    @@ -840,7 +838,7 @@ static ssize_t set_temp_min(struct device *dev,
    if (strict_strtol(buf, 10, &temp) || !temp_enabled(data, attr->index))
    return -EINVAL;

    - temp = ROUND_DIV(temp, 1000) + 64;
    + temp = DIV_ROUND_CLOSEST(temp, 1000) + 64;
    temp = SENSORS_LIMIT(temp, 0, 255);

    mutex_lock(&data->lock);
    @@ -878,7 +876,7 @@ static ssize_t set_temp_max(struct device *dev,
    if (strict_strtol(buf, 10, &temp) || !temp_enabled(data, attr->index))
    return -EINVAL;

    - temp = ROUND_DIV(temp, 1000) + 64;
    + temp = DIV_ROUND_CLOSEST(temp, 1000) + 64;
    temp = SENSORS_LIMIT(temp, 0, 255);

    mutex_lock(&data->lock);
    @@ -943,7 +941,7 @@ static ssize_t set_volt_max(struct device *dev,
    return -EINVAL;

    temp *= 1000; /* convert mV to uV */
    - temp = ROUND_DIV(temp, x);
    + temp = DIV_ROUND_CLOSEST(temp, x);
    temp = SENSORS_LIMIT(temp, 0, 255);

    mutex_lock(&data->lock);
    @@ -985,7 +983,7 @@ static ssize_t set_volt_min(struct device *dev,
    return -EINVAL;

    temp *= 1000; /* convert mV to uV */
    - temp = ROUND_DIV(temp, x);
    + temp = DIV_ROUND_CLOSEST(temp, x);
    temp = SENSORS_LIMIT(temp, 0, 255);

    mutex_lock(&data->lock);
    @@ -1250,7 +1248,7 @@ static ssize_t set_pwm_hyst(struct device *dev,
    if (strict_strtol(buf, 10, &temp))
    return -EINVAL;

    - temp = ROUND_DIV(temp, 1000);
    + temp = DIV_ROUND_CLOSEST(temp, 1000);
    temp = SENSORS_LIMIT(temp, 0, 15);

    /* package things up */
    @@ -1337,7 +1335,7 @@ static ssize_t set_pwm_tmin(struct device *dev,
    if (strict_strtol(buf, 10, &temp))
    return -EINVAL;

    - temp = ROUND_DIV(temp, 1000) + 64;
    + temp = DIV_ROUND_CLOSEST(temp, 1000) + 64;
    temp = SENSORS_LIMIT(temp, 0, 255);

    mutex_lock(&data->lock);
    diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
    index 1311a59..da6c930 100644
    --- a/drivers/hwmon/adt7470.c
    +++ b/drivers/hwmon/adt7470.c
    @@ -137,8 +137,6 @@ I2C_CLIENT_INSMOD_1(adt7470);
    #define FAN_PERIOD_INVALID 65535
    #define FAN_DATA_VALID(x) ((x) && (x) != FAN_PERIOD_INVALID)

    -#define ROUND_DIV(x, divisor) (((x) + ((divisor) / 2)) / (divisor))
    -
    struct adt7470_data {
    struct device *hwmon_dev;
    struct attribute_group attrs;
    @@ -360,7 +358,7 @@ static ssize_t set_temp_min(struct device *dev,
    if (strict_strtol(buf, 10, &temp))
    return -EINVAL;

    - temp = ROUND_DIV(temp, 1000);
    + temp = DIV_ROUND_CLOSEST(temp, 1000);
    temp = SENSORS_LIMIT(temp, 0, 255);

    mutex_lock(&data->lock);
    @@ -394,7 +392,7 @@ static ssize_t set_temp_max(struct device *dev,
    if (strict_strtol(buf, 10, &temp))
    return -EINVAL;

    - temp = ROUND_DIV(temp, 1000);
    + temp = DIV_ROUND_CLOSEST(temp, 1000);
    temp = SENSORS_LIMIT(temp, 0, 255);

    mutex_lock(&data->lock);
    @@ -671,7 +669,7 @@ static ssize_t set_pwm_tmin(struct device *dev,
    if (strict_strtol(buf, 10, &temp))
    return -EINVAL;

    - temp = ROUND_DIV(temp, 1000);
    + temp = DIV_ROUND_CLOSEST(temp, 1000);
    temp = SENSORS_LIMIT(temp, 0, 255);

    mutex_lock(&data->lock);
    diff --git a/drivers/hwmon/adt7473.c b/drivers/hwmon/adt7473.c
    index 18aa308..0a6ce23 100644
    --- a/drivers/hwmon/adt7473.c
    +++ b/drivers/hwmon/adt7473.c
    @@ -129,8 +129,6 @@ I2C_CLIENT_INSMOD_1(adt7473);
    #define FAN_PERIOD_INVALID 65535
    #define FAN_DATA_VALID(x) ((x) && (x) != FAN_PERIOD_INVALID)

    -#define ROUND_DIV(x, divisor) (((x) + ((divisor) / 2)) / (divisor))
    -
    struct adt7473_data {
    struct device *hwmon_dev;
    struct attribute_group attrs;
    @@ -459,7 +457,7 @@ static ssize_t set_temp_min(struct device *dev,
    if (strict_strtol(buf, 10, &temp))
    return -EINVAL;

    - temp = ROUND_DIV(temp, 1000);
    + temp = DIV_ROUND_CLOSEST(temp, 1000);
    temp = encode_temp(data->temp_twos_complement, temp);

    mutex_lock(&data->lock);
    @@ -495,7 +493,7 @@ static ssize_t set_temp_max(struct device *dev,
    if (strict_strtol(buf, 10, &temp))
    return -EINVAL;

    - temp = ROUND_DIV(temp, 1000);
    + temp = DIV_ROUND_CLOSEST(temp, 1000);
    temp = encode_temp(data->temp_twos_complement, temp);

    mutex_lock(&data->lock);
    @@ -720,7 +718,7 @@ static ssize_t set_temp_tmax(struct device *dev,
    if (strict_strtol(buf, 10, &temp))
    return -EINVAL;

    - temp = ROUND_DIV(temp, 1000);
    + temp = DIV_ROUND_CLOSEST(temp, 1000);
    temp = encode_temp(data->temp_twos_complement, temp);

    mutex_lock(&data->lock);
    @@ -756,7 +754,7 @@ static ssize_t set_temp_tmin(struct device *dev,
    if (strict_strtol(buf, 10, &temp))
    return -EINVAL;

    - temp = ROUND_DIV(temp, 1000);
    + temp = DIV_ROUND_CLOSEST(temp, 1000);
    temp = encode_temp(data->temp_twos_complement, temp);

    mutex_lock(&data->lock);

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding

    Hi Darrick,

    On Mon, 10 Nov 2008 17:01:32 -0800, Darrick J. Wong wrote:
    >
    > Create a helper macro to divide two numbers and round the result to the
    > nearest whole number. This is a helper macro for hwmon drivers that
    > want to convert incoming sysfs values per standard hwmon practice, though
    > the macro itself can be used by anyone.
    >
    > Signed-off-by: Darrick J. Wong
    > ---
    >
    > include/linux/kernel.h | 6 ++++++
    > 1 files changed, 6 insertions(+), 0 deletions(-)
    >
    > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
    > index fba141d..fb02266 100644
    > --- a/include/linux/kernel.h
    > +++ b/include/linux/kernel.h
    > @@ -48,6 +48,12 @@ extern const char linux_proc_banner[];
    > #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
    > #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
    > #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
    > +#define DIV_ROUND_CLOSEST(x, divisor)( \
    > +{ \
    > + typeof(divisor) __divisor = divisor; \
    > + (((x) + ((__divisor) / 2)) / (__divisor)); \
    > +} \
    > +)
    >
    > #define _RET_IP_ (unsigned long)__builtin_return_address(0)
    > #define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; })
    >


    I don't get why you implement this as a macro rather than an inline
    function? A function would look much better.

    --
    Jean Delvare
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding

    On Tue, 11 Nov 2008 11:04:54 +0100 Jean Delvare wrote:

    > Hi Darrick,
    >
    > On Mon, 10 Nov 2008 17:01:32 -0800, Darrick J. Wong wrote:
    > >
    > > Create a helper macro to divide two numbers and round the result to the
    > > nearest whole number. This is a helper macro for hwmon drivers that
    > > want to convert incoming sysfs values per standard hwmon practice, though
    > > the macro itself can be used by anyone.
    > >
    > > Signed-off-by: Darrick J. Wong
    > > ---
    > >
    > > include/linux/kernel.h | 6 ++++++
    > > 1 files changed, 6 insertions(+), 0 deletions(-)
    > >
    > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
    > > index fba141d..fb02266 100644
    > > --- a/include/linux/kernel.h
    > > +++ b/include/linux/kernel.h
    > > @@ -48,6 +48,12 @@ extern const char linux_proc_banner[];
    > > #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
    > > #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
    > > #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
    > > +#define DIV_ROUND_CLOSEST(x, divisor)( \
    > > +{ \
    > > + typeof(divisor) __divisor = divisor; \
    > > + (((x) + ((__divisor) / 2)) / (__divisor)); \
    > > +} \
    > > +)
    > >
    > > #define _RET_IP_ (unsigned long)__builtin_return_address(0)
    > > #define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; })
    > >

    >
    > I don't get why you implement this as a macro rather than an inline
    > function? A function would look much better.


    The idea is that DIV_ROUND_CLOSEST() can be used with arguments of any
    size (char, short, ... long long) and will do all the suitable
    promotion and will return a type of the appropriate width and
    signedness.

    It's not particularly pretty and can hide traps and pitfalls, but the
    other way is tricky as well - it'd need a family of functions and
    there's a risk that programmers will choose the wrong one.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding

    On Tue, 11 Nov 2008 09:07:02 -0800, Andrew Morton wrote:
    > On Tue, 11 Nov 2008 11:04:54 +0100 Jean Delvare wrote:
    >
    > > Hi Darrick,
    > >
    > > On Mon, 10 Nov 2008 17:01:32 -0800, Darrick J. Wong wrote:
    > > >
    > > > Create a helper macro to divide two numbers and round the result to the
    > > > nearest whole number. This is a helper macro for hwmon drivers that
    > > > want to convert incoming sysfs values per standard hwmon practice, though
    > > > the macro itself can be used by anyone.
    > > >
    > > > Signed-off-by: Darrick J. Wong
    > > > ---
    > > >
    > > > include/linux/kernel.h | 6 ++++++
    > > > 1 files changed, 6 insertions(+), 0 deletions(-)
    > > >
    > > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
    > > > index fba141d..fb02266 100644
    > > > --- a/include/linux/kernel.h
    > > > +++ b/include/linux/kernel.h
    > > > @@ -48,6 +48,12 @@ extern const char linux_proc_banner[];
    > > > #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
    > > > #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
    > > > #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
    > > > +#define DIV_ROUND_CLOSEST(x, divisor)( \
    > > > +{ \
    > > > + typeof(divisor) __divisor = divisor; \
    > > > + (((x) + ((__divisor) / 2)) / (__divisor)); \
    > > > +} \
    > > > +)
    > > >
    > > > #define _RET_IP_ (unsigned long)__builtin_return_address(0)
    > > > #define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; })
    > > >

    > >
    > > I don't get why you implement this as a macro rather than an inline
    > > function? A function would look much better.

    >
    > The idea is that DIV_ROUND_CLOSEST() can be used with arguments of any
    > size (char, short, ... long long) and will do all the suitable
    > promotion and will return a type of the appropriate width and
    > signedness.
    >
    > It's not particularly pretty and can hide traps and pitfalls, but the
    > other way is tricky as well - it'd need a family of functions and
    > there's a risk that programmers will choose the wrong one.


    OK, I get it now, thanks for the clarification.

    --
    Jean Delvare
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding

    On Tue, 2008-11-11 at 09:07 -0800, Andrew Morton wrote:
    > On Tue, 11 Nov 2008 11:04:54 +0100 Jean Delvare wrote:
    > > On Mon, 10 Nov 2008 17:01:32 -0800, Darrick J. Wong wrote:
    > > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
    > > > index fba141d..fb02266 100644
    > > > --- a/include/linux/kernel.h
    > > > +++ b/include/linux/kernel.h
    > > > @@ -48,6 +48,12 @@ extern const char linux_proc_banner[];
    > > > #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
    > > > #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
    > > > #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
    > > > +#define DIV_ROUND_CLOSEST(x, divisor)( \
    > > > +{ \
    > > > + typeof(divisor) __divisor = divisor; \
    > > > + (((x) + ((__divisor) / 2)) / (__divisor)); \
    > > > +} \
    > > > +)

    > > I don't get why you implement this as a macro rather than an inline
    > > function? A function would look much better.

    > The idea is that DIV_ROUND_CLOSEST() can be used with arguments of any
    > size (char, short, ... long long) and will do all the suitable
    > promotion and will return a type of the appropriate width and
    > signedness.


    Perhaps the macro should be placed directly after
    DIV_ROUND_UP and should use the same argument naming.

    Perhaps HALF_UP is more descriptive and fairly common.
    http://java.sun.com/j2se/1.5.0/docs/...ndingMode.html


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [lm-sensors] [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding

    On Mon, 10 Nov 2008, Darrick J. Wong wrote:
    > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
    > index fba141d..fb02266 100644
    > --- a/include/linux/kernel.h
    > +++ b/include/linux/kernel.h
    > @@ -48,6 +48,12 @@ extern const char linux_proc_banner[];
    > #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
    > #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
    > #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
    > +#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?

    Note that if divisor is a signed variable, divisor/2 generates worse code
    than divisor>>1.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [lm-sensors] [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding

    On Tue, 11 Nov 2008 15:05:02 -0800 (PST)
    Trent Piepho wrote:

    > On Mon, 10 Nov 2008, Darrick J. Wong wrote:
    > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
    > > index fba141d..fb02266 100644
    > > --- a/include/linux/kernel.h
    > > +++ b/include/linux/kernel.h
    > > @@ -48,6 +48,12 @@ extern const char linux_proc_banner[];
    > > #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
    > > #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
    > > #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
    > > +#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.

    > Note that if divisor is a signed variable, divisor/2 generates worse code
    > than divisor>>1.


    yup. I wonder why the compiler doesn't do that for itself - is there a
    case where it will generate a different result?

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [lm-sensors] [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding

    On Tue, 11 Nov 2008, Andrew Morton wrote:
    > On Tue, 11 Nov 2008 15:05:02 -0800 (PST)
    > Trent Piepho wrote:
    >> On Mon, 10 Nov 2008, Darrick J. Wong wrote:
    >>> #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
    >>> #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
    >>> #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
    >>> +#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?

    >> Note that if divisor is a signed variable, divisor/2 generates worse code
    >> than divisor>>1.

    >
    > yup. I wonder why the compiler doesn't do that for itself - is there a
    > case where it will generate a different result?


    main()
    {
    int x = -5;
    printf("%d %d\n", x>>1, x/2);
    }
    $ a.out
    -3 -2
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [lm-sensors] [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding

    Hi,

    2008/11/11 Andrew Morton :
    > yup. I wonder why the compiler doesn't do that for itself - is there a
    > case where it will generate a different result?


    The test program

    #include
    int
    main()
    {
    signed int x = -1;
    printf("%d %d\n", x/2, x>>1);
    return 0;
    }

    says

    0 -1

    so it seems to make a difference.

    All the best,
    Jochen
    --
    http://seehuhn.de/
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [lm-sensors] [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding

    On Tue, 11 Nov 2008 15:42:09 -0800 (PST)
    Trent Piepho wrote:

    > On Tue, 11 Nov 2008, Andrew Morton wrote:
    > > On Tue, 11 Nov 2008 15:05:02 -0800 (PST)
    > > Trent Piepho wrote:
    > >> On Mon, 10 Nov 2008, Darrick J. Wong wrote:
    > >>> #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
    > >>> #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
    > >>> #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
    > >>> +#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.

    > >> Note that if divisor is a signed variable, divisor/2 generates worse code
    > >> than divisor>>1.

    > >
    > > yup. I wonder why the compiler doesn't do that for itself - is there a
    > > case where it will generate a different result?

    >
    > main()
    > {
    > int x = -5;
    > printf("%d %d\n", x>>1, x/2);
    > }
    > $ a.out
    > -3 -2


    ah, thanks.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread