| Message ID | 1661479927-6953-1-git-send-email-yangtiezhu@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] bpf, mips: No need to use min() to get MAX_TAIL_CALL_CNT | expand |
On 8/26/22 4:12 AM, Tiezhu Yang wrote: > MAX_TAIL_CALL_CNT is 33, so min(MAX_TAIL_CALL_CNT, 0xffff) is always > MAX_TAIL_CALL_CNT, it is better to use MAX_TAIL_CALL_CNT directly. > > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > --- > arch/mips/net/bpf_jit_comp32.c | 2 +- > arch/mips/net/bpf_jit_comp64.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c > index 83c975d..8fee671 100644 > --- a/arch/mips/net/bpf_jit_comp32.c > +++ b/arch/mips/net/bpf_jit_comp32.c > @@ -1381,7 +1381,7 @@ void build_prologue(struct jit_context *ctx) > * 16-byte area in the parent's stack frame. On a tail call, the > * calling function jumps into the prologue after these instructions. > */ > - emit(ctx, ori, MIPS_R_T9, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff)); I presume this is the max that can be encoded, right? Maybe just convert this to a BUILD_BUG_ON(MAX_TAIL_CALL_CNT > 0xffff) with a comment on why the assertion is there? > + emit(ctx, ori, MIPS_R_T9, MIPS_R_ZERO, MAX_TAIL_CALL_CNT); > emit(ctx, sw, MIPS_R_T9, 0, MIPS_R_SP); > > /* > diff --git a/arch/mips/net/bpf_jit_comp64.c b/arch/mips/net/bpf_jit_comp64.c > index 6475828..ac175af 100644 > --- a/arch/mips/net/bpf_jit_comp64.c > +++ b/arch/mips/net/bpf_jit_comp64.c > @@ -552,7 +552,7 @@ void build_prologue(struct jit_context *ctx) > * On a tail call, the calling function jumps into the prologue > * after this instruction. > */ > - emit(ctx, ori, tc, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff)); > + emit(ctx, ori, tc, MIPS_R_ZERO, MAX_TAIL_CALL_CNT); > > /* === Entry-point for tail calls === */ > >
On Fri, Aug 26, 2022 at 6:18 PM Daniel Borkmann <daniel@iogearbox.net> wrote: > > On 8/26/22 4:12 AM, Tiezhu Yang wrote: > > MAX_TAIL_CALL_CNT is 33, so min(MAX_TAIL_CALL_CNT, 0xffff) is always > > MAX_TAIL_CALL_CNT, it is better to use MAX_TAIL_CALL_CNT directly. > > > > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > > --- > > arch/mips/net/bpf_jit_comp32.c | 2 +- > > arch/mips/net/bpf_jit_comp64.c | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c > > index 83c975d..8fee671 100644 > > --- a/arch/mips/net/bpf_jit_comp32.c > > +++ b/arch/mips/net/bpf_jit_comp32.c > > @@ -1381,7 +1381,7 @@ void build_prologue(struct jit_context *ctx) > > * 16-byte area in the parent's stack frame. On a tail call, the > > * calling function jumps into the prologue after these instructions. > > */ > > - emit(ctx, ori, MIPS_R_T9, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff)); > > I presume this is the max that can be encoded, right? Maybe just convert this > to a BUILD_BUG_ON(MAX_TAIL_CALL_CNT > 0xffff) with a comment on why the assertion > is there? Correct. The min() is there for a reason. In the unlikely event that the TCC limit is raised to more than 16 bits, it is clamped to the maximum value allowed for the generated code (0xffff). One can argue that it is better fail to compile instead of degrading gracefully, but some kind check should be there IMO. > > > + emit(ctx, ori, MIPS_R_T9, MIPS_R_ZERO, MAX_TAIL_CALL_CNT); > > emit(ctx, sw, MIPS_R_T9, 0, MIPS_R_SP); > > > > /* > > diff --git a/arch/mips/net/bpf_jit_comp64.c b/arch/mips/net/bpf_jit_comp64.c > > index 6475828..ac175af 100644 > > --- a/arch/mips/net/bpf_jit_comp64.c > > +++ b/arch/mips/net/bpf_jit_comp64.c > > @@ -552,7 +552,7 @@ void build_prologue(struct jit_context *ctx) > > * On a tail call, the calling function jumps into the prologue > > * after this instruction. > > */ > > - emit(ctx, ori, tc, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff)); > > + emit(ctx, ori, tc, MIPS_R_ZERO, MAX_TAIL_CALL_CNT); > > > > /* === Entry-point for tail calls === */ > > > > >
diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c index 83c975d..8fee671 100644 --- a/arch/mips/net/bpf_jit_comp32.c +++ b/arch/mips/net/bpf_jit_comp32.c @@ -1381,7 +1381,7 @@ void build_prologue(struct jit_context *ctx) * 16-byte area in the parent's stack frame. On a tail call, the * calling function jumps into the prologue after these instructions. */ - emit(ctx, ori, MIPS_R_T9, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff)); + emit(ctx, ori, MIPS_R_T9, MIPS_R_ZERO, MAX_TAIL_CALL_CNT); emit(ctx, sw, MIPS_R_T9, 0, MIPS_R_SP); /* diff --git a/arch/mips/net/bpf_jit_comp64.c b/arch/mips/net/bpf_jit_comp64.c index 6475828..ac175af 100644 --- a/arch/mips/net/bpf_jit_comp64.c +++ b/arch/mips/net/bpf_jit_comp64.c @@ -552,7 +552,7 @@ void build_prologue(struct jit_context *ctx) * On a tail call, the calling function jumps into the prologue * after this instruction. */ - emit(ctx, ori, tc, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff)); + emit(ctx, ori, tc, MIPS_R_ZERO, MAX_TAIL_CALL_CNT); /* === Entry-point for tail calls === */
MAX_TAIL_CALL_CNT is 33, so min(MAX_TAIL_CALL_CNT, 0xffff) is always MAX_TAIL_CALL_CNT, it is better to use MAX_TAIL_CALL_CNT directly. Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> --- arch/mips/net/bpf_jit_comp32.c | 2 +- arch/mips/net/bpf_jit_comp64.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)