[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-riscv] [PATCH v1 2/4] disas/riscv: Disassemble reserved compre
From: |
Alistair Francis |
Subject: |
Re: [Qemu-riscv] [PATCH v1 2/4] disas/riscv: Disassemble reserved compressed encodings as illegal |
Date: |
Wed, 19 Jun 2019 13:26:35 -0700 |
On Fri, Jun 14, 2019 at 2:18 AM Palmer Dabbelt <address@hidden> wrote:
>
> On Fri, 17 May 2019 15:11:01 PDT (-0700), Alistair Francis wrote:
> > From: Michael Clark <address@hidden>
> >
> > Due to the design of the disassembler, the immediate is not
> > known during decoding of the opcode; so to handle compressed
> > encodings with reserved immediate values (non-zero), we need
> > to add an additional check during decompression to match
> > reserved encodings with zero immediates and translate them
> > into the illegal instruction.
> >
> > The following compressed opcodes have reserved encodings with
> > zero immediates: c.addi4spn, c.addi, c.lui, c.addi16sp, c.srli,
> > c.srai, c.andi and c.slli
> >
> > Signed-off-by: Michael Clark <address@hidden>
> > Signed-off-by: Alistair Francis <address@hidden>
> > ---
> > disas/riscv.c | 51 ++++++++++++++++++++++++++++++++++-----------------
> > 1 file changed, 34 insertions(+), 17 deletions(-)
> >
> > diff --git a/disas/riscv.c b/disas/riscv.c
> > index 59a9b0437a..3ab4586f0a 100644
> > --- a/disas/riscv.c
> > +++ b/disas/riscv.c
> > @@ -504,14 +504,19 @@ typedef struct {
> > const rvc_constraint *constraints;
> > } rv_comp_data;
> >
> > +enum {
> > + rvcd_imm_nz = 0x1
> > +};
> > +
> > typedef struct {
> > const char * const name;
> > const rv_codec codec;
> > const char * const format;
> > const rv_comp_data *pseudo;
> > - const int decomp_rv32;
> > - const int decomp_rv64;
> > - const int decomp_rv128;
> > + const short decomp_rv32;
> > + const short decomp_rv64;
> > + const short decomp_rv128;
> > + const short decomp_data;
> > } rv_opcode_data;
> >
> > /* register names */
> > @@ -1011,7 +1016,7 @@ const rv_opcode_data opcode_data[] = {
> > { "fcvt.q.lu", rv_codec_r_m, rv_fmt_rm_frd_rs1, NULL, 0, 0, 0 },
> > { "fmv.x.q", rv_codec_r, rv_fmt_rd_frs1, NULL, 0, 0, 0 },
> > { "fmv.q.x", rv_codec_r, rv_fmt_frd_rs1, NULL, 0, 0, 0 },
> > - { "c.addi4spn", rv_codec_ciw_4spn, rv_fmt_rd_rs1_imm, NULL,
> > rv_op_addi, rv_op_addi, rv_op_addi },
> > + { "c.addi4spn", rv_codec_ciw_4spn, rv_fmt_rd_rs1_imm, NULL,
> > rv_op_addi, rv_op_addi, rv_op_addi, rvcd_imm_nz },
> > { "c.fld", rv_codec_cl_ld, rv_fmt_frd_offset_rs1, NULL, rv_op_fld,
> > rv_op_fld, 0 },
> > { "c.lw", rv_codec_cl_lw, rv_fmt_rd_offset_rs1, NULL, rv_op_lw,
> > rv_op_lw, rv_op_lw },
> > { "c.flw", rv_codec_cl_lw, rv_fmt_frd_offset_rs1, NULL, rv_op_flw, 0,
> > 0 },
> > @@ -1019,14 +1024,14 @@ const rv_opcode_data opcode_data[] = {
> > { "c.sw", rv_codec_cs_sw, rv_fmt_rs2_offset_rs1, NULL, rv_op_sw,
> > rv_op_sw, rv_op_sw },
> > { "c.fsw", rv_codec_cs_sw, rv_fmt_frs2_offset_rs1, NULL, rv_op_fsw, 0,
> > 0 },
> > { "c.nop", rv_codec_ci_none, rv_fmt_none, NULL, rv_op_addi,
> > rv_op_addi, rv_op_addi },
> > - { "c.addi", rv_codec_ci, rv_fmt_rd_rs1_imm, NULL, rv_op_addi,
> > rv_op_addi, rv_op_addi },
> > + { "c.addi", rv_codec_ci, rv_fmt_rd_rs1_imm, NULL, rv_op_addi,
> > rv_op_addi, rv_op_addi, rvcd_imm_nz },
> > { "c.jal", rv_codec_cj_jal, rv_fmt_rd_offset, NULL, rv_op_jal, 0, 0 },
> > { "c.li", rv_codec_ci_li, rv_fmt_rd_rs1_imm, NULL, rv_op_addi,
> > rv_op_addi, rv_op_addi },
> > - { "c.addi16sp", rv_codec_ci_16sp, rv_fmt_rd_rs1_imm, NULL, rv_op_addi,
> > rv_op_addi, rv_op_addi },
> > - { "c.lui", rv_codec_ci_lui, rv_fmt_rd_imm, NULL, rv_op_lui, rv_op_lui,
> > rv_op_lui },
> > - { "c.srli", rv_codec_cb_sh6, rv_fmt_rd_rs1_imm, NULL, rv_op_srli,
> > rv_op_srli, rv_op_srli },
> > - { "c.srai", rv_codec_cb_sh6, rv_fmt_rd_rs1_imm, NULL, rv_op_srai,
> > rv_op_srai, rv_op_srai },
> > - { "c.andi", rv_codec_cb_imm, rv_fmt_rd_rs1_imm, NULL, rv_op_andi,
> > rv_op_andi, rv_op_andi },
> > + { "c.addi16sp", rv_codec_ci_16sp, rv_fmt_rd_rs1_imm, NULL, rv_op_addi,
> > rv_op_addi, rv_op_addi, rvcd_imm_nz },
> > + { "c.lui", rv_codec_ci_lui, rv_fmt_rd_imm, NULL, rv_op_lui, rv_op_lui,
> > rv_op_lui, rvcd_imm_nz },
> > + { "c.srli", rv_codec_cb_sh6, rv_fmt_rd_rs1_imm, NULL, rv_op_srli,
> > rv_op_srli, rv_op_srli, rvcd_imm_nz },
> > + { "c.srai", rv_codec_cb_sh6, rv_fmt_rd_rs1_imm, NULL, rv_op_srai,
> > rv_op_srai, rv_op_srai, rvcd_imm_nz },
> > + { "c.andi", rv_codec_cb_imm, rv_fmt_rd_rs1_imm, NULL, rv_op_andi,
> > rv_op_andi, rv_op_andi, rvcd_imm_nz },
>
> Unless I'm missing something, c.andi can have a zero immediate.
Yeah, I'll remove that.
Alistair
>
> > { "c.sub", rv_codec_cs, rv_fmt_rd_rs1_rs2, NULL, rv_op_sub, rv_op_sub,
> > rv_op_sub },
> > { "c.xor", rv_codec_cs, rv_fmt_rd_rs1_rs2, NULL, rv_op_xor, rv_op_xor,
> > rv_op_xor },
> > { "c.or", rv_codec_cs, rv_fmt_rd_rs1_rs2, NULL, rv_op_or, rv_op_or,
> > rv_op_or },
> > @@ -1036,7 +1041,7 @@ const rv_opcode_data opcode_data[] = {
> > { "c.j", rv_codec_cj, rv_fmt_rd_offset, NULL, rv_op_jal, rv_op_jal,
> > rv_op_jal },
> > { "c.beqz", rv_codec_cb, rv_fmt_rs1_rs2_offset, NULL, rv_op_beq,
> > rv_op_beq, rv_op_beq },
> > { "c.bnez", rv_codec_cb, rv_fmt_rs1_rs2_offset, NULL, rv_op_bne,
> > rv_op_bne, rv_op_bne },
> > - { "c.slli", rv_codec_ci_sh6, rv_fmt_rd_rs1_imm, NULL, rv_op_slli,
> > rv_op_slli, rv_op_slli },
> > + { "c.slli", rv_codec_ci_sh6, rv_fmt_rd_rs1_imm, NULL, rv_op_slli,
> > rv_op_slli, rv_op_slli, rvcd_imm_nz },
> > { "c.fldsp", rv_codec_ci_ldsp, rv_fmt_frd_offset_rs1, NULL, rv_op_fld,
> > rv_op_fld, rv_op_fld },
> > { "c.lwsp", rv_codec_ci_lwsp, rv_fmt_rd_offset_rs1, NULL, rv_op_lw,
> > rv_op_lw, rv_op_lw },
> > { "c.flwsp", rv_codec_ci_lwsp, rv_fmt_frd_offset_rs1, NULL, rv_op_flw,
> > 0, 0 },
> > @@ -2795,8 +2800,12 @@ static void decode_inst_decompress_rv32(rv_decode
> > *dec)
> > {
> > int decomp_op = opcode_data[dec->op].decomp_rv32;
> > if (decomp_op != rv_op_illegal) {
> > - dec->op = decomp_op;
> > - dec->codec = opcode_data[decomp_op].codec;
> > + if ((opcode_data[dec->op].decomp_data & rvcd_imm_nz) && dec->imm
> > == 0) {
> > + dec->op = rv_op_illegal;
> > + } else {
> > + dec->op = decomp_op;
> > + dec->codec = opcode_data[decomp_op].codec;
> > + }
> > }
> > }
> >
> > @@ -2804,8 +2813,12 @@ static void decode_inst_decompress_rv64(rv_decode
> > *dec)
> > {
> > int decomp_op = opcode_data[dec->op].decomp_rv64;
> > if (decomp_op != rv_op_illegal) {
> > - dec->op = decomp_op;
> > - dec->codec = opcode_data[decomp_op].codec;
> > + if ((opcode_data[dec->op].decomp_data & rvcd_imm_nz) && dec->imm
> > == 0) {
> > + dec->op = rv_op_illegal;
> > + } else {
> > + dec->op = decomp_op;
> > + dec->codec = opcode_data[decomp_op].codec;
> > + }
> > }
> > }
> >
> > @@ -2813,8 +2826,12 @@ static void decode_inst_decompress_rv128(rv_decode
> > *dec)
> > {
> > int decomp_op = opcode_data[dec->op].decomp_rv128;
> > if (decomp_op != rv_op_illegal) {
> > - dec->op = decomp_op;
> > - dec->codec = opcode_data[decomp_op].codec;
> > + if ((opcode_data[dec->op].decomp_data & rvcd_imm_nz) && dec->imm
> > == 0) {
> > + dec->op = rv_op_illegal;
> > + } else {
> > + dec->op = decomp_op;
> > + dec->codec = opcode_data[decomp_op].codec;
> > + }
> > }
> > }