Re: [PATCH bpf-next v2 01/13] bpf: move linked-scalar flags out of bpf_reg_state->id [NFC]

[Vineet Gupta <vineet.gupta@linux.dev>]

On 9/16/26 5:36 PM, Alexei Starovoitov wrote:
> On Wed, Sep 16, 2026 at 2:24 PM Vineet Gupta<vineet.gupta@linux.dev> wrote:
>> On 9/16/26 2:02 PM, Alexei Starovoitov wrote:
>>> On Tue Sep 15, 2026 at 1:17 AM UTC, Vineet Gupta wrote:
>>>> Back when I started, keeping it NFC made more sense. But with all the
>>>> nuances discovered in the process, I'm inclined to drop the NFC stance
>>> What is NFC ?
>> non functional change aka refactoring.
>> Like I said when I started I wanted to keep the id / flag breakout
>> purely non functional. But given that splitting it does change one thing
>> (mentioned below) - I'm dropping NFC attribution.
>>
>>>> and make functional changes - there's at least one which is the
>>>> following accepted pre-series and now rejected.
>>>>
>>>>          old {r2.id=A+delta32}         vs cur {r2.id=B+delta64}
>>> that's a red flag, no? The refactoring patch shouldn't have
>>> any changes to selftest and veristat numbers?
>>> Unless I'm missing it again.
>> Yes understood except that it is no longer refactoring.
> NFC is gcc probably lingvo? In kernel people just put in the commit
> log "No functional changes", so it's easier for humans and for AI
> to review patches.
> So pls skip such unusual tags in the future,

Yes keeping kernel's existing conventions makes sense, although LLMs 
understand what NFC is.

> but keep the first patch as "No functional change".

Just to rehash since the meaning of first has moved significantly.

1. Now the first change (patch 1/x and its test 2/x )  is no longer the 
compound id conversion, but coerce_reg_to_size_sx / 
coerce_subreg_to_size_sx change to fix the errno range case, as 
suggested by you. It is a functional change, with following improvement:

    === VERDICT CHANGES: 3 ===
        mov32sx_s8_negative_range     failure -> success
        mov64sx_s32_negative_range    failure -> success
        mov64sx_s8_negative_range     failure -> success

    === total_insns:  0 changed among 1242 both-passing, 0 worse ===
    === total_states: 0 changed among 1242 both-passing, 0 worse ===

2. The second change (3/x and its test 4/x) is accumulating add_const 
deltas (w3 = w0; w3 -=1). This makes the 32-bit link with delta special 
handling of zext unnecessary so its a prereq for the series.

3. I guess you are referring to the patch to convert compound id to 
broken out id + flags (5/x ) is also technically a functional change - 
due to the state comparison behavior improvement that fell out of the 
seemingly mechanical conversion and is clearly called out now. However I 
verified it in isolation (on top of 4 prior of course) and there are no 
veristat or selftest changes with it (just a cosmetic name change due to 
base_id -> id)

    === VERDICT CHANGES: 0 ===
    === total_insns:  0 changed among 1247 both-passing, 0 worse ===
    === total_states: 0 changed among 1247 both-passing, 0 worse ===


I presume that is what you were asking for ?

Thx,
-Vineet