Re: [PATCH 3/7] disas/riscv.c: Support disas for Zcm* extensions

[Daniel Henrique Barboza]

On 5/22/23 11:27, Weiwei Li wrote:
> On 2023/5/22 21:10, Daniel Henrique Barboza wrote:
> > In fact, apparently checkpatch.pl is not too happy about this patch:
> >
> > On 5/18/23 23:19, Weiwei Li wrote:
> > > Support disas for Zcmt* instructions only when related extensions
> > > are supported.
> > >
> > > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> > > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> > > ---
> > >   disas/riscv.c | 20 ++++++++++++--------
> > >   1 file changed, 12 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/disas/riscv.c b/disas/riscv.c
> > > index 729ab684da..9e01810eef 100644
> > > --- a/disas/riscv.c
> > > +++ b/disas/riscv.c
> > > @@ -2501,7 +2501,7 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
> > >                   op = rv_op_c_sqsp;
> > >               } else {
> > >                   op = rv_op_c_fsdsp;
> > > -                if (((inst >> 12) & 0b01)) {
> > > +                if (dec->cfg->ext_zcmp && ((inst >> 12) & 0b01)) {
> > >                       switch ((inst >> 8) & 0b01111) {
> > >                       case 8:
> > >                           if (((inst >> 4) & 0b01111) >= 4) {
> > > @@ -2527,16 +2527,20 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa 
> > > isa)
> > >                   } else {
> > >                       switch ((inst >> 10) & 0b011) {
> > >                       case 0:
> > > -                        if (((inst >> 2) & 0xFF) >= 32) {
> > > -                            op = rv_op_cm_jalt;
> > > -                        } else {
> > > -                            op = rv_op_cm_jt;
> > > +                        if (dec->cfg->ext_zcmt) {
> > > +                            if (((inst >> 2) & 0xFF) >= 32) {
> > > +                                op = rv_op_cm_jalt;
> > > +                            } else {
> > > +                                op = rv_op_cm_jt;
> > > +                            }
> > >                           }
> > >                           break;
> > >                       case 3:
> > > -                        switch ((inst >> 5) & 0b011) {
> > > -                        case 1: op = rv_op_cm_mvsa01; break;
> > > -                        case 3: op = rv_op_cm_mva01s; break;
> > > +                        if (dec->cfg->ext_zcmp) {
> > > +                            switch ((inst >> 5) & 0b011) {
> > > +                            case 1: op = rv_op_cm_mvsa01; break;
> > > +                            case 3: op = rv_op_cm_mva01s; break;
> > > +                            }
> >
> > At this point:
> >
> > ================
> > 3/7 Checking commit 989059d476f9 (disas/riscv.c: Support disas for Zcm* 
> > extensions)
> > ERROR: trailing statements should be on next line
> > #51: FILE: disas/riscv.c:2541:
> > +                            case 1: op = rv_op_cm_mvsa01; break;
> >
> > ERROR: trailing statements should be on next line
> > #52: FILE: disas/riscv.c:2542:
> > +                            case 3: op = rv_op_cm_mva01s; break;
> >
> > total: 2 errors, 0 warnings, 35 lines checked
> >
> > Patch 3/7 has style problems, please review.  If any of these errors
> > are false positives report them to the maintainer, see
> > ================
> Yeah. I also found this errors when I ran the checkpatch.pl.
>
> However, this is the usual code style in this file. So I didn't fix it.

As long as the maintainer is ok with it (since it'll make noise in Gitlab when
running the test runners and so on) we can leave it as is.



Daniel

> Regards,
>
> Weiwei Li
>
> > The issue predates your patch. It would be very nice of you if you can fix it
> > though :D
> >
> >
> >
> > Daniel
> >
> >
> > >                           }
> > >                           break;
> > >                       }