diff mbox series

[08/11] tcg/optimize: Remove redundant statement in tcg_optimize()

Message ID 20200813073712.4001404-9-kuhn.chenqun@huawei.com
State New
Headers show
Series trivial patchs for static code analyzer fixes | expand

Commit Message

Chenqun (kuhn) Aug. 13, 2020, 7:37 a.m. UTC
Clang static code analyzer show warning:
tcg/optimize.c:1267:17: warning: Value stored to 'nb_iargs' is never read
                nb_iargs = 2;
                ^          ~

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
---
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Richard Henderson <rth@twiddle.net>
---
 tcg/optimize.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Richard Henderson Aug. 13, 2020, 4:22 p.m. UTC | #1
On 8/13/20 12:37 AM, Chen Qun wrote:
> Clang static code analyzer show warning:
> tcg/optimize.c:1267:17: warning: Value stored to 'nb_iargs' is never read
>                 nb_iargs = 2;
>                 ^          ~
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> ---
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/optimize.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tcg/optimize.c b/tcg/optimize.c
> index 53aa8e5329..d5bea37290 100644
> --- a/tcg/optimize.c
> +++ b/tcg/optimize.c
> @@ -1264,7 +1264,6 @@ void tcg_optimize(TCGContext *s)
>                  op->opc = opc = (opc == INDEX_op_movcond_i32
>                                   ? INDEX_op_setcond_i32
>                                   : INDEX_op_setcond_i64);
> -                nb_iargs = 2;
>              }
>              goto do_default;

I prefer not to make this change.

nb_iargs is the cached value that corresponds to opc, which we have just
changed.  If we later make a change at "do_default" that uses nb_iargs, failure
to update the value here will be a very hard bug to find.

I am happy to let the compiler remove this as dead code itself.


r~
Chenqun (kuhn) Aug. 17, 2020, 1:04 p.m. UTC | #2
> > diff --git a/tcg/optimize.c b/tcg/optimize.c index
> > 53aa8e5329..d5bea37290 100644
> > --- a/tcg/optimize.c
> > +++ b/tcg/optimize.c
> > @@ -1264,7 +1264,6 @@ void tcg_optimize(TCGContext *s)
> >                  op->opc = opc = (opc == INDEX_op_movcond_i32
> >                                   ? INDEX_op_setcond_i32
> >                                   : INDEX_op_setcond_i64);
> > -                nb_iargs = 2;
> >              }
> >              goto do_default;
> 
> I prefer not to make this change.
> 
> nb_iargs is the cached value that corresponds to opc, which we have just
> changed.  If we later make a change at "do_default" that uses nb_iargs,
> failure to update the value here will be a very hard bug to find.
Hi Richard,

  I think you're thinking more carefully, even though ' nb_iargs' not used in 'do_default' now.

However, I have only one small question. Why does 'nb_iargs' need to be assigned a value for this case branch?
Other branches(eg:' CASE_OP_32_64(brcond)') have changed the opc value too. Do we need to assign a value to nb_iargs for it?

Thanks.
Richard Henderson Aug. 18, 2020, 12:24 a.m. UTC | #3
On 8/17/20 6:04 AM, Chenqun (kuhn) wrote:
> Other branches(eg:' CASE_OP_32_64(brcond)') have changed the opc value too. Do we need to assign a value to nb_iargs for it?

In those cases nb_iargs does not change.


r~
diff mbox series

Patch

diff --git a/tcg/optimize.c b/tcg/optimize.c
index 53aa8e5329..d5bea37290 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -1264,7 +1264,6 @@  void tcg_optimize(TCGContext *s)
                 op->opc = opc = (opc == INDEX_op_movcond_i32
                                  ? INDEX_op_setcond_i32
                                  : INDEX_op_setcond_i64);
-                nb_iargs = 2;
             }
             goto do_default;