| Message ID | 20241111224452.61276-3-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/mips: Convert nanoMIPS LSA opcode to decodetree | expand |
On 11/11/24 14:44, Philippe Mathieu-Daudé wrote: > From: Philippe Mathieu-Daudé <f4bug@amsat.org> > > Simply call the generic gen_lsa() helper, taking care > to substract 1 to the shift field. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > target/mips/tcg/nanomips32.decode | 8 ++++++++ > target/mips/tcg/nanomips_translate.c | 12 ++++++++++++ > target/mips/tcg/nanomips_translate.c.inc | 9 --------- > 3 files changed, 20 insertions(+), 9 deletions(-) > > diff --git a/target/mips/tcg/nanomips32.decode b/target/mips/tcg/nanomips32.decode > index 9cecf1e13d..11bf5cd6c4 100644 > --- a/target/mips/tcg/nanomips32.decode > +++ b/target/mips/tcg/nanomips32.decode > @@ -6,3 +6,11 @@ > # > # Reference: nanoMIPS32 Instruction Set Technical Reference Manual > # (Document Number: MD01247) > + > +&r rs rt rd sa > + > +%lsa_u2 9:2 !function=minus_1 > + > +@lsa ...... rt:5 rs:5 rd:5 .. --- ... ... &r sa=%lsa_u2 > + > +LSA 001000 ..... ..... ..... .. ... 001 111 @lsa > diff --git a/target/mips/tcg/nanomips_translate.c b/target/mips/tcg/nanomips_translate.c > index c148c13ed9..9a6db4a828 100644 > --- a/target/mips/tcg/nanomips_translate.c > +++ b/target/mips/tcg/nanomips_translate.c > @@ -9,6 +9,18 @@ > #include "qemu/osdep.h" > #include "translate.h" > > +static inline int minus_1(DisasContext *ctx, int x) > +{ > + return x - 1; > +} > + > /* Include the auto-generated decoders. */ > #include "decode-nanomips16.c.inc" > #include "decode-nanomips32.c.inc" > + > +static bool trans_LSA(DisasContext *ctx, arg_r *a) > +{ > + gen_lsa(ctx, a->rd, a->rt, a->rs, a->sa); I think decoding shift-amount - 1 is misleading. Surely it would be better for other callers to pass extract(...) + 1? r~
On 12/11/24 14:30, Richard Henderson wrote: > On 11/11/24 14:44, Philippe Mathieu-Daudé wrote: >> From: Philippe Mathieu-Daudé <f4bug@amsat.org> >> >> Simply call the generic gen_lsa() helper, taking care >> to substract 1 to the shift field. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> target/mips/tcg/nanomips32.decode | 8 ++++++++ >> target/mips/tcg/nanomips_translate.c | 12 ++++++++++++ >> target/mips/tcg/nanomips_translate.c.inc | 9 --------- >> 3 files changed, 20 insertions(+), 9 deletions(-) >> diff --git a/target/mips/tcg/nanomips_translate.c >> b/target/mips/tcg/nanomips_translate.c >> index c148c13ed9..9a6db4a828 100644 >> --- a/target/mips/tcg/nanomips_translate.c >> +++ b/target/mips/tcg/nanomips_translate.c >> @@ -9,6 +9,18 @@ >> #include "qemu/osdep.h" >> #include "translate.h" >> +static inline int minus_1(DisasContext *ctx, int x) >> +{ >> + return x - 1; >> +} >> + >> /* Include the auto-generated decoders. */ >> #include "decode-nanomips16.c.inc" >> #include "decode-nanomips32.c.inc" >> + >> +static bool trans_LSA(DisasContext *ctx, arg_r *a) >> +{ >> + gen_lsa(ctx, a->rd, a->rt, a->rs, a->sa); > > I think decoding shift-amount - 1 is misleading. Yeah I thought the same while rebasing ... > Surely it would be better for other callers to pass extract(...) + 1? ... but was too lazy to do the changes :p > > > r~ >
diff --git a/target/mips/tcg/nanomips32.decode b/target/mips/tcg/nanomips32.decode index 9cecf1e13d..11bf5cd6c4 100644 --- a/target/mips/tcg/nanomips32.decode +++ b/target/mips/tcg/nanomips32.decode @@ -6,3 +6,11 @@ # # Reference: nanoMIPS32 Instruction Set Technical Reference Manual # (Document Number: MD01247) + +&r rs rt rd sa + +%lsa_u2 9:2 !function=minus_1 + +@lsa ...... rt:5 rs:5 rd:5 .. --- ... ... &r sa=%lsa_u2 + +LSA 001000 ..... ..... ..... .. ... 001 111 @lsa diff --git a/target/mips/tcg/nanomips_translate.c b/target/mips/tcg/nanomips_translate.c index c148c13ed9..9a6db4a828 100644 --- a/target/mips/tcg/nanomips_translate.c +++ b/target/mips/tcg/nanomips_translate.c @@ -9,6 +9,18 @@ #include "qemu/osdep.h" #include "translate.h" +static inline int minus_1(DisasContext *ctx, int x) +{ + return x - 1; +} + /* Include the auto-generated decoders. */ #include "decode-nanomips16.c.inc" #include "decode-nanomips32.c.inc" + +static bool trans_LSA(DisasContext *ctx, arg_r *a) +{ + gen_lsa(ctx, a->rd, a->rt, a->rs, a->sa); + + return true; +} diff --git a/target/mips/tcg/nanomips_translate.c.inc b/target/mips/tcg/nanomips_translate.c.inc index e401b92bfd..0e012ab3d0 100644 --- a/target/mips/tcg/nanomips_translate.c.inc +++ b/target/mips/tcg/nanomips_translate.c.inc @@ -399,7 +399,6 @@ enum { /* POOL32A7 instruction pool */ enum { NM_P_LSX = 0x00, - NM_LSA = 0x01, NM_EXTW = 0x03, NM_POOL32AXF = 0x07, }; @@ -3625,14 +3624,6 @@ static int decode_nanomips_32_48_opc(CPUMIPSState *env, DisasContext *ctx) case NM_P_LSX: gen_p_lsx(ctx, rd, rs, rt); break; - case NM_LSA: - /* - * In nanoMIPS, the shift field directly encodes the shift - * amount, meaning that the supported shift values are in - * the range 0 to 3 (instead of 1 to 4 in MIPSR6). - */ - gen_lsa(ctx, rd, rt, rs, extract32(ctx->opcode, 9, 2) - 1); - break; case NM_EXTW: gen_ext(ctx, 32, rd, rs, rt, extract32(ctx->opcode, 6, 5)); break;