Messages in this thread |  | | From | Ard Biesheuvel <> | Date | Wed, 1 Feb 2017 19:31:21 +0000 | Subject | Re: Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness |
| |
On 1 February 2017 at 19:04, Joe Perches <joe@perches.com> wrote: > On Wed, 2017-02-01 at 18:19 +0000, Ard Biesheuvel wrote: >> On 1 February 2017 at 17:36, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> > I still think order_base_2() is broken, since it may invoke >> > roundup_pow_of_two() with an input value that is documented as >> > producing undefined output. I would argue that the below is the >> > correct fix. >> > >> > diff --git a/include/linux/log2.h b/include/linux/log2.h >> > index fd7ff3d91e6a..46523731bec0 100644 >> > --- a/include/linux/log2.h >> > +++ b/include/linux/log2.h >> > @@ -203,6 +203,18 @@ unsigned long __rounddown_pow_of_two(unsigned long n) >> > * ... and so on. >> > */ >> > >> > -#define order_base_2(n) ilog2(roundup_pow_of_two(n)) >> > +static inline __attribute__((__const__)) >> > +unsigned long __order_base_2(unsigned long n) >> > +{ >> > + return n ? 1UL << fls_long(n - 1) : 1; >> > +} >> > + >> > +#define order_base_2(n) \ >> > +( \ >> > + __builtin_constant_p(n) ? ( \ >> > + ((n) < 2) ? (n) : \ >> > + ilog2((n) - 1) + 1) : \ >> > + ilog2(__order_base_2(n)) \ >> > + ) >> > >> > #endif /* _LINUX_LOG2_H */ >> >> Actually, there is a still a redundant shift/fls() in there, this is >> even simpler: >> >> diff --git a/include/linux/log2.h b/include/linux/log2.h >> index fd7ff3d91e6a..4741534bd7af 100644 >> --- a/include/linux/log2.h >> +++ b/include/linux/log2.h >> @@ -203,6 +203,18 @@ unsigned long __rounddown_pow_of_two(unsigned long n) >> * ... and so on. >> */ >> >> -#define order_base_2(n) ilog2(roundup_pow_of_two(n)) >> +static inline __attribute__((__const__)) > > commonly __attribute_const__ >
... except in <linux/ilog2.h>, which probably predates that.
>> +unsigned long __order_base_2(unsigned long n) >> +{ >> + return n > 1 ? ilog2(n - 1) + 1 : 0; >> +} >> + >> +#define order_base_2(n) \ >> +( \ >> + __builtin_constant_p(n) ? ( \ >> + ((n) < 2) ? (n) : \ >> + ilog2((n) - 1) + 1) : \ >> + __order_base_2(n) \ >> + ) > > Does this work properly when n is a signed negative value? >
No, but order_base_2() is explicitly only defined for inputs [0, ->), so its behavior for negative inputs is best left undefined.
|  |