| Message ID | 20241112190840.601378-5-mailhol.vincent@wanadoo.fr (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | add _statically_true() to simplify GENMASK_INPUT_CHECK() | expand |
On Wed, Nov 13 2024, Vincent Mailhol <mailhol.vincent@wanadoo.fr> wrote: > > Declare a new _statically_true() macro which, by making use of the > __builtin_choose_expr() and __is_constexpr(x) combo, always produces a > constant expression. Looks sane, but $subject needs s/_static_assert/_statically_true/. And to be completely pedantic, macros are defined, not declared. > > - above examples, and a bit more: > > https://godbolt.org/z/zzqM1ajPj > > - a proof that statically_true() does better constant folding than _statically_true() > > https://godbolt.org/z/vK6KK4hMG At least the second of these might be good to refer to in the commit message itself. Rasmus
On Wed, Nov 13, 2024 at 04:08:39AM +0900, Vincent Mailhol wrote: > __builtin_constant_p() is known for not always being able to produce > constant expression [1] which lead to the introduction of > __is_constexpr() [2]. Because of its dependency on > __builtin_constant_p(), statically_true() suffers from the same > issues. > > For example: > > void foo(int a) > { > /* fail on GCC */ > BUILD_BUG_ON_ZERO(statically_true(a)); > > /* fail both clang and GCC */ > static char arr[statically_true(a) ? 1 : 2]; > } > > For the same reasons why __is_constexpr() was created to cover > __builtin_constant_p() edge cases, __is_constexpr() can be used to > resolve statically_true() limitations. > > Note that, somehow, GCC is not always able to fold this: > > __is_constexpr(x) && (x) > > It is OK in BUILD_BUG_ON_ZERO() but not in array declarations or in > static_assert(): > > void bar(int a) > { > /* success */ > BUILD_BUG_ON_ZERO(__is_constexpr(a) && (a)); > > /* fail on GCC */ > static char arr[__is_constexpr(a) && (a) ? 1 : 2]; > > /* fail on GCC */ > static_assert(__is_constexpr(a) && (a)); > } > > Encapsulating the expression in a __builtin_choose_expr() switch > resolves all these failed test. > > Declare a new _statically_true() macro which, by making use of the > __builtin_choose_expr() and __is_constexpr(x) combo, always produces a > constant expression. So, maybe name it const_true() then? > It should be noted that statically_true() still produces better > folding: > > statically_true(!(var * 8 % 8)) > > always evaluates to true even if var is unknown, whereas > > _statically_true(!(var * 8 % 8)) > > fails to fold the expression and return false. > > For this reason, usage of _statically_true() be should the exception. > Reflect in the documentation that _statically_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()") > > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > --- > Bonuses: > > - above examples, and a bit more: > > https://godbolt.org/z/zzqM1ajPj > > - a proof that statically_true() does better constant folding than _statically_true() > > https://godbolt.org/z/vK6KK4hMG > --- > include/linux/compiler.h | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > index 4d4e23b6e3e7..c76db8b50202 100644 > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -308,6 +308,20 @@ static inline void *offset_to_ptr(const int *off) > */ > #define statically_true(x) (__builtin_constant_p(x) && (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 tradeoff: _statically_true() is less efficient at > + * constant folding and will fail to optimize any expressions in which > + * at least one of the subcomponent is not constant. For the general > + * case, statically_true() is better. I agree with Rasmus. Would be nice to have examples where should I use one vs another right here in the comment. > + */ > +#define _statically_true(x) __builtin_choose_expr(__is_constexpr(x), x, false) > + > /* > * This is needed in functions which generate the stack canary, see > * arch/x86/kernel/smpboot.c::start_secondary() for an example. > -- > 2.45.2
On 13/11/2024 at 05:26, Yury Norov wrote: > On Wed, Nov 13, 2024 at 04:08:39AM +0900, Vincent Mailhol wrote: >> __builtin_constant_p() is known for not always being able to produce >> constant expression [1] which lead to the introduction of >> __is_constexpr() [2]. Because of its dependency on >> __builtin_constant_p(), statically_true() suffers from the same >> issues. >> >> For example: >> >> void foo(int a) >> { >> /* fail on GCC */ >> BUILD_BUG_ON_ZERO(statically_true(a)); >> >> /* fail both clang and GCC */ >> static char arr[statically_true(a) ? 1 : 2]; >> } >> >> For the same reasons why __is_constexpr() was created to cover >> __builtin_constant_p() edge cases, __is_constexpr() can be used to >> resolve statically_true() limitations. >> >> Note that, somehow, GCC is not always able to fold this: >> >> __is_constexpr(x) && (x) >> >> It is OK in BUILD_BUG_ON_ZERO() but not in array declarations or in >> static_assert(): >> >> void bar(int a) >> { >> /* success */ >> BUILD_BUG_ON_ZERO(__is_constexpr(a) && (a)); >> >> /* fail on GCC */ >> static char arr[__is_constexpr(a) && (a) ? 1 : 2]; >> >> /* fail on GCC */ >> static_assert(__is_constexpr(a) && (a)); >> } >> >> Encapsulating the expression in a __builtin_choose_expr() switch >> resolves all these failed test. >> >> Declare a new _statically_true() macro which, by making use of the >> __builtin_choose_expr() and __is_constexpr(x) combo, always produces a >> constant expression. > So, maybe name it const_true() then? OK. I pretty like the _statically_true() because the link with statically_true() was obvious and the _ underscore prefix hinted that this variant was "special". But I have to admit that the const_true() is also really nice, and I finally adopted it in the v4. >> It should be noted that statically_true() still produces better >> folding: >> >> statically_true(!(var * 8 % 8)) >> >> always evaluates to true even if var is unknown, whereas >> >> _statically_true(!(var * 8 % 8)) >> >> fails to fold the expression and return false. >> >> For this reason, usage of _statically_true() be should the exception. >> Reflect in the documentation that _statically_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()") >> >> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> >> --- >> Bonuses: >> >> - above examples, and a bit more: >> >> https://godbolt.org/z/zzqM1ajPj >> >> - a proof that statically_true() does better constant folding than _statically_true() >> >> https://godbolt.org/z/vK6KK4hMG >> --- >> include/linux/compiler.h | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/include/linux/compiler.h b/include/linux/compiler.h >> index 4d4e23b6e3e7..c76db8b50202 100644 >> --- a/include/linux/compiler.h >> +++ b/include/linux/compiler.h >> @@ -308,6 +308,20 @@ static inline void *offset_to_ptr(const int *off) >> */ >> #define statically_true(x) (__builtin_constant_p(x) && (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 tradeoff: _statically_true() is less efficient at >> + * constant folding and will fail to optimize any expressions in which >> + * at least one of the subcomponent is not constant. For the general >> + * case, statically_true() is better. > I agree with Rasmus. Would be nice to have examples where should I use > one vs another right here in the comment. I rewrote the full set of examples in v4. I added the godbolt link in the patch description and I cherry picked what seems to me the two most meaningful examples and put them in the macro comment. Let me know what you think. Yours sincerely, Vincent Mailhol
On 13/11/2024 at 05:03, Rasmus Villemoes wrote: > On Wed, Nov 13 2024, Vincent Mailhol <mailhol.vincent@wanadoo.fr> wrote: > >> Declare a new _statically_true() macro which, by making use of the >> __builtin_choose_expr() and __is_constexpr(x) combo, always produces a >> constant expression. > Looks sane, but $subject needs s/_static_assert/_statically_true/. And > to be completely pedantic, macros are defined, not declared. Sorry for the silly mistake in the subject. And agreed that "define" is more appropriate than "declare" when speaking of macros. >> - above examples, and a bit more: >> >> https://godbolt.org/z/zzqM1ajPj >> >> - a proof that statically_true() does better constant folding than _statically_true() >> >> https://godbolt.org/z/vK6KK4hMG > At least the second of these might be good to refer to in the commit > message itself. Ack. I rewrote the set of examples in the v4 and added a few samples to the macro comment. Yours sincerely, Vincent Mailhol
diff --git a/include/linux/compiler.h b/include/linux/compiler.h index 4d4e23b6e3e7..c76db8b50202 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -308,6 +308,20 @@ static inline void *offset_to_ptr(const int *off) */ #define statically_true(x) (__builtin_constant_p(x) && (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 tradeoff: _statically_true() is less efficient at + * constant folding and will fail to optimize any expressions in which + * at least one of the subcomponent is not constant. For the general + * case, statically_true() is better. + */ +#define _statically_true(x) __builtin_choose_expr(__is_constexpr(x), x, false) + /* * This is needed in functions which generate the stack canary, see * arch/x86/kernel/smpboot.c::start_secondary() for an example.
__builtin_constant_p() is known for not always being able to produce constant expression [1] which lead to the introduction of __is_constexpr() [2]. Because of its dependency on __builtin_constant_p(), statically_true() suffers from the same issues. For example: void foo(int a) { /* fail on GCC */ BUILD_BUG_ON_ZERO(statically_true(a)); /* fail both clang and GCC */ static char arr[statically_true(a) ? 1 : 2]; } For the same reasons why __is_constexpr() was created to cover __builtin_constant_p() edge cases, __is_constexpr() can be used to resolve statically_true() limitations. Note that, somehow, GCC is not always able to fold this: __is_constexpr(x) && (x) It is OK in BUILD_BUG_ON_ZERO() but not in array declarations or in static_assert(): void bar(int a) { /* success */ BUILD_BUG_ON_ZERO(__is_constexpr(a) && (a)); /* fail on GCC */ static char arr[__is_constexpr(a) && (a) ? 1 : 2]; /* fail on GCC */ static_assert(__is_constexpr(a) && (a)); } Encapsulating the expression in a __builtin_choose_expr() switch resolves all these failed test. Declare a new _statically_true() macro which, by making use of the __builtin_choose_expr() and __is_constexpr(x) combo, always produces a constant expression. It should be noted that statically_true() still produces better folding: statically_true(!(var * 8 % 8)) always evaluates to true even if var is unknown, whereas _statically_true(!(var * 8 % 8)) fails to fold the expression and return false. For this reason, usage of _statically_true() be should the exception. Reflect in the documentation that _statically_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()") Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> --- Bonuses: - above examples, and a bit more: https://godbolt.org/z/zzqM1ajPj - a proof that statically_true() does better constant folding than _statically_true() https://godbolt.org/z/vK6KK4hMG --- include/linux/compiler.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)