Re: [PATCH v1 2/2] LoongArch: BPF: Eliminate zero-extension for memory load operations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch introduces redundant double-zero-extension instructions for all 32-bit ALU and atomic RMW operations, significantly bloating the JIT image size rather than reducing it.
--

--- Patch [2]: [PATCH v1 2/2] LoongArch: BPF: Eliminate zero-extension for memory load operations ---
commit 0efa0ea6d44efe5e18fb08bd07d86b901c2332ec
Author: Tiezhu Yang <yangtiezhu@loongson.cn>

LoongArch: BPF: Eliminate zero-extension for memory load operations

This commit overwrites bpf_jit_needs_zext() to return true, causing the
verifier to explicitly insert zero-extension instructions. It then adds
look-ahead logic to skip these verifier-inserted instructions for memory
loads that naturally zero-extend, aiming to reduce the JITted image size.

> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index 4c54e57455ae3..417b69b7ec4a0 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c

[ ... ]

> @@ -2383,6 +2399,11 @@ bool bpf_jit_supports_subprog_tailcalls(void)
>  	return true;
>  }
>  
> +bool bpf_jit_needs_zext(void)
> +{
> +	return true;
> +}
> +

[Severity: High]
Does returning true here cause the verifier to insert zext instructions for
all 32-bit operations globally, rather than just memory loads?

If the verifier now inserts a zext instruction after every ALU32 operation,
and the JIT still unconditionally calls emit_zext_32() for ALU32 ops:

arch/loongarch/net/bpf_jit.c:build_insn() {
	case BPF_ALU | BPF_ADD | BPF_X:
	case BPF_ALU64 | BPF_ADD | BPF_X:
		emit_insn(ctx, addd, dst, dst, src);
		emit_zext_32(ctx, dst, is32);
		break;
}

Will this cause the JIT to emit double zero-extensions (one internally, and
one when processing the verifier's injected instruction) for all 32-bit ALU
and atomic RMW operations? This seems like it could bloat the JIT image size
rather than reducing it, as the look-ahead skip logic was only added for
memory loads.

>  bool bpf_jit_inlines_helper_call(s32 imm)
>  {
>  	switch (imm) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728031615.9389-1-yangtiezhu@loongson.cn?part=2