RE: [PATCH v3] Hexagon (target/hexagon) Additional instructions handled by idef-parser

[Taylor Simpson]

> -----Original Message-----
> From: Anton Johansson <anjo@rev.ng>
> Sent: Tuesday, May 2, 2023 6:12 AM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: richard.henderson@linaro.org; philmd@linaro.org; ale@rev.ng; Brian Cain
> <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>
> Subject: Re: [PATCH v3] Hexagon (target/hexagon) Additional instructions
> handled by idef-parser
> 
> On 5/1/23 22:31, Taylor Simpson wrote:
> > diff --git a/target/hexagon/idef-parser/parser-helpers.c
> > b/target/hexagon/idef-parser/parser-helpers.c
> > index 86511efb62..0ad917f591 100644
> > --- a/target/hexagon/idef-parser/parser-helpers.c
> > +++ b/target/hexagon/idef-parser/parser-helpers.c
> > -961,9 +973,16 @@ HexValue gen_cast_op(Context *c,
> >   {
> >       assert_signedness(c, locp, src->signedness);
> >       if (src->bit_width == target_width) {
> > -        return *src;
> > -    } else if (src->type == IMMEDIATE) {
> >           HexValue res = *src;
> > +        res.signedness = signedness;
> > +        return res;
> > +    } else if (src->type == IMMEDIATE) {
> > +        HexValue res;
> > +        if (src->bit_width < target_width) {
> > +            res = gen_rvalue_extend(c, locp, src);
> > +        } else {
> > +            res = *src;
> > +        }
> >           res.bit_width = target_width;
> >           res.signedness = signedness;
> >           return res;
> 
> Ah, gen_cast_op() can be simplified a great deal here to
> 
>      HexValue gen_cast_op(Context *c,
>                           YYLTYPE *locp,
>                           HexValue *src,
>                           unsigned target_width,
>                           HexSignedness signedness)
>      {
>          HexValue res;
>          assert_signedness(c, locp, src->signedness);
>          if (src->bit_width == target_width) {
>              res = *src;
>          } else if (src->bit_width < target_width) {
>              res = gen_rvalue_extend(c, locp, src);
>          } else {
>              res = gen_rvalue_truncate(c, locp, src);
>          }
>          res.signedness = signedness;
>          return res;
>      }

Great suggestion, thanks!

> 
> Other than that this patch looks good:
> 
> Tested-by: Anton Johansson <anjo@rev.ng>
> Reviewed-by: Anton Johansson <anjo@rev.ng>
>