diff mbox series

[v4,110/163] tcg/riscv: Drop support for add2/sub2

Message ID 20250415192515.232910-111-richard.henderson@linaro.org
State New
Headers show
Series tcg: Convert to TCGOutOp structures | expand

Commit Message

Richard Henderson April 15, 2025, 7:24 p.m. UTC 
We now produce exactly the same code via generic expansion.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/riscv/tcg-target-con-set.h |  1 -
 tcg/riscv/tcg-target-has.h     |  6 +--
 tcg/riscv/tcg-target.c.inc     | 86 +---------------------------------
 3 files changed, 3 insertions(+), 90 deletions(-)

Comments

Pierrick Bouvier April 15, 2025, 10:05 p.m. UTC | #1 
On 4/15/25 12:24, Richard Henderson wrote:
> We now produce exactly the same code via generic expansion.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/riscv/tcg-target-con-set.h |  1 -
>   tcg/riscv/tcg-target-has.h     |  6 +--
>   tcg/riscv/tcg-target.c.inc     | 86 +---------------------------------
>   3 files changed, 3 insertions(+), 90 deletions(-)
> 
> diff --git a/tcg/riscv/tcg-target-con-set.h b/tcg/riscv/tcg-target-con-set.h
> index 5ff2c2db60..0fc26d3f98 100644
> --- a/tcg/riscv/tcg-target-con-set.h
> +++ b/tcg/riscv/tcg-target-con-set.h
> @@ -18,7 +18,6 @@ C_O1_I2(r, r, ri)
>   C_O1_I2(r, r, rI)
>   C_N1_I2(r, r, rM)
>   C_O1_I4(r, r, rI, rM, rM)
> -C_O2_I4(r, r, rz, rz, rM, rM)
>   C_O0_I2(v, r)
>   C_O1_I1(v, r)
>   C_O1_I1(v, v)
> diff --git a/tcg/riscv/tcg-target-has.h b/tcg/riscv/tcg-target-has.h
> index b2814f8ef9..c95dc1921e 100644
> --- a/tcg/riscv/tcg-target-has.h
> +++ b/tcg/riscv/tcg-target-has.h
> @@ -10,13 +10,11 @@
>   #include "host/cpuinfo.h"
>   
>   /* optional instructions */
> -#define TCG_TARGET_HAS_add2_i32         1
> -#define TCG_TARGET_HAS_sub2_i32         1
>   #define TCG_TARGET_HAS_qemu_st8_i32     0
>   
>   #define TCG_TARGET_HAS_extr_i64_i32     1
> -#define TCG_TARGET_HAS_add2_i64         1
> -#define TCG_TARGET_HAS_sub2_i64         1
> +#define TCG_TARGET_HAS_add2_i64         0
> +#define TCG_TARGET_HAS_sub2_i64         0
>   
>   #define TCG_TARGET_HAS_qemu_ldst_i128   0
>   
> diff --git a/tcg/riscv/tcg-target.c.inc b/tcg/riscv/tcg-target.c.inc
> index d74ac7587a..dce46dcba6 100644
> --- a/tcg/riscv/tcg-target.c.inc
> +++ b/tcg/riscv/tcg-target.c.inc
> @@ -401,7 +401,7 @@ static bool tcg_target_const_match(int64_t val, int ct,
>       }
>       /*
>        * Sign extended from 12 bits, +/- matching: [-0x7ff, 0x7ff].
> -     * Used by addsub2 and movcond, which may need the negative value,
> +     * Used by movcond, which may need the negative value,
>        * and requires the modified constant to be representable.
>        */
>       if ((ct & TCG_CT_CONST_M12) && val >= -0x7ff && val <= 0x7ff) {
> @@ -1073,67 +1073,6 @@ 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,
> -                            TCGArg bl, TCGArg bh,
> -                            bool cbl, bool cbh, bool is_sub, bool is32bit)
> -{
> -    const RISCVInsn opc_add = is32bit ? OPC_ADDW : OPC_ADD;
> -    const RISCVInsn opc_addi = is32bit ? OPC_ADDIW : OPC_ADDI;
> -    const RISCVInsn opc_sub = is32bit ? OPC_SUBW : OPC_SUB;
> -    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_add), 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;
> -    }
> -
> -    /* Note that tcg optimization should eliminate the bl == 0 case.  */
> -    if (is_sub) {
> -        if (cbl) {
> -            tcg_out_opc_imm(s, OPC_SLTIU, TCG_REG_TMP0, al, bl);
> -            tcg_out_opc_imm(s, opc_addi, rl, al, -bl);
> -        } else {
> -            tcg_out_opc_reg(s, OPC_SLTU, TCG_REG_TMP0, al, bl);
> -            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_SLTIU, TCG_REG_TMP0, rl, bl);
> -        } else if (al == bl) {
> -            /*
> -             * If the input regs overlap, this is a simple doubling
> -             * and carry-out is the input msb.  This special case is
> -             * required when the output reg overlaps the input,
> -             * but we might as well use it always.
> -             */
> -            tcg_out_opc_imm(s, OPC_SLTI, TCG_REG_TMP0, al, 0);
> -            tcg_out_opc_reg(s, opc_add, rl, al, al);
> -        } else {
> -            tcg_out_opc_reg(s, opc_add, rl, al, bl);
> -            tcg_out_opc_reg(s, OPC_SLTU, TCG_REG_TMP0,
> -                            rl, (rl == bl ? al : bl));
> -        }
> -        tcg_out_opc_reg(s, opc_add, rh, th, TCG_REG_TMP0);
> -    }
> -}
> -
>   static bool tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece,
>                                      TCGReg dst, TCGReg src)
>   {
> @@ -2608,23 +2547,6 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, TCGType type,
>           tcg_out_ldst(s, OPC_SD, a0, a1, a2);
>           break;
>   
> -    case INDEX_op_add2_i32:
> -        tcg_out_addsub2(s, a0, a1, a2, args[3], args[4], args[5],
> -                        const_args[4], const_args[5], false, true);
> -        break;
> -    case INDEX_op_add2_i64:
> -        tcg_out_addsub2(s, a0, a1, a2, args[3], args[4], args[5],
> -                        const_args[4], const_args[5], false, false);
> -        break;
> -    case INDEX_op_sub2_i32:
> -        tcg_out_addsub2(s, a0, a1, a2, args[3], args[4], args[5],
> -                        const_args[4], const_args[5], true, true);
> -        break;
> -    case INDEX_op_sub2_i64:
> -        tcg_out_addsub2(s, a0, a1, a2, args[3], args[4], args[5],
> -                        const_args[4], const_args[5], true, false);
> -        break;
> -
>       case INDEX_op_qemu_ld_i32:
>           tcg_out_qemu_ld(s, a0, a1, a2, TCG_TYPE_I32);
>           break;
> @@ -2897,12 +2819,6 @@ tcg_target_op_def(TCGOpcode op, TCGType type, unsigned flags)
>       case INDEX_op_st_i64:
>           return C_O0_I2(rz, r);
>   
> -    case INDEX_op_add2_i32:
> -    case INDEX_op_add2_i64:
> -    case INDEX_op_sub2_i32:
> -    case INDEX_op_sub2_i64:
> -        return C_O2_I4(r, r, rz, rz, rM, rM);
> -
>       case INDEX_op_qemu_ld_i32:
>       case INDEX_op_qemu_ld_i64:
>           return C_O1_I1(r, r);

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
diff mbox series

Patch

diff --git a/tcg/riscv/tcg-target-con-set.h b/tcg/riscv/tcg-target-con-set.h
index 5ff2c2db60..0fc26d3f98 100644
--- a/tcg/riscv/tcg-target-con-set.h
+++ b/tcg/riscv/tcg-target-con-set.h
@@ -18,7 +18,6 @@  C_O1_I2(r, r, ri)
 C_O1_I2(r, r, rI)
 C_N1_I2(r, r, rM)
 C_O1_I4(r, r, rI, rM, rM)
-C_O2_I4(r, r, rz, rz, rM, rM)
 C_O0_I2(v, r)
 C_O1_I1(v, r)
 C_O1_I1(v, v)
diff --git a/tcg/riscv/tcg-target-has.h b/tcg/riscv/tcg-target-has.h
index b2814f8ef9..c95dc1921e 100644
--- a/tcg/riscv/tcg-target-has.h
+++ b/tcg/riscv/tcg-target-has.h
@@ -10,13 +10,11 @@ 
 #include "host/cpuinfo.h"
 
 /* optional instructions */
-#define TCG_TARGET_HAS_add2_i32         1
-#define TCG_TARGET_HAS_sub2_i32         1
 #define TCG_TARGET_HAS_qemu_st8_i32     0
 
 #define TCG_TARGET_HAS_extr_i64_i32     1
-#define TCG_TARGET_HAS_add2_i64         1
-#define TCG_TARGET_HAS_sub2_i64         1
+#define TCG_TARGET_HAS_add2_i64         0
+#define TCG_TARGET_HAS_sub2_i64         0
 
 #define TCG_TARGET_HAS_qemu_ldst_i128   0
 
diff --git a/tcg/riscv/tcg-target.c.inc b/tcg/riscv/tcg-target.c.inc
index d74ac7587a..dce46dcba6 100644
--- a/tcg/riscv/tcg-target.c.inc
+++ b/tcg/riscv/tcg-target.c.inc
@@ -401,7 +401,7 @@  static bool tcg_target_const_match(int64_t val, int ct,
     }
     /*
      * Sign extended from 12 bits, +/- matching: [-0x7ff, 0x7ff].
-     * Used by addsub2 and movcond, which may need the negative value,
+     * Used by movcond, which may need the negative value,
      * and requires the modified constant to be representable.
      */
     if ((ct & TCG_CT_CONST_M12) && val >= -0x7ff && val <= 0x7ff) {
@@ -1073,67 +1073,6 @@  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,
-                            TCGArg bl, TCGArg bh,
-                            bool cbl, bool cbh, bool is_sub, bool is32bit)
-{
-    const RISCVInsn opc_add = is32bit ? OPC_ADDW : OPC_ADD;
-    const RISCVInsn opc_addi = is32bit ? OPC_ADDIW : OPC_ADDI;
-    const RISCVInsn opc_sub = is32bit ? OPC_SUBW : OPC_SUB;
-    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_add), 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;
-    }
-
-    /* Note that tcg optimization should eliminate the bl == 0 case.  */
-    if (is_sub) {
-        if (cbl) {
-            tcg_out_opc_imm(s, OPC_SLTIU, TCG_REG_TMP0, al, bl);
-            tcg_out_opc_imm(s, opc_addi, rl, al, -bl);
-        } else {
-            tcg_out_opc_reg(s, OPC_SLTU, TCG_REG_TMP0, al, bl);
-            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_SLTIU, TCG_REG_TMP0, rl, bl);
-        } else if (al == bl) {
-            /*
-             * If the input regs overlap, this is a simple doubling
-             * and carry-out is the input msb.  This special case is
-             * required when the output reg overlaps the input,
-             * but we might as well use it always.
-             */
-            tcg_out_opc_imm(s, OPC_SLTI, TCG_REG_TMP0, al, 0);
-            tcg_out_opc_reg(s, opc_add, rl, al, al);
-        } else {
-            tcg_out_opc_reg(s, opc_add, rl, al, bl);
-            tcg_out_opc_reg(s, OPC_SLTU, TCG_REG_TMP0,
-                            rl, (rl == bl ? al : bl));
-        }
-        tcg_out_opc_reg(s, opc_add, rh, th, TCG_REG_TMP0);
-    }
-}
-
 static bool tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece,
                                    TCGReg dst, TCGReg src)
 {
@@ -2608,23 +2547,6 @@  static void tcg_out_op(TCGContext *s, TCGOpcode opc, TCGType type,
         tcg_out_ldst(s, OPC_SD, a0, a1, a2);
         break;
 
-    case INDEX_op_add2_i32:
-        tcg_out_addsub2(s, a0, a1, a2, args[3], args[4], args[5],
-                        const_args[4], const_args[5], false, true);
-        break;
-    case INDEX_op_add2_i64:
-        tcg_out_addsub2(s, a0, a1, a2, args[3], args[4], args[5],
-                        const_args[4], const_args[5], false, false);
-        break;
-    case INDEX_op_sub2_i32:
-        tcg_out_addsub2(s, a0, a1, a2, args[3], args[4], args[5],
-                        const_args[4], const_args[5], true, true);
-        break;
-    case INDEX_op_sub2_i64:
-        tcg_out_addsub2(s, a0, a1, a2, args[3], args[4], args[5],
-                        const_args[4], const_args[5], true, false);
-        break;
-
     case INDEX_op_qemu_ld_i32:
         tcg_out_qemu_ld(s, a0, a1, a2, TCG_TYPE_I32);
         break;
@@ -2897,12 +2819,6 @@  tcg_target_op_def(TCGOpcode op, TCGType type, unsigned flags)
     case INDEX_op_st_i64:
         return C_O0_I2(rz, r);
 
-    case INDEX_op_add2_i32:
-    case INDEX_op_add2_i64:
-    case INDEX_op_sub2_i32:
-    case INDEX_op_sub2_i64:
-        return C_O2_I4(r, r, rz, rz, rM, rM);
-
     case INDEX_op_qemu_ld_i32:
     case INDEX_op_qemu_ld_i64:
         return C_O1_I1(r, r);