Re: [PATCH] bpf: Simplify tnum_step()

[Shung-Hsi Yu]

On Thu, Mar 19, 2026 at 10:35:06AM +0100, Paul Chaignon wrote:
> On Thu, Mar 19, 2026 at 10:01:31AM +0100, Hao Sun wrote:
...
> > > > +     carry_mask = (1ULL << fls64(d & ~t.mask)) - 1;
> > > > +     inc = ((d | carry_mask | ~t.mask) + 1) & t.mask;
> > >
> > > Maybe break out the calculation of inc, give it a name (next_submask?),
> > 
> > Maybe we can break this down as:
> > 
> > d = z - t.value;
> > carry_mask = (1ULL << fls64(d & ~t.mask)) - 1;
> > filled = d | carry_mask | ~t.mask;
> > inc = (filled + 1) & t.mask;
> > 
> > `next_submask` is not precise, as we ored `~t.mask` into it.

I was thinking of something like this: 

	static inline u64 refactored(u64 mask, u64 d)
	{
		u64 carry_mask;
	
		carry_mask = (1ULL << fls64(d & ~mask)) - 1;
		return ((d | carry_mask | ~mask) + 1) & mask;
	}

	u64 tnum_step(struct tnum t, u64 z)
	{
		...
		d = z - t.value;
		inc = refactored(t.mask, d);
		return t.value | inc;
	}

For me I find having to grapple 1) new way to calculate tnum_step and 2)
finding a submask of tmask > d, both at the same time rather difficult.
Hence the suggestion for separation.

> > It's always hard to name the intermediate result :)

Indeed, and let me not even try to name the function here.

> > > and have it as preceding patch? It seems generic enough that I thought
> > > would have been implemented already, but bitmap_scatter() was the
> > > closest I could find.
> > >
> > > Perhaps could be submitted to include/linux/bitops.h in the future.
> > 
> > The calculation above is direct, adding a bitmap helper intros an indirection.
> > For me, it means I would have to check out the meaning of the helper
> > and then get back to understanding the algorithm.
> 
> I think I agree with Hao in this case. If we can reuse existing helpers,
> great, but otherwise it's short enough that it's probably easier to keep
> inlined here.

Okay, I can now slightly see that it does introducing indirection, make
sense to keep as-is.

Acked-by: Shung-Hsi Yu <shung-hsi.yu@xxxxxxxx>