Re: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension

[Conor Dooley]

On Wed, May 15, 2024 at 11:31:43AM +0000, Wang, Xiao W wrote:
> > From: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > > > My preferences is to remove as much of the TOOLCHAIN_HAS_ stuff as
> > > > possible. We should audit the extensions which have them to see if
> > > > they're really necessary.
> > >
> > > While I think it is reasonable to allow the "RISCV_ISA_ZBB" option to
> > > control whether or not bpf is allowed to use it for optimisations, only
> > > allowing bpf to do that if there's toolchain support feels odd to me..
> > > Maybe we need to sorta steal from Charlie's patchset and introduce
> > > some hidden options that have the toolchain dep that are used by the
> > > alternative macros etc?
> > >
> > > I'll have a poke at how bad that looks I think.
> > 
> > I don't love this, in particular my option naming, but it would allow
> > the Zbb optimisations in the kernel to not depend on toolchain support
> > while not muddying the Kconfig waters for users:
> > https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=ri
> > scv-zbb_split
> 
> In that patch, I think the bpt jit part should check IS_ENABLED(CONFIG_RISCV_ISA_ZBB)
> rather than IS_ENABLED(CONFIG_RISCV_ISA_ZBB_ALT).

D'oh, you're right. The bpf code being different was meant to be the whole
point of the change...

> > A similar model could be followed if there were to be some
> > optimisations for Zba in the future that do require toolchain support:
> 
> Though this model introduces extra hidden Kconfig option, it does provide finer 
> config granularity. This should be a separate patch in the future, we can discuss about
> the option naming there.

Yeah, not expecting you to do this as part of this patch.

Thanks,
Conor.

Attachment: signature.asc
Description: PGP signature