Re: [PATCH v2 1/8] minmax: Put all the clamp() definitions together

[Linus Torvalds <torvalds@linuxfoundation.org>]

On Tue, 30 Jul 2024 at 07:15, Arnd Bergmann <arnd@kernel.org> wrote:
>
> There is another one that I see with gcc-8 randconfigs (arm64):

So I ended up undoing that part of my patch, so it's a non-issue, but
I wanted to figure out what went wrong.

It's actually quite funny.

> net/netfilter/ipvs/ip_vs_conn.c:1498:8: note: in expansion of macro 'clamp'
>  1498 |  max = clamp(max, min, max_avail);

So it turns out that min is seen by the compiler to be constant (8).
And max_avail() uses order_base_2(), which expands to this huge
comparison table for the constant case.

Now, in the end all of those comparisons will go away, but I think
what happens is that because they exist at the source level, the
compiler ends up expanding them, and then - for exactly the same
reason as before - the compiler can find a path in that tree of
conditionals where "max_avail" does indeed end up being a constant
smaller than 8.

The fact that that path is then never taken, and pruned away later,
doesn't help us, and the compiletime_assert happens because in one
intermediate form the compiler had folded the now trivial 'clamp()' of
two constants in with the conditionals that get thrown away, and the
warning happens even when it's not reachable.

Anyway, it does mean that yeah, the much more stupid static_assert()
that will never try to follow any chain of expressions si the way to
go.

For the type checking, this isn't as much of an issue.

The whole "is that a non-negative expression" also uses the same
machinery, but if the expression could be negative in some situation
(that goes away in the end) at worst it just means that the new rules
won't be as relaxed as they could be when the expressions get
sufficiently complicated.

                Linus