Re: [PATCH v2 2/6] powerpc64/bpf: Support tailcalls with subprogs

[Hari Bathini <hbathini@linux.ibm.com>]

On 16/01/26 1:19 pm, Christophe Leroy (CS GROUP) wrote:
> 
> 
> Le 16/01/2026 à 05:50, Hari Bathini a écrit :
> 
> Not received this mail that Hari is reponding to.

That is weird.

>>
>>
>> On 14/01/26 6:33 pm, adubey wrote:
>>> On 2026-01-14 17:57, Christophe Leroy (CS GROUP) wrote:
>>>> Le 14/01/2026 à 12:44, adubey@linux.ibm.com a écrit :
>>>>> From: Abhishek Dubey <adubey@linux.ibm.com>
>>>>>
>>>>> Enabling tailcalls with subprog combinations by referencing
>>>>> method. The actual tailcall count is always maintained in the
>>>>> tail_call_info variable present in the frame of main function
>>>>> (also called entry function). The tail_call_info variables in
>>>>> the frames of all other subprog contains reference to the
>>>>> tail_call_info present in frame of main function.
>>>>>
>>>>> Dynamic resolution interprets the tail_call_info either as
>>>>> value or reference depending on the context of active frame
>>>>> while tailcall is invoked.
>>>>>
>>>>> Signed-off-by: Abhishek Dubey <adubey@linux.ibm.com>
>>>>> ---
>>>>>   arch/powerpc/net/bpf_jit.h        | 12 +++++-
>>>>>   arch/powerpc/net/bpf_jit_comp.c   | 10 ++++-
>>>>>   arch/powerpc/net/bpf_jit_comp64.c | 68 ++++++++++++++++++++++ 
>>>>> +--------
>>>>>   3 files changed, 70 insertions(+), 20 deletions(-)
>>>>>
>>>>> diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
>>>>> index 45d419c0ee73..5d735bc5e6bd 100644
>>>>> --- a/arch/powerpc/net/bpf_jit.h
>>>>> +++ b/arch/powerpc/net/bpf_jit.h
>>>>> @@ -51,6 +51,12 @@
>>>>>           EMIT(PPC_INST_BRANCH_COND | (((cond) & 0x3ff) << 16) | 
>>>>> (offset & 0xfffc));                    \
>>>>>       } while (0)
>>>>>   +/* Same as PPC_BCC_SHORT, except valid dest is known prior to 
>>>>> call. */
>>>>> +#define PPC_COND_BRANCH(cond, dest)         \
>>>>> +    do {                                      \
>>>>> +        long offset = (long)(dest) - CTX_NIA(ctx);              \
>>>>> +        EMIT(PPC_INST_BRANCH_COND | (((cond) & 0x3ff) << 16) | 
>>>>> (offset & 0xfffc));    \
>>>>> +    } while (0)
>>>>
>>>> I don't like the idea of duplicating PPC_BCC_SHORT() to just kick the
>>>> verification out. Now we will have two macros doing the exact same
>>>> thing with one handling failure case and one ignoring failure case.
>>>> There is a big risk that one day or another someone will use the wrong
>>>> macro.
>>>>
>>>> Could you change bpf_jit_build_prologue() to return an int add use
>>>> PPC_BCC_SHORT() instead of that new PPC_COND_BRANCH() ?
>>> I implemented exactly same change in bpf_jit_build_prologue(). But, 
>>> during internal review, @HariBathini suggested
>>> to have separate macro with a caution note.
>>>
>>> @Hari please suggest here!
>>
>> Not just about the change of return type but the check seems like an
>> overkill for cases where the offset is known and within branch range.
>> How about using BUILD_BUG_ON() to avoid unecessary checks and
>> return type change for places where the branch offset is known
>> and is a constant?
> 
> When offset is a constant known at build time, checks are eliminated by 
> gcc at build, see exemple below from disasembly of bpf_jit_comp32.o, 
> there are no checks.
> 
>                                  PPC_BCC_SHORT(COND_GT, (ctx->idx + 4) * 
> 4);
>      36d8:       3c 80 41 81     lis     r4,16769
>                                  EMIT(PPC_RAW_CMPLW(src_reg, _R0));
>      36dc:       81 3f 00 04     lwz     r9,4(r31)
>                                  PPC_BCC_SHORT(COND_GT, (ctx->idx + 4) * 
> 4);
>      36e0:       60 84 00 10     ori     r4,r4,16
>                                  EMIT(PPC_RAW_CMPLW(src_reg, _R0));
>      36e4:       39 29 00 01     addi    r9,r9,1
>                                  PPC_BCC_SHORT(COND_GT, (ctx->idx + 4) * 
> 4);
>      36e8:       55 23 10 3a     slwi    r3,r9,2
>                                  EMIT(PPC_RAW_CMPLW(src_reg, _R0));
>      36ec:       91 3f 00 04     stw     r9,4(r31)
>                                  PPC_BCC_SHORT(COND_GT, (ctx->idx + 4) * 
> 4);
>      36f0:       7c 97 19 2e     stwx    r4,r23,r3
>                                  EMIT(PPC_RAW_LI(dst_reg, 0));
>      36f4:       55 49 a9 94     rlwinm  r9,r10,21,6,10
>                                  PPC_BCC_SHORT(COND_GT, (ctx->idx + 4) * 
> 4);
>      36f8:       80 9f 00 04     lwz     r4,4(r31)
>                                  EMIT(PPC_RAW_LI(dst_reg, 0));
>      36fc:       65 29 38 00     oris    r9,r9,14336
>                                  PPC_BCC_SHORT(COND_GT, (ctx->idx + 4) * 
> 4);
>      3700:       38 84 00 01     addi    r4,r4,1
>                                  EMIT(PPC_RAW_LI(dst_reg, 0));
>      3704:       54 83 10 3a     slwi    r3,r4,2
>                                  PPC_BCC_SHORT(COND_GT, (ctx->idx + 4) * 
> 4);
>      3708:       90 9f 00 04     stw     r4,4(r31)
>                                  EMIT(PPC_RAW_LI(dst_reg, 0));
>      370c:       7d 37 19 2e     stwx    r9,r23,r3
> 

Interesting. I do see is_offset_in_cond_branch_range() in action with
constant offsets too, on ppc64 compile at least. fwiw, I had this
optimized version in mind for constant offset:

   #define PPC_BCC_CONST_SHORT(cond, offset) 
                     \
         do { 
                     \
                 BUILD_BUG_ON(offset < -0x8000 || offset > 0x7fff || 
(offset & 0x3));        \
                 EMIT(PPC_INST_BRANCH_COND | (((cond) & 0x3ff) << 16) | 
(offset & 0xfffc));  \
         } while (0)

With that, something like:

     PPC_BCC_SHORT(COND_NE, (ctx->idx + 3) * 4);

becomes

     PPC_BCC_CONST_SHORT(COND_NE, 12);

- Hari