| Message ID | 20211019094812.614056-5-frederic.petrot@univ-grenoble-alpes.fr |
|---|---|
| State | New |
| Headers | show |
| Series | Adding partial support for 128-bit riscv target | expand |
On 10/19/21 2:47 AM, Frédéric Pétrot wrote: > Given that the 128-bit version of the riscv spec adds new instructions, and > that some instructions that were previously only available in 64-bit mode > are now available for both 64-bit and 128-bit, we added new macros to check > for the processor mode during translation. > > Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr> > Co-authored-by: Fabien Portas <fabien.portas@grenoble-inp.org> > --- > target/riscv/translate.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index 35245aafa7..121fcd71fe 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -350,6 +350,24 @@ EX_SH(12) > } \ > } while (0) > > +#define REQUIRE_128BIT(ctx) do { \ > + if (get_xl(ctx) < MXL_RV128) { \ > + return false; \ > + } \ > +} while (0) > + > +#define REQUIRE_32_OR_64BIT(ctx) do { \ > + if (get_xl(ctx) == MXL_RV128) { \ > + return false; \ > + } \ > +} while (0) > + > +#define REQUIRE_64_OR_128BIT(ctx) do { \ > + if (get_xl(ctx) == MXL_RV32) { \ > + return false; \ > + } \ > +} while (0) So... you've left REQUIRE_64BIT accepting RV128, so that means that your current REQUIRE_64_OR_128BIT is redundant. Is that intentional? It does seem like all places that accept RV128 should accept RV64, but perhaps that's just your "limited" caveat in the cover letter. You don't use REQUIRE_32_OR_64BIT at all. Remove it? r~
Le 20/10/2021 à 16:08, Richard Henderson a écrit : > On 10/19/21 2:47 AM, Frédéric Pétrot wrote: >> + >> +#define REQUIRE_64_OR_128BIT(ctx) do { \ >> + if (get_xl(ctx) == MXL_RV32) { \ >> + return false; \ >> + } \ >> +} while (0) > > So... you've left REQUIRE_64BIT accepting RV128, so that means that your current > REQUIRE_64_OR_128BIT is redundant. Is that intentional? > > It does seem like all places that accept RV128 should accept RV64, but perhaps > that's just your "limited" caveat in the cover letter. My bad, indeed there is no instruction only required by RV64. "Limited" was related to the minimal support of the priviledge spec. > You don't use REQUIRE_32_OR_64BIT at all. Remove it? It's a bug : some compressed insns are only RV32/RV64 (this is linked to the other bug in the order in which the insns stand in the insn16.decode file that you pointed out). Frédéric > > > r~
diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 35245aafa7..121fcd71fe 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -350,6 +350,24 @@ EX_SH(12) } \ } while (0) +#define REQUIRE_128BIT(ctx) do { \ + if (get_xl(ctx) < MXL_RV128) { \ + return false; \ + } \ +} while (0) + +#define REQUIRE_32_OR_64BIT(ctx) do { \ + if (get_xl(ctx) == MXL_RV128) { \ + return false; \ + } \ +} while (0) + +#define REQUIRE_64_OR_128BIT(ctx) do { \ + if (get_xl(ctx) == MXL_RV32) { \ + return false; \ + } \ +} while (0) + static int ex_rvc_register(DisasContext *ctx, int reg) { return 8 + reg;
Given that the 128-bit version of the riscv spec adds new instructions, and that some instructions that were previously only available in 64-bit mode are now available for both 64-bit and 128-bit, we added new macros to check for the processor mode during translation. Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr> Co-authored-by: Fabien Portas <fabien.portas@grenoble-inp.org> --- target/riscv/translate.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)