RE: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

From: dthaler1968@googlemail.com
To: "'Yonghong Song'" <yonghong.song@linux.dev>,
	"'Jose E. Marchesi'" <jose.marchesi@oracle.com>
Cc: <bpf@vger.kernel.org>, <bpf@ietf.org>
Subject: RE: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group
Date: Mon, 12 Feb 2024 17:18:27 -0800	[thread overview]
Message-ID: <03a801da5e1a$8d0274c0$a7075e40$@gmail.com> (raw)
In-Reply-To: <a81da29b-b671-484a-8f3d-743f1dac44f1@linux.dev>

> -----Original Message-----
> From: Yonghong Song <yonghong.song@linux.dev>
> Sent: Monday, February 12, 2024 2:49 PM
> To: dthaler1968@googlemail.com; 'Jose E. Marchesi'
> <jose.marchesi@oracle.com>; 'Dave Thaler'
> <dthaler1968=40googlemail.com@dmarc.ietf.org>
> Cc: bpf@vger.kernel.org; bpf@ietf.org
> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new
> conformance group
> 
> 
> On 2/12/24 1:52 PM, dthaler1968@googlemail.com wrote:
> >> -----Original Message-----
> >> From: Yonghong Song <yonghong.song@linux.dev>
> >> Sent: Monday, February 12, 2024 1:49 PM
> >> To: Jose E. Marchesi <jose.marchesi@oracle.com>; Dave Thaler
> >> <dthaler1968=40googlemail.com@dmarc.ietf.org>
> >> Cc: bpf@vger.kernel.org; bpf@ietf.org; Dave Thaler
> >> <dthaler1968@gmail.com>
> >> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx
> >> instructions in new conformance group
> >>
> >>
> >> On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
> >>>> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
> >> only, see `Program-local functions`_
> >>> If the instruction requires a register operand, why not using one of
> >>> the register fields?  Is there any reason for not doing that?
> >> Talked to Alexei and we think using dst_reg for the register for
> >> callx insn is better. I will craft a llvm patch for this today. Thanks!
> > Why dst_reg instead of src_reg?
> > BPF_X is supposed to mean use src_reg.
> 
> Let us use dst_reg. Currently, for BPF_K, we have src_reg for a bunch of flags
> (pseudo call, kfunc call, etc.). So for BPF_X, let us preserve this property as
> well in case in the future we will introduce variants for callx.

Ah yes, that makes sense.

> The following is the llvm diff:
> 
> https://github.com/llvm/llvm-project/pull/81546

Which llvm release is it targeted for?
18.1.0-rc3? 18.1.1?  later?

> > But this thread is about reserving/documenting the existing practice,
> > since anyone trying to use it would run into interop issues because
> > of existing clang.   Should we document both and list one as deprecated?
> 
> I think just documenting the new encoding is good enough. But other
> people can chime in just in case that I missed something.

Ok.

Dave