Re: [Qemu-riscv] [RFC v2 23/24] WIP: Add missing instructions

[Richard Henderson]

On 11/27/18 1:10 PM, Alistair Francis wrote:
> Signed-off-by: Alistair Francis <address@hidden>
> ---
>  tcg/riscv/tcg-target.inc.c | 77 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
> 
> diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
> index 13756f6d0d..b8e9c0e126 100644
> --- a/tcg/riscv/tcg-target.inc.c
> +++ b/tcg/riscv/tcg-target.inc.c
> @@ -606,6 +606,57 @@ static bool tcg_out_sti(TCGContext *s, TCGType type, 
> TCGArg val,
>      return false;
>  }
>  
> +static void tcg_out_addsub2(TCGContext *s,
> +                            TCGReg rl, TCGReg rh,
> +                            TCGReg al, TCGReg ah,
> +                            TCGReg bl, TCGReg bh,
> +                            bool cbl, bool cbh, bool is_sub)
> +{
> +    /* FIXME: This is just copied from MIPS */
> +    TCGReg th = TCG_REG_TMP1;
> +
> +    /* If we have a negative constant such that negating it would
> +       make the high part zero, we can (usually) eliminate one insn.  */
> +    if (cbl && cbh && bh == -1 && bl != 0) {
> +        bl = -bl;
> +        bh = 0;
> +        is_sub = !is_sub;
> +    }
> +
> +    /* By operating on the high part first, we get to use the final
> +       carry operation to move back from the temporary.  */
> +    if (!cbh) {
> +        tcg_out_opc_reg(s, (is_sub ? OPC_SUB : OPC_ADDI), th, ah, bh);
> +    } else if (bh != 0 || ah == rl) {
> +        tcg_out_opc_imm(s, OPC_ADDI, th, ah, (is_sub ? -bh : bh));
> +    } else {
> +        th = ah;
> +    }
> +
> +    if (is_sub) {
> +        if (cbl) {
> +            tcg_out_opc_imm(s, OPC_SLLI, TCG_REG_TMP0, al, bl);

s/SLLI/SLTIU/

> +            tcg_out_opc_imm(s, OPC_ADDI, rl, al, -bl);
> +        } else {
> +            tcg_out_opc_reg(s, OPC_SLLI, TCG_REG_TMP0, al, bl);

s/SLLI/SLTU/

> +            tcg_out_opc_reg(s, OPC_SUB, rl, al, bl);
> +        }
> +        tcg_out_opc_reg(s, OPC_SUB, rh, th, TCG_REG_TMP0);
> +    } else {
> +        if (cbl) {
> +            tcg_out_opc_imm(s, OPC_ADDI, rl, al, bl);
> +            tcg_out_opc_imm(s, OPC_SLLI, TCG_REG_TMP0, rl, bl);

SLTIU

> +        } else if (rl == al && rl == bl) {
> +            tcg_out_opc_imm(s, OPC_SRLI, TCG_REG_TMP0, al, 31);

s/31/TCG_TARGET_REG_BITS - 1/
or somesuch.  I think that's an existing mips bug too...

> +            tcg_out_opc_reg(s, OPC_ADDI, rl, al, bl);

ADD, not ADDI.

> +        } else {
> +            tcg_out_opc_reg(s, OPC_ADDI, rl, al, bl);
> +            tcg_out_opc_reg(s, OPC_SLLI, TCG_REG_TMP0, rl, (rl == bl ? al : 
> bl));

Similarly.

> +        }
> +        tcg_out_opc_reg(s, OPC_ADDI, rh, th, TCG_REG_TMP0);

ADD.

> @@ -1537,6 +1604,8 @@ static const TCGTargetOpDef 
> *tcg_target_op_def(TCGOpcode op)
>      case INDEX_op_ext16s_i32:
>      case INDEX_op_ext16s_i64:
>      case INDEX_op_ext32s_i64:
> +    case INDEX_op_extrl_i64_i32:
> +    case INDEX_op_extrh_i64_i32:

This belongs in a previous patch.


> +    case INDEX_op_add2_i32:
> +    case INDEX_op_add2_i64:
> +        return &rZ_rZ_rZ_rZ_rZ_rZ;
> +
> +    case INDEX_op_sub2_i32:
> +    case INDEX_op_sub2_i64:
> +        return &rZ_rZ_rZ_rZ_rZ_rZ;

Adding 0 with TCG_REG_ZERO is sorta pointless.  You want the I and N
constraints.  Note that MIPS uses a modified symmetric N that can always be
negated [-32767, 32767] instead of [-32767, 32768].  Otherwise that first if
statement doesn't work.


r~