| Message ID | 1633777076-17256-3-git-send-email-yangtiezhu@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf, mips: Do some small changes | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next | success | VM_Test |
| bpf/vmtest-bpf-next-PR | success | PR summary |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 14 of 14 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 14 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | No static functions without inline keyword in header files |
On Sat, Oct 9, 2021 at 12:58 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > > In emit_tail_call() of bpf_jit_comp32.c, "blez t2" (t2 <= 0) is not > consistent with the comment "t2 < 0", modify the check condition to > keep consistency. > > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > --- > arch/mips/net/bpf_jit_comp32.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c > index 9d7041a..b887c01 100644 > --- a/arch/mips/net/bpf_jit_comp32.c > +++ b/arch/mips/net/bpf_jit_comp32.c > @@ -1312,12 +1312,12 @@ static int emit_tail_call(struct jit_context *ctx) > emit(ctx, sltu, t1, ind, t1); /* t1 = ind < t1 */ > emit(ctx, beqz, t1, get_offset(ctx, 1)); /* PC += off(1) if t1 == 0 */ > /* (next insn delay slot) */ > - /* if (TCC-- <= 0) goto out */ > + /* if (--TCC < 0) goto out */ > emit(ctx, lw, t2, ctx->stack_size, MIPS_R_SP); /* t2 = *(SP + size) */ > emit_load_delay(ctx); /* Load delay slot */ > - emit(ctx, blez, t2, get_offset(ctx, 1)); /* PC += off(1) if t2 < 0 */ > emit(ctx, addiu, t2, t2, -1); /* t2-- (delay slot) */ > emit(ctx, sw, t2, ctx->stack_size, MIPS_R_SP); /* *(SP + size) = t2 */ > + emit(ctx, bltz, t2, get_offset(ctx, 1)); /* PC += off(1) if t2 < 0 */ If the comment is not consistent with the code, and the code is correct, why did you change the code? Have you seen the JIT fail on any of the tail call test cases? The current code works as intended. The t2 register is decremented in the branch delay slot of the blez. After your change, the the comment still says "delay slot", but it is no longer in the delay slot of a branch. Instead the next instruction emitted, not visible in the patch context, fills the delay slot of the bltz. In this case it probably is ok, but if that instruction is also a branch, the result would be unpredictable. I prefer to emit the delay slot instruction immediately after the branch is emitted when possible. If a branch and its delay slot is separated in the JIT logic, it makes the JIT more brittle IMO. Please keep the original logic, but update the blez comment so it is consistent with the code. > > /* prog = ary->ptrs[ind] */ > off = offsetof(struct bpf_array, ptrs); > -- > 2.1.0 >
diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c index 9d7041a..b887c01 100644 --- a/arch/mips/net/bpf_jit_comp32.c +++ b/arch/mips/net/bpf_jit_comp32.c @@ -1312,12 +1312,12 @@ static int emit_tail_call(struct jit_context *ctx) emit(ctx, sltu, t1, ind, t1); /* t1 = ind < t1 */ emit(ctx, beqz, t1, get_offset(ctx, 1)); /* PC += off(1) if t1 == 0 */ /* (next insn delay slot) */ - /* if (TCC-- <= 0) goto out */ + /* if (--TCC < 0) goto out */ emit(ctx, lw, t2, ctx->stack_size, MIPS_R_SP); /* t2 = *(SP + size) */ emit_load_delay(ctx); /* Load delay slot */ - emit(ctx, blez, t2, get_offset(ctx, 1)); /* PC += off(1) if t2 < 0 */ emit(ctx, addiu, t2, t2, -1); /* t2-- (delay slot) */ emit(ctx, sw, t2, ctx->stack_size, MIPS_R_SP); /* *(SP + size) = t2 */ + emit(ctx, bltz, t2, get_offset(ctx, 1)); /* PC += off(1) if t2 < 0 */ /* prog = ary->ptrs[ind] */ off = offsetof(struct bpf_array, ptrs);
In emit_tail_call() of bpf_jit_comp32.c, "blez t2" (t2 <= 0) is not consistent with the comment "t2 < 0", modify the check condition to keep consistency. Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> --- arch/mips/net/bpf_jit_comp32.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)