| Message ID | 20241111142028.67708-1-hardevsinh.palaniya@siliconsignals.io |
|---|---|
| State | New |
| Headers | show |
| Series | ARC: bpf_jit_arcv2: Remove redundant condition check | expand |
On 11/11/2024 14:19, Hardevsinh Palaniya wrote: > The condition 'if (ARC_CC_AL)' is always true, as ARC_CC_AL is a constant > integer. This makes the check redundant, so it is safe to remove. > > Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io> > --- > arch/arc/net/bpf_jit_arcv2.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/arch/arc/net/bpf_jit_arcv2.c b/arch/arc/net/bpf_jit_arcv2.c > index 4458e409ca0a..19792ce952be 100644 > --- a/arch/arc/net/bpf_jit_arcv2.c > +++ b/arch/arc/net/bpf_jit_arcv2.c > @@ -2916,10 +2916,7 @@ bool check_jmp_32(u32 curr_off, u32 targ_off, u8 cond) > addendum = (cond == ARC_CC_AL) ? 0 : INSN_len_normal; > disp = get_displacement(curr_off + addendum, targ_off); > > - if (ARC_CC_AL) > - return is_valid_far_disp(disp); > - else > - return is_valid_near_disp(disp); > + return is_valid_far_disp(disp); > } > > /* The original code is obviously optimized out, but the intention, I believe, was to check if the jump is conditional or not. So the proper fix should change the code to check cond: - if (ARC_CC_AL) + if (cond == ARC_CC_AL)
Vadim Fedorenko wrote: > The original code is obviously optimized out, but the intention, I > believe, was to check if the jump is conditional or not. > So the proper fix should change the code to check cond: > > - if (ARC_CC_AL) > + if (cond == ARC_CC_AL) That is absolutely correct. If a new patch is not submitted soon I'll try to fix it myself. Cheers, Shahab
Hi Vadim , Shahab Thanks for the feedback > > The original code is obviously optimized out, but the intention, I > > believe, was to check if the jump is conditional or not. > > So the proper fix should change the code to check cond: > > > > - if (ARC_CC_AL) > > + if (cond == ARC_CC_AL) Okay > That is absolutely correct. If a new patch is not submitted soon > I'll try to fix it myself. if you are okay with that then I can proceed by submitting version 2 of the patch with the proposed changes included Best Regards, Hardev
Hardev wrote: > Shahab wrote: > > > > Vadim wrote: > > > > > > > > > The original code is obviously optimized out, but the intention, I > > > believe, was to check if the jump is conditional or not. > > > So the proper fix should change the code to check cond: > > > > > > - if (ARC_CC_AL) > > > + if (cond == ARC_CC_AL) > > > > > > That is absolutely correct. If a new patch is not submitted soon > > I'll try to fix it myself. > > if you are okay with that then I can proceed by submitting version 2 > of the patch with the proposed changes included Of course. Please go ahead. To be clear, What I meant by "soon" was something around a week time. Cheers, Shahab
diff --git a/arch/arc/net/bpf_jit_arcv2.c b/arch/arc/net/bpf_jit_arcv2.c index 4458e409ca0a..19792ce952be 100644 --- a/arch/arc/net/bpf_jit_arcv2.c +++ b/arch/arc/net/bpf_jit_arcv2.c @@ -2916,10 +2916,7 @@ bool check_jmp_32(u32 curr_off, u32 targ_off, u8 cond) addendum = (cond == ARC_CC_AL) ? 0 : INSN_len_normal; disp = get_displacement(curr_off + addendum, targ_off); - if (ARC_CC_AL) - return is_valid_far_disp(disp); - else - return is_valid_near_disp(disp); + return is_valid_far_disp(disp); } /*
The condition 'if (ARC_CC_AL)' is always true, as ARC_CC_AL is a constant integer. This makes the check redundant, so it is safe to remove. Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io> --- arch/arc/net/bpf_jit_arcv2.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)