Re: [PATCH v6 10/61] target/riscv: vector single-width integer add and subtract

[LIU Zhiwei]

On 2020/3/28 7:54, Richard Henderson wrote:
> On 3/17/20 8:06 AM, LIU Zhiwei wrote:
> > +    if (a->vm && s->vl_eq_vlmax) {
> > +        gvec_fn(s->sew, vreg_ofs(s, a->rd),
> > +            vreg_ofs(s, a->rs2), vreg_ofs(s, a->rs1),
> > +            MAXSZ(s), MAXSZ(s));
> Indentation is off here.
Do you mean I should adjust the indentation for parameters in gvec_fn like

+    if (a->vm && s->vl_eq_vlmax) {
+        gvec_fn(s->sew, vreg_ofs(s, a->rd),
+                vreg_ofs(s, a->rs2), vreg_ofs(s, a->rs1),
+                MAXSZ(s), MAXSZ(s));

> > +static inline bool
> > +do_opivx_gvec(DisasContext *s, arg_rmrr *a, GVecGen2sFn *gvec_fn,
> > +              gen_helper_opivx *fn)
> > +{
> > +    if (!opivx_check(s, a)) {
> > +        return false;
> > +    }
> > +
> > +    if (a->vm && s->vl_eq_vlmax) {
> > +        TCGv_i64 src1 = tcg_temp_new_i64();
> > +        TCGv tmp = tcg_temp_new();
> > +
> > +        gen_get_gpr(tmp, a->rs1);
> > +        tcg_gen_ext_tl_i64(src1, tmp);
> > +        gvec_fn(s->sew, vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2),
> > +                src1, MAXSZ(s), MAXSZ(s));
> > +
> > +        tcg_temp_free_i64(src1);
> > +        tcg_temp_free(tmp);
> > +        return true;
> > +    } else {
> > +        return opivx_trans(a->rd, a->rs1, a->rs2, a->vm, fn, s);
> > +    }
> > +    return true;
> > +}
> This final return is unreachable, and I'm sure some static analyzer (e.g.
> Coverity) will complain.
>
> Since the if-then has a return, we can drop the else like so:
>
>      if (a->vm && s->vl_eq_vlmax) {
>          ...
>          return true;
>      }
>      return opivx_trans(a->rd, a->rs1, a->rs2, a->vm, fn, s);
Yes, it's tidier. Thanks.

Zhiwei
> Otherwise,
> Reviewed-by: Richard Henderson <address@hidden>
>
> r~