Re: [PATCH bpf-next v2 03/13] bpf: track low-32 scalar equality across zero-extending movs

[Alexei Starovoitov]

On Tue, Sep 15, 2026 at 1:31 PM Vineet Gupta <vineet.gupta@xxxxxxxxx> wrote:
>
> On 9/12/26 11:59 AM, Alexei Starovoitov wrote:
> > On Thu Sep 10, 2026 at 9:46 AM PDT, Vineet Gupta wrote:
> >> Linked-scalar equality is full-64-bit only. A 32-bit mov from a source
> >> with unknown high bits therefore has to drop the relationship, and a later
> >> narrowing of the source never reaches the destination:
> >>
> >>    r6 = ...              /* full 64-bit unknown */
> >>    w7 = w6               /* 32-bit zero-extending mov */
> >>    if w6 != 0 goto ...   /* not taken: r6's low 32 bits are 0 */
> >>    if w7 == 0 goto ...   /* not deduced today */
> >>
> >> Record a low-32-only link instead: dst shares src's low 32 bits and its
> >> high half is zero. On a later narrowing, sync_linked_regs() rebuilds such
> >> a register from the base rather than copying it, by re-applying the same
> >> zext_32_to_64() the mov used. The reverse direction is skipped: a ->subreg
> >> base knows nothing about a full register's high half.
> > at the first glance SUBREG_ZEXT is exactly the same as ADD_CONST32 delta == 0.
> > no?
> > Both are unidirectional:
> > w6->id == 1
> > w7->id == 1, add_const == 32, delta == 0
> >
> > will zero extend w7.
> >
> > This new SUBREG_ZEXT will do the same.
> > What am I missing?
>
> The 2 examples below both imply ADD_CONST32 + delta=0, with different
> semantics and need special casing.
>
> w3 = w2; w3 += 0
> w7 = w6; (r6 was wide)
>
> With code hacks it could in fact me made to work, but its less cleaner.
>
>
> Current approach we have something like below in sync_linked_regs()
>
>                /*
>                 * A ->subreg register shares only the base's low 32
> bits, so it
>                 * is rebuilt rather than copied. Not modelled together
> with a
>                 * delta, so skip if either side has one (sound, less
> precise).
>                 */
>                if (reg->subreg) {
>                        if (reg->add_const || known_reg->add_const)
>                                continue;
>                        if (reg->subreg == SUBREG_ZEXT)
>                                reconstruct_zext32(reg, known_reg);
>                        else
>                                reconstruct_sext32(reg, known_reg);
>                        if (e->is_reg)
>                                mark_reg_scratched(env, e->regno);
>                        else
>                                mark_stack_slot_scratched(env, e->spi);
>                        continue;
>                }
>                /*
>                 * The reverse: known_reg knows only its low 32 bits,
> which say
>                 * nothing about reg's high half.
>                 */
>                if (known_reg->subreg)
>                        continue;
>
>
> With the suggested approach it looks something like below.
> ADD_CONST_32, delta == 0 has to be disambiguated from a real += 0 by
> inspecting another register's bounds.
>
>                /*
>                 * A low-32 link shares only the base's low 32 bits, so it is
>                 * rebuilt rather than copied.
>                 *
>                 * SIGN_EXTEND_32 says so outright. ADD_CONST_32 with
> delta == 0
>                 * does not: it is also what "w3 = w2; w3 += 0" records,
> which is
>                 * a plain equality. The two are separable only by the base's
>                 * width
>                 */
>                if (reg->add_const == SIGN_EXTEND_32 &&
> !known_reg->add_const) {
>                        reconstruct_sext32(reg, known_reg);
>                        goto scratched;
>                }
>                if (reg->add_const == ADD_CONST_32 && reg->delta == 0 &&
>                    !known_reg->add_const) {
>                        reconstruct_zext32(reg, known_reg);
>                        goto ...;
>                }

I think that's a problem with reconstruct_zext32().
It shouldn't be necessary.
The existing code that adds a constant should
already handle it.
If not, we have a bug in add_const_32.
Instead of losing link at wx=wy time
we can keep it with delta == 0
and sync_linked_regs() shouldn't need any new code.