* [PATCH v3 0/2] add _statically_true() to simplify GENMASK_INPUT_CHECK()
@ 2024-11-12 13:59 Vincent Mailhol
2024-11-12 13:59 ` [PATCH v3 1/2] compiler.h: add _static_assert() Vincent Mailhol
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Vincent Mailhol @ 2024-11-12 13:59 UTC (permalink / raw)
To: Linus Torvalds, Yury Norov, Rasmus Villemoes, Luc Van Oostenryck
Cc: linux-kernel, linux-sparse, Rikard Falkeborn, Vincent Mailhol
The first patch introduces a new variant of statically_true() named
_statically_true() which rely on __is_constexpr() to produce a
constant expression result which can be used in BUILD_BUG_ON_ZERO()
and other macros which expect a constant expression as input.
The second patch applies this newly created _statically_true() to
GENMASK_INPUT_CHECK().
** Changelog **
v2 -> v3:
- split the single patch into a series of two patches.
- add explanation of why _statically_true() is needed in addition
to the existing statically_true(). Explain the pros and cons of
each.
- use __builtin_choose_expr() in _statically_true(). The
_statically_true() of the v2 works perfectly fine when used in
conjunction with BUILD_BUG_ON_ZERO() but fails if used, for
example, in arrays or in static_assert().
Link: https://lore.kernel.org/all/20241111164743.339117-2-mailhol.vincent@wanadoo.fr/
v1 -> v2:
- introduce _statically_true(), taking inspiration from
statically_true() as introduced in commit 22f546873149 ("minmax:
improve macro expansion and type checking").
Link: https://lore.kernel.org/all/20240609073513.256179-1-mailhol.vincent@wanadoo.fr/
Vincent Mailhol (2):
compiler.h: add _static_assert()
linux/bits.h: simplify GENMASK_INPUT_CHECK()
include/linux/bits.h | 5 ++---
include/linux/compiler.h | 14 ++++++++++++++
2 files changed, 16 insertions(+), 3 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] compiler.h: add _static_assert()
2024-11-12 13:59 [PATCH v3 0/2] add _statically_true() to simplify GENMASK_INPUT_CHECK() Vincent Mailhol
@ 2024-11-12 13:59 ` Vincent Mailhol
2024-11-12 13:59 ` [PATCH v3 2/2] linux/bits.h: simplify GENMASK_INPUT_CHECK() Vincent Mailhol
2024-11-12 17:52 ` [PATCH v3 0/2] add _statically_true() to " Linus Torvalds
2 siblings, 0 replies; 5+ messages in thread
From: Vincent Mailhol @ 2024-11-12 13:59 UTC (permalink / raw)
To: Linus Torvalds, Yury Norov, Rasmus Villemoes, Luc Van Oostenryck
Cc: linux-kernel, linux-sparse, Rikard Falkeborn, Vincent Mailhol
__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.
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];
}
For the same reasons why __is_constexpr() was created to remediate
__builtin_constant_p() edge cases, __is_constexpr() can be used to
resolve statically_true()'s 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 examples.
Declare a new _statically_true() macro which, by making use of the
__builtin_choose_expr() and __is_constexpr() 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 returns false.
For this reason, usage of _statically_true() should be 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) in godbolt:
https://godbolt.org/z/GYeEK5d7s
- that proof, in godbolt, that statically_true() is bettera at
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 subcomponents 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.
--
2.45.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] linux/bits.h: simplify GENMASK_INPUT_CHECK()
2024-11-12 13:59 [PATCH v3 0/2] add _statically_true() to simplify GENMASK_INPUT_CHECK() Vincent Mailhol
2024-11-12 13:59 ` [PATCH v3 1/2] compiler.h: add _static_assert() Vincent Mailhol
@ 2024-11-12 13:59 ` Vincent Mailhol
2024-11-12 17:52 ` [PATCH v3 0/2] add _statically_true() to " Linus Torvalds
2 siblings, 0 replies; 5+ messages in thread
From: Vincent Mailhol @ 2024-11-12 13:59 UTC (permalink / raw)
To: Linus Torvalds, Yury Norov, Rasmus Villemoes, Luc Van Oostenryck
Cc: linux-kernel, linux-sparse, Rikard Falkeborn, Vincent Mailhol
In GENMASK_INPUT_CHECK(),
__builtin_choose_expr(__is_constexpr((l) > (h)), (l) > (h), 0)
is the exact expansion of:
_statically_true((l) > (h))
Apply _statically_true() to simplify GENMASK_INPUT_CHECK().
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
This change passes the unit tests from CONFIG_BITS_TEST, including the
extra negative tests provided under #ifdef TEST_GENMASK_FAILURES [1].
[1] commit 6d511020e13d ("lib/test_bits.c: add tests of GENMASK")
Link: https://git.kernel.org/torvalds/c/6d511020e13d
---
include/linux/bits.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/include/linux/bits.h b/include/linux/bits.h
index 60044b608817..01713e1eda56 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -20,9 +20,8 @@
*/
#if !defined(__ASSEMBLY__)
#include <linux/build_bug.h>
-#define GENMASK_INPUT_CHECK(h, l) \
- (BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
- __is_constexpr((l) > (h)), (l) > (h), 0)))
+#include <linux/compiler.h>
+#define GENMASK_INPUT_CHECK(h, l) BUILD_BUG_ON_ZERO(_statically_true((l) > (h)))
#else
/*
* BUILD_BUG_ON_ZERO is not available in h files included from asm files,
--
2.45.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 0/2] add _statically_true() to simplify GENMASK_INPUT_CHECK()
2024-11-12 13:59 [PATCH v3 0/2] add _statically_true() to simplify GENMASK_INPUT_CHECK() Vincent Mailhol
2024-11-12 13:59 ` [PATCH v3 1/2] compiler.h: add _static_assert() Vincent Mailhol
2024-11-12 13:59 ` [PATCH v3 2/2] linux/bits.h: simplify GENMASK_INPUT_CHECK() Vincent Mailhol
@ 2024-11-12 17:52 ` Linus Torvalds
2024-11-12 19:14 ` Vincent Mailhol
2 siblings, 1 reply; 5+ messages in thread
From: Linus Torvalds @ 2024-11-12 17:52 UTC (permalink / raw)
To: Vincent Mailhol
Cc: Yury Norov, Rasmus Villemoes, Luc Van Oostenryck, linux-kernel,
linux-sparse, Rikard Falkeborn
On Tue, Nov 12, 2024 at 6:05 AM Vincent Mailhol
<mailhol.vincent@wanadoo.fr> wrote:
>
> v2 -> v3:
>
> - split the single patch into a series of two patches.
I haven't actually gotten to the patches yet, because all your emails
end up in my spam box.
The reason is because your git-send-email setup is broken, resulting in:
dmarc=fail (p=QUARANTINE sp=NONE dis=QUARANTINE) header.from=wanadoo.fr
because you claim to use a wanadoo.fr address in your "From:" line:
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
but you actually used gmail to send it, and the DKIM hash was
generated by gmail:
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net;
And then DMARC complains because the From: and the DKIM doesn't match.
So to actually get the right DKIM hashes, you need to either
(a) send email using the wanadoo.fr smtp gateway
or
(b) make the sender be that gmail address that you actually use for sending.
Pls fix.
Linus
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 0/2] add _statically_true() to simplify GENMASK_INPUT_CHECK()
2024-11-12 17:52 ` [PATCH v3 0/2] add _statically_true() to " Linus Torvalds
@ 2024-11-12 19:14 ` Vincent Mailhol
0 siblings, 0 replies; 5+ messages in thread
From: Vincent Mailhol @ 2024-11-12 19:14 UTC (permalink / raw)
To: Linus Torvalds
Cc: Yury Norov, Rasmus Villemoes, Luc Van Oostenryck, linux-kernel,
linux-sparse, Rikard Falkeborn
On 13/11/2024 at 02:52, Linus Torvalds wrote:
> On Tue, Nov 12, 2024 at 6:05 AM Vincent Mailhol
> <mailhol.vincent@wanadoo.fr> wrote:
>> v2 -> v3:
>>
>> - split the single patch into a series of two patches.
> I haven't actually gotten to the patches yet, because all your emails
> end up in my spam box.
>
> The reason is because your git-send-email setup is broken, resulting in:
>
> dmarc=fail (p=QUARANTINE sp=NONE dis=QUARANTINE) header.from=wanadoo.fr
>
> because you claim to use a wanadoo.fr address in your "From:" line:
>
> From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>
> but you actually used gmail to send it, and the DKIM hash was
> generated by gmail:
>
> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
> d=1e100.net;
>
> And then DMARC complains because the From: and the DKIM doesn't match.
>
> So to actually get the right DKIM hashes, you need to either
>
> (a) send email using the wanadoo.fr smtp gateway
>
> or
>
> (b) make the sender be that gmail address that you actually use for sending.
>
> Pls fix.
D'oh. Sorry for that and thanks for the notice. I just resent the full
v3 with the correct smtp gateway (note that you may see it coming from
smtp.orange.fr, this is the same as smtp.wanadoo.fr).
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-11-12 19:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-12 13:59 [PATCH v3 0/2] add _statically_true() to simplify GENMASK_INPUT_CHECK() Vincent Mailhol
2024-11-12 13:59 ` [PATCH v3 1/2] compiler.h: add _static_assert() Vincent Mailhol
2024-11-12 13:59 ` [PATCH v3 2/2] linux/bits.h: simplify GENMASK_INPUT_CHECK() Vincent Mailhol
2024-11-12 17:52 ` [PATCH v3 0/2] add _statically_true() to " Linus Torvalds
2024-11-12 19:14 ` Vincent Mailhol
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).