Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness

On 1 February 2017 at 19:04, Joe Perches [off-list ref] wrote:

On Wed, 2017-02-01 at 18:19 +0000, Ard Biesheuvel wrote:

quoted

On 1 February 2017 at 17:36, Ard Biesheuvel [off-list ref] wrote:

quoted

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.

quoted

+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.