Re: [PATCH 2/4] gdb/riscv: Extend instruction decode to cover more instructions

[Palmer Dabbelt <palmer@sifive.com>]

On Wed, 29 Aug 2018 09:40:52 PDT (-0700), andrew.burgess@embecosm.com wrote:
> Extends the instruction decoder used during prologue scan and software
> single step to cover more instructions.  These instructions are
> encountered when running the current GDB testsuite with the DWARF
> stack unwinders turned off.
>
> gdb/ChangeLog:
>
> 	* riscv-tdep.c (riscv_insn::decode): Decode c.addi4spn, c.sd,
> 	c.sw, c.swsp, and c.sdsp.
> ---
>  gdb/ChangeLog    |  5 +++++
>  gdb/riscv-tdep.c | 40 ++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 2f619c35e75..b4ac83a6dd9 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -972,6 +972,31 @@ private:
>      m_imm.s = EXTRACT_STYPE_IMM (ival);
>    }
>
> +  /* Helper for DECODE, decode 16-bit CS-type instruction.  The immediate
> +     encoding is different for each CSS format instruction, so extracting

I think you mean "each CS format" here -- it's true for CSS formats as well, 
just not relevant until below.

> +     the immediate is left up to the caller, who should pass the extracted
> +     immediate value through in IMM.  */
> +  void decode_cs_type_insn (enum opcode opcode, ULONGEST ival, int imm)
> +  {
> +    m_opcode = opcode;
> +    m_imm.s = imm;
> +    m_rs1 = decode_register_index_short (ival, OP_SH_CRS1S);
> +    m_rs2 = decode_register_index_short (ival, OP_SH_CRS2S);
> +  }
> +
> +  /* Helper for DECODE, decode 16-bit CSS-type instruction.  The immediate
> +     encoding is different for each CSS format instruction, so extracting
> +     the immediate is left up to the caller, who should pass the extracted
> +     immediate value through in IMM.  */
> +  void decode_css_type_insn (enum opcode opcode, ULONGEST ival, int imm)
> +  {
> +    m_opcode = opcode;
> +    m_imm.s = imm;
> +    m_rs1 = RISCV_SP_REGNUM;
> +    /* Not a compressed register number in this case.  */
> +    m_rs2 = decode_register_index (ival, OP_SH_CRS2);
> +  }
> +
>    /* Helper for DECODE, decode 32-bit U-type instruction.  */
>    void decode_u_type_insn (enum opcode opcode, ULONGEST ival)
>    {
> @@ -1165,14 +1190,25 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
>  	  m_rd = m_rs1 = decode_register_index (ival, OP_SH_RD);
>  	  m_imm.s = EXTRACT_RVC_ADDI16SP_IMM (ival);
>  	}
> +      else if (xlen < 16 && is_c_addi4spn_insn (ival))
> +	{
> +	  m_opcode = ADDI;
> +	  m_rd = decode_register_index_short (ival, OP_SH_CRS2S);
> +	  m_rs1 = RISCV_SP_REGNUM;
> +	  m_imm.s = EXTRACT_RVC_ADDI4SPN_IMM (ival);

I think this is OK: the immediate is actually unsigned, but it's only 10 bits 
long so it doesn't matter.

> +	}
>        else if (is_c_lui_insn (ival))
>  	m_opcode = OTHER;
>        /* C_SD and C_FSW have the same opcode.  C_SD is RV64 and RV128 only,
>  	 and C_FSW is RV32 only.  */
>        else if (xlen != 4 && is_c_sd_insn (ival))
> -	m_opcode = OTHER;
> +	decode_cs_type_insn (SD, ival, EXTRACT_RVC_LD_IMM (ival));
>        else if (is_c_sw_insn (ival))
> -	m_opcode = OTHER;
> +	decode_cs_type_insn (SW, ival, EXTRACT_RVC_LW_IMM (ival));
> +      else if (is_c_swsp_insn (ival))
> +	decode_css_type_insn (SW, ival, EXTRACT_RVC_SWSP_IMM (ival));
> +      else if (is_c_sdsp_insn (ival))
> +	decode_css_type_insn (SW, ival, EXTRACT_RVC_SDSP_IMM (ival));
>        /* C_JR and C_MV have the same opcode.  If RS2 is 0, then this is a C_JR.
>  	 So must try to match C_JR first as it ahs more bits in mask.  */
>        else if (is_c_jr_insn (ival))