Re: [PATCH bpf-next 2/4] selftests/bpf: Update reg_bound range refinement logic

[Paul Chaignon <paul.chaignon@gmail.com>]

On Mon, Jul 21, 2025 at 02:29:47PM -0700, Eduard Zingerman wrote:
> On Sat, 2025-07-19 at 16:22 +0200, Paul Chaignon wrote:
> > This patch updates the range refinement logic in the reg_bound test to
> > match the new logic from the previous commit. Without this change, tests
> > would fail because we end with more precise ranges than the tests
> > expect.
> > 
> > Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> > ---
> 
> Acked-by: Eduard Zingerman <eddyz87@gmail.com>

Thanks for the review!

> 
> >  .../testing/selftests/bpf/prog_tests/reg_bounds.c  | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/reg_bounds.c b/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
> > index 39d42271cc46..e261b0e872db 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
> > @@ -465,6 +465,20 @@ static struct range range_refine(enum num_t x_t, struct range x, enum num_t y_t,
> >  		return range_improve(x_t, x, x_swap);
> >  	}
> >  
> > +	if (!t_is_32(x_t) && !t_is_32(y_t) && x_t != y_t) {
> 
> Nit: I'd swap x and y if necessary, to avoid a second branch.

That works, but we'd have to swap them back before we hit range_improve
below. Something like:

    if (x_t != S64)
        swap(x, y);
    if (x.a > x.b) {
        if (x.b < y.a && x.a <= y.b)
            return range(x_t, x.a, y.b);
        if (x.a > y.b && x.b >= y.a)
            return range(x_t, y.a, x.b);
    }
    if (x_t != S64)
        swap(x, y);

I'm not sure it's better.

> 
> > +		if (x_t == S64 && x.a > x.b) {
> > +			if (x.b < y.a && x.a <= y.b)
> > +				return range(x_t, x.a, y.b);
> > +			if (x.a > y.b && x.b >= y.a)
> > +				return range(x_t, y.a, x.b);
> > +		} else if (x_t == U64 && y.a > y.b) {
> > +			if (y.b < x.a && y.a <= x.b)
> > +				return range(x_t, y.a, x.b);
> > +			if (y.a > x.b && y.b >= x.a)
> > +				return range(x_t, x.a, y.b);
> 
> Nit: here returned type us U64, while above it is S64, I don't think
>      it matters but having same type in both branches would be less
>      confusing.

What do you mean? We have to return x's original type as we're refining
the x range by using the y range.

> 
> > +		}
> > +	}
> > +
> >  	/* otherwise, plain range cast and intersection works */
> >  	return range_improve(x_t, x, y_cast);
> >  }
>