| Message ID | 20181020071451.27808-3-kbastian@mail.uni-paderborn.de |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: Convert to decodetree | expand |
On 10/20/18 12:14 AM, Bastian Koppelmann wrote:
> + static int32_t ex_shift_##amount(int imm) \
Not that it'll matter in practice, but s/int32_t/int/.
That's the type that's passed in, and it's the type that
is stored in the argument set structures.
r~
On Sat, 20 Oct 2018 00:14:24 PDT (-0700), kbastian@mail.uni-paderborn.de wrote: > for now only LUI & AUIPC are decoded and translated. If decodetree fails, we > fall back to the old decoder. > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> > Signed-off-by: Peer Adelt <peer.adelt@hni.uni-paderborn.de> > --- > v1 -> v2: > - ex_shift_amount returns uint32_t You mean int32_t, right? That's what the code does, and I believe that's what it's supposed to do according to the ISA. > > target/riscv/Makefile.objs | 10 +++++++ > target/riscv/insn32.decode | 30 +++++++++++++++++++++ > target/riscv/insn_trans/trans_rvi.inc.c | 35 +++++++++++++++++++++++++ > target/riscv/translate.c | 24 ++++++++++++----- > 4 files changed, 92 insertions(+), 7 deletions(-) > create mode 100644 target/riscv/insn32.decode > create mode 100644 target/riscv/insn_trans/trans_rvi.inc.c > > diff --git a/target/riscv/Makefile.objs b/target/riscv/Makefile.objs > index abd0a7cde3..ea02f9b9ef 100644 > --- a/target/riscv/Makefile.objs > +++ b/target/riscv/Makefile.objs > @@ -1 +1,11 @@ > obj-y += translate.o op_helper.o helper.o cpu.o fpu_helper.o gdbstub.o pmp.o > + > +DECODETREE = $(SRC_PATH)/scripts/decodetree.py > + > +target/riscv/decode_insn32.inc.c: \ > + $(SRC_PATH)/target/riscv/insn32.decode $(DECODETREE) > + $(call quiet-command, \ > + $(PYTHON) $(DECODETREE) -o $@ --decode decode_insn32 $<, \ > + "GEN", $(TARGET_DIR)$@) > + > +target/riscv/translate.o: target/riscv/decode_insn32.inc.c > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode > new file mode 100644 > index 0000000000..44d4e922b6 > --- /dev/null > +++ b/target/riscv/insn32.decode > @@ -0,0 +1,30 @@ > +# > +# RISC-V translation routines for the RVXI Base Integer Instruction Set. > +# > +# Copyright (c) 2018 Peer Adelt, peer.adelt@hni.uni-paderborn.de > +# Bastian Koppelmann, kbastian@mail.uni-paderborn.de > +# > +# This program is free software; you can redistribute it and/or modify it > +# under the terms and conditions of the GNU General Public License, > +# version 2 or later, as published by the Free Software Foundation. > +# > +# This program is distributed in the hope it will be useful, but WITHOUT > +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > +# more details. > +# > +# You should have received a copy of the GNU General Public License along with > +# this program. If not, see <http://www.gnu.org/licenses/>. > + > +# Fields: > +%rd 7:5 > + > +# immediates: > +%imm_u 12:s20 !function=ex_shift_12 > + > +# Formats 32: > +@u .................... ..... ....... imm=%imm_u %rd > + > +# *** RV32I Base Instruction Set *** > +lui .................... ..... 0110111 @u > +auipc .................... ..... 0010111 @u > diff --git a/target/riscv/insn_trans/trans_rvi.inc.c b/target/riscv/insn_trans/trans_rvi.inc.c > new file mode 100644 > index 0000000000..aee0d1637d > --- /dev/null > +++ b/target/riscv/insn_trans/trans_rvi.inc.c > @@ -0,0 +1,35 @@ > +/* > + * RISC-V translation routines for the RVXI Base Integer Instruction Set. > + * > + * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu > + * Copyright (c) 2018 Peer Adelt, peer.adelt@hni.uni-paderborn.de > + * Bastian Koppelmann, kbastian@mail.uni-paderborn.de > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2 or later, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + * You should have received a copy of the GNU General Public License along with > + * this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +static bool trans_lui(DisasContext *ctx, arg_lui *a, uint32_t insn) > +{ > + if (a->rd != 0) { > + tcg_gen_movi_tl(cpu_gpr[a->rd], a->imm); > + } > + return true; > +} > + > +static bool trans_auipc(DisasContext *ctx, arg_auipc *a, uint32_t insn) > +{ > + if (a->rd != 0) { > + tcg_gen_movi_tl(cpu_gpr[a->rd], a->imm + ctx->base.pc_next); > + } > + return true; > +} > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index e81b9f097e..65a323a201 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -1667,6 +1667,19 @@ static void decode_RV32_64C(CPURISCVState *env, DisasContext *ctx) > } > } > > +#define EX_SH(amount) \ > + static int32_t ex_shift_##amount(int imm) \ > + { \ > + return imm << amount; \ > + } > +EX_SH(12) > + > +bool decode_insn32(DisasContext *ctx, uint32_t insn); > +/* Include the auto-generated decoder for 32 bit insn */ > +#include "decode_insn32.inc.c" > +/* Include insn module translation function */ > +#include "insn_trans/trans_rvi.inc.c" > + > static void decode_RV32_64G(CPURISCVState *env, DisasContext *ctx) > { > int rs1; > @@ -1687,12 +1700,6 @@ static void decode_RV32_64G(CPURISCVState *env, DisasContext *ctx) > imm = GET_IMM(ctx->opcode); > > switch (op) { > - case OPC_RISC_LUI: > - if (rd == 0) { > - break; /* NOP */ > - } > - tcg_gen_movi_tl(cpu_gpr[rd], sextract64(ctx->opcode, 12, 20) << 12); > - break; > case OPC_RISC_AUIPC: > if (rd == 0) { > break; /* NOP */ > @@ -1802,7 +1809,10 @@ static void decode_opc(DisasContext *ctx) > } > } else { > ctx->pc_succ_insn = ctx->base.pc_next + 4; > - decode_RV32_64G(ctx->env, ctx); > + if (!decode_insn32(ctx, ctx->opcode)) { > + /* fallback to old decoder */ > + decode_RV32_64G(ctx->env, ctx); > + } > } > } Reviewed-by: Palmer Dabbelt <palmer@sifive.com> How do you want to go about merging these? It looks like it should be possible to merge the patch set piecemeal, which I'd actually be happy doing as it'll be easier to get these out for testing that way. That way we can avoid having one day where everyone changes over to a new decoder. If that's OK then I'll start slurping up this up into my weekly pull requests as I can review them, with the idea being that I won't re-order anything (in other words, I'll start at the first patch and take patches until I find one that isn't ready to go). Thanks!
On 10/25/18 6:58 PM, Palmer Dabbelt wrote: > > Reviewed-by: Palmer Dabbelt <palmer@sifive.com> > > How do you want to go about merging these? It looks like it should be > possible to merge the patch set piecemeal, which I'd actually be happy > doing as it'll be easier to get these out for testing that way. That > way we can avoid having one day where everyone changes over to a new > decoder. If that's OK then I'll start slurping up this up into my > weekly pull requests as I can review them, with the idea being that I > won't re-order anything (in other words, I'll start at the first patch > and take patches until I find one that isn't ready to go). I think you can pick up everything up to the RVC conversion which still needs the work suggested by Richard. Thanks, for picking it up :) Cheers, Bastian
On 10/26/18 11:49 AM, Bastian Koppelmann wrote: > I think you can pick up everything up to the RVC conversion which still needs > the work suggested by Richard. Thanks, for picking it up :) Even then I thought we were talking about splitting the RV64 insns into a separate file, reducing the ifdefs, and renaming the arg-sets to match the instruction formats described in the riscv spec. r~
On 10/26/18 3:58 PM, Richard Henderson wrote: > On 10/26/18 11:49 AM, Bastian Koppelmann wrote: >> I think you can pick up everything up to the RVC conversion which still needs >> the work suggested by Richard. Thanks, for picking it up :) > Even then I thought we were talking about splitting the RV64 insns > into a separate file, reducing the ifdefs, and renaming the arg-sets > to match the instruction formats described in the riscv spec. Yes, you are right I forgot that. Cheers, Bastian
On Fri, 26 Oct 2018 07:53:17 PDT (-0700), Bastian Koppelmann wrote: > > On 10/26/18 3:58 PM, Richard Henderson wrote: >> On 10/26/18 11:49 AM, Bastian Koppelmann wrote: >>> I think you can pick up everything up to the RVC conversion which still needs >>> the work suggested by Richard. Thanks, for picking it up :) >> Even then I thought we were talking about splitting the RV64 insns >> into a separate file, reducing the ifdefs, and renaming the arg-sets >> to match the instruction formats described in the riscv spec. > > > Yes, you are right I forgot that. OK, so I think I'll hold off for a v3, then.
diff --git a/target/riscv/Makefile.objs b/target/riscv/Makefile.objs index abd0a7cde3..ea02f9b9ef 100644 --- a/target/riscv/Makefile.objs +++ b/target/riscv/Makefile.objs @@ -1 +1,11 @@ obj-y += translate.o op_helper.o helper.o cpu.o fpu_helper.o gdbstub.o pmp.o + +DECODETREE = $(SRC_PATH)/scripts/decodetree.py + +target/riscv/decode_insn32.inc.c: \ + $(SRC_PATH)/target/riscv/insn32.decode $(DECODETREE) + $(call quiet-command, \ + $(PYTHON) $(DECODETREE) -o $@ --decode decode_insn32 $<, \ + "GEN", $(TARGET_DIR)$@) + +target/riscv/translate.o: target/riscv/decode_insn32.inc.c diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode new file mode 100644 index 0000000000..44d4e922b6 --- /dev/null +++ b/target/riscv/insn32.decode @@ -0,0 +1,30 @@ +# +# RISC-V translation routines for the RVXI Base Integer Instruction Set. +# +# Copyright (c) 2018 Peer Adelt, peer.adelt@hni.uni-paderborn.de +# Bastian Koppelmann, kbastian@mail.uni-paderborn.de +# +# This program is free software; you can redistribute it and/or modify it +# under the terms and conditions of the GNU General Public License, +# version 2 or later, as published by the Free Software Foundation. +# +# This program is distributed in the hope it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# this program. If not, see <http://www.gnu.org/licenses/>. + +# Fields: +%rd 7:5 + +# immediates: +%imm_u 12:s20 !function=ex_shift_12 + +# Formats 32: +@u .................... ..... ....... imm=%imm_u %rd + +# *** RV32I Base Instruction Set *** +lui .................... ..... 0110111 @u +auipc .................... ..... 0010111 @u diff --git a/target/riscv/insn_trans/trans_rvi.inc.c b/target/riscv/insn_trans/trans_rvi.inc.c new file mode 100644 index 0000000000..aee0d1637d --- /dev/null +++ b/target/riscv/insn_trans/trans_rvi.inc.c @@ -0,0 +1,35 @@ +/* + * RISC-V translation routines for the RVXI Base Integer Instruction Set. + * + * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu + * Copyright (c) 2018 Peer Adelt, peer.adelt@hni.uni-paderborn.de + * Bastian Koppelmann, kbastian@mail.uni-paderborn.de + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2 or later, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see <http://www.gnu.org/licenses/>. + */ + +static bool trans_lui(DisasContext *ctx, arg_lui *a, uint32_t insn) +{ + if (a->rd != 0) { + tcg_gen_movi_tl(cpu_gpr[a->rd], a->imm); + } + return true; +} + +static bool trans_auipc(DisasContext *ctx, arg_auipc *a, uint32_t insn) +{ + if (a->rd != 0) { + tcg_gen_movi_tl(cpu_gpr[a->rd], a->imm + ctx->base.pc_next); + } + return true; +} diff --git a/target/riscv/translate.c b/target/riscv/translate.c index e81b9f097e..65a323a201 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -1667,6 +1667,19 @@ static void decode_RV32_64C(CPURISCVState *env, DisasContext *ctx) } } +#define EX_SH(amount) \ + static int32_t ex_shift_##amount(int imm) \ + { \ + return imm << amount; \ + } +EX_SH(12) + +bool decode_insn32(DisasContext *ctx, uint32_t insn); +/* Include the auto-generated decoder for 32 bit insn */ +#include "decode_insn32.inc.c" +/* Include insn module translation function */ +#include "insn_trans/trans_rvi.inc.c" + static void decode_RV32_64G(CPURISCVState *env, DisasContext *ctx) { int rs1; @@ -1687,12 +1700,6 @@ static void decode_RV32_64G(CPURISCVState *env, DisasContext *ctx) imm = GET_IMM(ctx->opcode); switch (op) { - case OPC_RISC_LUI: - if (rd == 0) { - break; /* NOP */ - } - tcg_gen_movi_tl(cpu_gpr[rd], sextract64(ctx->opcode, 12, 20) << 12); - break; case OPC_RISC_AUIPC: if (rd == 0) { break; /* NOP */ @@ -1802,7 +1809,10 @@ static void decode_opc(DisasContext *ctx) } } else { ctx->pc_succ_insn = ctx->base.pc_next + 4; - decode_RV32_64G(ctx->env, ctx); + if (!decode_insn32(ctx, ctx->opcode)) { + /* fallback to old decoder */ + decode_RV32_64G(ctx->env, ctx); + } } }