| Message ID | 20241203-is_constexpr-refactor-v1-3-4e4cbaecc216@wanadoo.fr (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | compiler.h: refactor __is_constexpr() into is_const{,_true,_false}() | expand |
On Thu. 5 Dec 2024 at 03:48, David Laight <David.Laight@aculab.com> wrote: > From: Vincent Mailhol > > Sent: 02 December 2024 17:33 > > > > __builtin_constant_p() is known for not always being able to produce > > constant expression [1] which led to the introduction of > > __is_constexpr() [2]. Because of its dependency on > > __builtin_constant_p(), statically_true() suffers from the same > > issues. > > No, they are testing different things. OK, I will remove this paragraph. > > For example: > > > > void foo(int a) > > { > > /* fail on GCC */ > > BUILD_BUG_ON_ZERO(statically_true(a)); > > > > /* fail on both clang and GCC */ > > static char arr[statically_true(a) ? 1 : 2]; > > } > > > > Define a new is_const_true() and is_const_false() pair of macros > > which, by making use of __is_const_zero(), always produces a constant > > expression. > > > > Note that is_const_false() can not be directly defined as an alias to > > __is_const_zero(). Otherwise, it could yield some false positives on > > huge numbers because of a lost of precision when doing the (long) cast > > in __is_const_zero(). Example: > > > > is_const_false((u128)ULONG_MAX << BITS_PER_LONG) > > > > Furthermore, using the ! operator like this: > > > > #define is_const_true(x) __is_const_zero(!(x)) > > #define is_const_false(x) __is_const_zero(!!(x)) > > > > would yield a -Wint-in-bool-context compiler warning if the argument > > is not a boolean. Use the == and != operators instead. > > > > It should be noted that statically_true/false() are the only ones > > capable of folding tautologic expressions in which at least one on the > > operands is not a constant expression. For example: > > > > statically_true(true || var) > > statically_true(var == var) > > statically_false(var * 0) > > statically_false(var * 8 % 4) > > > > always evaluate to true, whereas all of these would be false under > > is_const_true/false() if var is not a constant expression [3]. > > > > For this reason, usage of const_true/false() should be the exception. > > Reflect in the documentation that const_true() is less powerful and > > that statically_true() is the overall preferred solution. > > > > [1] __builtin_constant_p cannot resolve to const when optimizing > > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=19449 > > > > [2] commit 3c8ba0d61d04 ("kernel.h: Retain constant expression output for max()/min()") > > Link: https://git.kernel.org/torvalds/c/3c8ba0d61d04 > > > > [3] https://godbolt.org/z/E4r7EaxW9 D'oh, I used some old versions of the macros in that link. The link will be updated to this in v2: https://godbolt.org/z/E4r7EaxW9 > > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > > --- > > include/linux/compiler.h | 23 +++++++++++++++++++++++ > > 1 file changed, 23 insertions(+) > > > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > > index 30ce06df4153cfdc0fad9bc7bffab9097f8b0450..165aa5b9bc484376087a130a1ac1f3edb50c983d 100644 > > --- a/include/linux/compiler.h > > +++ b/include/linux/compiler.h > > @@ -357,6 +357,29 @@ static inline void *offset_to_ptr(const int *off) > > */ > > #define is_const(x) __is_const_zero(0 * (x)) > > > > +/* > > + * Similar to statically_true() but produces a constant expression > > No. > It tests whether a value is a 'constant integer expression' and > the result is a 'constant integer expression'. > statically_true() checks for the value being a 'compile time constant'. I still would argue that ’constant integer expressions’ and ’compile time constants’ are *similar*. Not the same, agreed, but not drastically different either. I picked the term *similar* for that reason. > Most code really doesn't care, it all got added to min() so that > a very few places could do: > char foo[min(16, sizeof (type))]; > without triggering the 'variable length array' warning. > But that just bloated everywhere else and (IIRC) Linus replaced > them with a MIN() that was just an expression. What about: Return an integer constant expression while evaluating if the argument is a true (non zero) integer constant expression. > > + * > > + * To be used in conjunction with macros, such as BUILD_BUG_ON_ZERO(), > > + * which require their input to be a constant expression and for which > > + * statically_true() would otherwise fail. > > Use a different BUILD_BUG macro instead. > Look at the current definition of min(). Do you mean BUILD_BUG_ON_MSG()? That one, at the end, relies on the error attribute: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-error-function-attribute And the error attribute logic relies on compiler optimization. So BUILD_BUG_ON_MSG() is not a valid example here because it does not require its argument to be an integer constant expression. It works well with other compile time constants. Another valid example would be _Static_assert() but as a matter of fact, it is more common to use __is_constexpr() together with BUILD_BUG_ON_ZERO() than it is with _Static_assert(). So I think that BUILD_BUG_ON_ZERO() is best here. Yours sincerely, Vincent Mailhol
diff --git a/include/linux/compiler.h b/include/linux/compiler.h index 30ce06df4153cfdc0fad9bc7bffab9097f8b0450..165aa5b9bc484376087a130a1ac1f3edb50c983d 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -357,6 +357,29 @@ static inline void *offset_to_ptr(const int *off) */ #define is_const(x) __is_const_zero(0 * (x)) +/* + * Similar to statically_true() but produces a constant expression + * + * To be used in conjunction with macros, such as BUILD_BUG_ON_ZERO(), + * which require their input to be a constant expression and for which + * statically_true() would otherwise fail. + * + * This is a trade-off: is_const_true() requires all its operands to + * be compile time constants. Else, it would always returns false even + * on the most trivial cases like: + * + * true || non_const_expr + * + * On the opposite, statically_true() is able to fold more complex + * tautologies and will return true on expressions such as: + * + * !(non_const_expr * 8 % 4) + * + * For the general case, statically_true() is better. + */ +#define is_const_true(x) __is_const_zero((x) == 0) +#define is_const_false(x) __is_const_zero((x) != 0) + /* * This is needed in functions which generate the stack canary, see * arch/x86/kernel/smpboot.c::start_secondary() for an example.