[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-riscv] [PATCH v3 24/35] target/riscv: Move gen_arith_imm() dec
From: |
Richard Henderson |
Subject: |
Re: [Qemu-riscv] [PATCH v3 24/35] target/riscv: Move gen_arith_imm() decoding into trans_* functions |
Date: |
Wed, 31 Oct 2018 22:18:44 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 |
On 10/31/18 1:20 PM, Bastian Koppelmann wrote:
> static bool trans_slli(DisasContext *ctx, arg_slli *a)
> {
> - gen_arith_imm(ctx, OPC_RISC_SLLI, a->rd, a->rs1, a->shamt);
> + if (a->rd != 0) {
> + TCGv t = tcg_temp_new();
> + gen_get_gpr(t, a->rs1);
> +
> + if (a->shamt >= TARGET_LONG_BITS) {
> + return false;
> + }
> + tcg_gen_shli_tl(t, t, a->shamt);
> +
> + gen_set_gpr(a->rd, t);
> + tcg_temp_free(t);
> + } /* NOP otherwise */
> return true;
> }
>
> static bool trans_srli(DisasContext *ctx, arg_srli *a)
> {
> - gen_arith_imm(ctx, OPC_RISC_SHIFT_RIGHT_I, a->rd, a->rs1, a->shamt);
> + if (a->rd != 0) {
> + TCGv t = tcg_temp_new();
> + gen_get_gpr(t, a->rs1);
> + tcg_gen_shri_tl(t, t, a->shamt);
> + gen_set_gpr(a->rd, t);
> + tcg_temp_free(t);
> + } /* NOP otherwise */
> return true;
> }
>
> static bool trans_srai(DisasContext *ctx, arg_srai *a)
> {
> - gen_arith_imm(ctx, OPC_RISC_SHIFT_RIGHT_I, a->rd, a->rs1, a->shamt |
> 0x400);
> + if (a->rd != 0) {
> + TCGv t = tcg_temp_new();
> + gen_get_gpr(t, a->rs1);
> + tcg_gen_sari_tl(t, t, a->shamt);
> + gen_set_gpr(a->rd, t);
> + tcg_temp_free(t);
> + } /* NOP otherwise */
> return true;
> }
Surely the shri and sari functions need the same shamt >= TARGET_LONG_BITS
check as slli. Otherwise RV32 shri should definitely produce an assert in
tcg_gen_shri_tl.
I did wonder about changing the decode of the shift functions such that only
the top two bits of the imm are reserved for secondary parsing of the shift
type, and the other 10 bits are passed down into trans_foo. At which point the
TARGET_LONG_BITS check takes care of other illegalities.
Which means that the parsing for slli and slliw are identical, and also that
for the far future when RV128 is a thing, we don't have to change the parsing.
r~
- [Qemu-riscv] [PATCH v3 25/35] target/riscv: make ADD/SUB/OR/XOR/AND insn use arg lists, (continued)
- [Qemu-riscv] [PATCH v3 25/35] target/riscv: make ADD/SUB/OR/XOR/AND insn use arg lists, Bastian Koppelmann, 2018/10/31
- [Qemu-riscv] [PATCH v3 21/35] target/riscv: Remove manual decoding from gen_branch(), Bastian Koppelmann, 2018/10/31
- [Qemu-riscv] [PATCH v3 33/35] target/riscv: Splice fsw_sd and flw_ld for riscv32 vs riscv64, Bastian Koppelmann, 2018/10/31
- [Qemu-riscv] [PATCH v3 23/35] target/riscv: Remove manual decoding from gen_store(), Bastian Koppelmann, 2018/10/31
- [Qemu-riscv] [PATCH v3 24/35] target/riscv: Move gen_arith_imm() decoding into trans_* functions, Bastian Koppelmann, 2018/10/31
- [Qemu-riscv] [PATCH v3 34/35] target/riscv: Splice remaining compressed insn pairs for riscv32 vs riscv64, Bastian Koppelmann, 2018/10/31
- [Qemu-riscv] [PATCH v3 11/35] target/riscv: Convert RV64A insns to decodetree, Bastian Koppelmann, 2018/10/31
- [Qemu-riscv] [PATCH v3 18/35] target/riscv: Convert quadrant 1 of RVXC insns to decodetree, Bastian Koppelmann, 2018/10/31
- [Qemu-riscv] [PATCH v3 01/35] target/riscv: Move CPURISCVState pointer to DisasContext, Bastian Koppelmann, 2018/10/31
- [Qemu-riscv] [PATCH v3 07/35] target/riscv: Convert RVXI fence insns to decodetree, Bastian Koppelmann, 2018/10/31