[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <051521cf-5dd2-4831-ab4c-b0db32436ba9@linux.ibm.com>
Date: Fri, 16 Jan 2026 19:29:02 +0530
From: Hari Bathini <hbathini@...ux.ibm.com>
To: "Christophe Leroy (CS GROUP)" <chleroy@...nel.org>,
adubey <adubey@...p.linux.ibm.com>
Cc: adubey@...ux.ibm.com, bpf@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
sachinpb@...ux.ibm.com, venkat88@...ux.ibm.com, andrii@...nel.org,
eddyz87@...il.com, mykolal@...com, ast@...nel.org,
daniel@...earbox.net, martin.lau@...ux.dev, song@...nel.org,
yonghong.song@...ux.dev, john.fastabend@...il.com, kpsingh@...nel.org,
sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org,
naveen@...nel.org, maddy@...ux.ibm.com, mpe@...erman.id.au,
npiggin@...il.com, memxor@...il.com, iii@...ux.ibm.com,
shuah@...nel.org
Subject: Re: [PATCH v2 2/6] powerpc64/bpf: Support tailcalls with subprogs
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@...ux.ibm.com a écrit :
>>>>> From: Abhishek Dubey <adubey@...ux.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@...ux.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
Powered by blists - more mailing lists