[v5, 5/6] riscv: lib: vectorize copy_to_user/copy_from_user

Andrew Jones ajones at ventanamicro.com
Fri Dec 15 05:52:57 PST 2023


On Thu, Dec 14, 2023 at 10:25:49PM -0800, Charlie Jenkins wrote:
> On Thu, Dec 14, 2023 at 03:57:20PM +0000, Andy Chiu wrote:
...
> >  SYM_FUNC_START(__asm_copy_to_user)
> > +#ifdef CONFIG_RISCV_ISA_V
> > +	ALTERNATIVE("j fallback_scalar_usercopy", "nop", 0, RISCV_ISA_EXT_v, CONFIG_RISCV_ISA_V)
> 
> has_vector uses riscv_has_extension_unlikely, but this is the equivalent
> of riscv_has_extension_likely. It seems like this should be consistent
> across all call sites. Since has_vector uses the unlikely version, this
> should probably be rearranged so that the nop is in the non-vector
> version and the jump is for the vector version.

I think I prefer it the way it is, where the optimized path is fully
optimized and the fallback path also suffers the jump. (I've also
taken that approach for clear_page()). Also, as extensions are adopted
by more an more platforms, and we start to consider switching unlikelys
to likelys, then it would be easy to miss stuff like this.

> 
> A neat optimization you can do here is replace the "nop" with the
> instruction that will be executed first. With how it's written right now
> you could replace the nop with the la instruction. It's just a nop so
> the performance difference is probably not going to be noticable but
> it's theoretically better without the nop. The downside of doing this is

I think I prefer the nop, because it's easier to read and maintain the 
assembly function when the ALTERNATIVE doesn't do anything other than
choose the entry point.

> that it seems like alternatives do not work with macros so you couldn't
> replace the nop with a REG_L instruction, unless there is some trick to
> make it work.

One should be able to use REG_L in an alternative since macro expansion
will result in the string "ld" or "lw", which can then be concatenated
with its parameters, e.g.

  ALTERNATIVE(REG_L " a1, 0(a2)", "nop", 0, 0, 0)

(But note the space before the a1. Without it, we'd get "lda1,")

Thanks,
drew



More information about the linux-riscv mailing list