Re: Buiild error in i915/xe (was: [PATCH next 4/7] minmax.h: Use BUILD_BUG_ON_MSG() for the lo < hi test in clamp())

[Pedro Falcato <pedro.falcato@gmail.com>]

On Sat, Jan 18, 2025 at 10:11 PM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Sat, 18 Jan 2025 13:21:39 -0800
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> > On Sat, 18 Jan 2025 at 09:49, Guenter Roeck <linux@roeck-us.net> wrote:
> > >
> > > No idea why the compiler would know that the values are invalid.
> >
> > It's not that the compiler knows tat they are invalid, but I bet what
> > happens is in scale() (and possibly other places that do similar
> > checks), which does this:
> >
> >         WARN_ON(source_min > source_max);
> >         ...
> >         source_val = clamp(source_val, source_min, source_max);
> >
> > and the compiler notices that the ordering comparison in the first
> > WARN_ON() is the same as the one in clamp(), so it basically converts
> > the logic to
> >
> >         if (source_min > source_max) {
> >                 WARN(..);
> >                 /* Do the clamp() knowing that source_min > source_max */
> >                 source_val = clamp(source_val, source_min, source_max);
> >         } else {
> >                 /* Do the clamp knowing that source_min <= source_max */
> >                 source_val = clamp(source_val, source_min, source_max);
> >         }
> >
> > (obviously I dropped the other WARN_ON in the conversion, it wasn't
> > relevant for this case).
> >
> > And now that first clamp() case is done with source_min > source_max,
> > and it triggers that build error because that's invalid.
> >
> > So the condition is not statically true in the *source* code, but in
> > the "I have moved code around to combine tests" case it now *is*
> > statically true as far as the compiler is concerned.
>
> Well spotted :-)
>
> One option would be to move the WARN_ON() below the clamp() and
> add an OPTIMISER_HIDE_VAR(source_max) between them.
>
> Or do something more sensible than the WARN().

Given the awful problems we've found with these macros (clamp, min,
max, etc), maybe the best option is to not play these awful games in
general?

These macros seem footgunned to hell just as a way to try to
compensate for C's lack of language features.

-- 
Pedro