Re: [PATCH] minmax: clamp more efficiently by avoiding extra comparison
From: Andy Shevchenko
Date: Fri Sep 23 2022 - 11:11:08 EST
On Fri, Sep 23, 2022 at 12:40:47PM +0200, Jason A. Donenfeld wrote:
> On Fri, Sep 23, 2022 at 12:36 PM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> >
> > On Fri, Sep 23, 2022 at 12:06:21PM +0200, Jason A. Donenfeld wrote:
> > > Currently the clamp algorithm does:
> > >
> > > if (val > hi)
> > > val = hi;
> > > if (val < lo)
> > > val = lo;
> > >
> > > But since hi > lo by definition, this can be made more efficient with:
> >
> > It's strongly speaking, but we have to proof that, right?
> > So, while I haven't checked the code, this change should also
> > include (does it?) the corresponding compile-time checks (for
> > constant arguments) in similar way how it's done for GENMASK().
> >
> > Otherwise I have no objections.
>
> I think most cases are with compile time constants, but some cases are
> with variables. What should we do in that case? Checking variables at
> runtime incurs the same cost as the old code. I guess we could do this
> fast thing for constants and the slower old thing for non-constants?
> Or not do either, keep this commit as is, and just accept that if you
> pass bogus bounds to clamp, you're going to end up with something
> weird, which is already the case now so not a big deal?
I'm talking only for the cases where we _can_ check. For variables it's
probably tricky to do at compile time if possible at all.
--
With Best Regards,
Andy Shevchenko