diff mbox series

[22/23] tcg/tci: Implement 64-bit division

Message ID 20210128082331.196801-23-richard.henderson@linaro.org
State New
Headers show
Series TCI fixes and cleanups | expand

Commit Message

Richard Henderson Jan. 28, 2021, 8:23 a.m. UTC 
Trivially implemented like other arithmetic.
Tested via check-tcg and the ppc64 target.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 tcg/tci/tcg-target.h     |  4 ++--
 tcg/tci.c                | 28 ++++++++++++++++++++++------
 tcg/tci/tcg-target.c.inc | 12 ++++--------
 3 files changed, 28 insertions(+), 16 deletions(-)

-- 
2.25.1

Comments

Stefan Weil Jan. 28, 2021, 10:04 a.m. UTC | #1 
Am 28.01.21 um 09:23 schrieb Richard Henderson:

> Trivially implemented like other arithmetic.

> Tested via check-tcg and the ppc64 target.

>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>   tcg/tci/tcg-target.h     |  4 ++--

>   tcg/tci.c                | 28 ++++++++++++++++++++++------

>   tcg/tci/tcg-target.c.inc | 12 ++++--------

>   3 files changed, 28 insertions(+), 16 deletions(-)

>

> diff --git a/tcg/tci/tcg-target.h b/tcg/tci/tcg-target.h

> index bb784e018e..7fc349a3de 100644

> --- a/tcg/tci/tcg-target.h

> +++ b/tcg/tci/tcg-target.h

> @@ -100,8 +100,8 @@

>   #define TCG_TARGET_HAS_extract_i64      0

>   #define TCG_TARGET_HAS_sextract_i64     0

>   #define TCG_TARGET_HAS_extract2_i64     0

> -#define TCG_TARGET_HAS_div_i64          0

> -#define TCG_TARGET_HAS_rem_i64          0

> +#define TCG_TARGET_HAS_div_i64          1

> +#define TCG_TARGET_HAS_rem_i64          1

>   #define TCG_TARGET_HAS_ext8s_i64        1

>   #define TCG_TARGET_HAS_ext16s_i64       1

>   #define TCG_TARGET_HAS_ext32s_i64       1

> diff --git a/tcg/tci.c b/tcg/tci.c

> index 32931ea611..0065c854a4 100644

> --- a/tcg/tci.c

> +++ b/tcg/tci.c

> @@ -889,14 +889,30 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,

>               t2 = tci_read_ri64(regs, &tb_ptr);

>               tci_write_reg(regs, t0, t1 * t2);

>               break;

> -#if TCG_TARGET_HAS_div_i64



I suggest to keep this and other identical #if TCG_TARGET_HAS_div_i64 
and the matching #endif in the code.

They are not needed for the default case, but useful as long as it is 
possible to write a TCG backend without those opcodes. Then it helps 
running TCI with the same subset of opcodes.

As far as I see currently s390 does not set that macro. So to compare 
the s390 TCG with TCI, I'd want to have the same subset of TCG opcodes, 
which means none of those implemented here. That can be done when the 
preprocessor conditionals remain in the code.


>           case INDEX_op_div_i64:

> -        case INDEX_op_divu_i64:

> -        case INDEX_op_rem_i64:

> -        case INDEX_op_remu_i64:

> -            TODO();

> +            t0 = *tb_ptr++;

> +            t1 = tci_read_ri64(regs, &tb_ptr);

> +            t2 = tci_read_ri64(regs, &tb_ptr);

> +            tci_write_reg(regs, t0, (int64_t)t1 / (int64_t)t2);

> +            break;

> +        case INDEX_op_divu_i64:

> +            t0 = *tb_ptr++;

> +            t1 = tci_read_ri64(regs, &tb_ptr);

> +            t2 = tci_read_ri64(regs, &tb_ptr);

> +            tci_write_reg(regs, t0, (uint64_t)t1 / (uint64_t)t2);

> +            break;

> +        case INDEX_op_rem_i64:

> +            t0 = *tb_ptr++;

> +            t1 = tci_read_ri64(regs, &tb_ptr);

> +            t2 = tci_read_ri64(regs, &tb_ptr);

> +            tci_write_reg(regs, t0, (int64_t)t1 % (int64_t)t2);

> +            break;

> +        case INDEX_op_remu_i64:

> +            t0 = *tb_ptr++;

> +            t1 = tci_read_ri64(regs, &tb_ptr);

> +            t2 = tci_read_ri64(regs, &tb_ptr);

> +            tci_write_reg(regs, t0, (uint64_t)t1 % (uint64_t)t2);

>               break;

> -#endif

>           case INDEX_op_and_i64:

>               t0 = *tb_ptr++;

>               t1 = tci_read_ri64(regs, &tb_ptr);

> diff --git a/tcg/tci/tcg-target.c.inc b/tcg/tci/tcg-target.c.inc

> index 842807ff2e..8c0e77a0be 100644

> --- a/tcg/tci/tcg-target.c.inc

> +++ b/tcg/tci/tcg-target.c.inc

> @@ -146,12 +146,10 @@ static const TCGTargetOpDef tcg_target_op_defs[] = {

>       { INDEX_op_add_i64, { R, RI, RI } },

>       { INDEX_op_sub_i64, { R, RI, RI } },

>       { INDEX_op_mul_i64, { R, RI, RI } },

> -#if TCG_TARGET_HAS_div_i64

>       { INDEX_op_div_i64, { R, R, R } },

>       { INDEX_op_divu_i64, { R, R, R } },

>       { INDEX_op_rem_i64, { R, R, R } },

>       { INDEX_op_remu_i64, { R, R, R } },

> -#endif

>       { INDEX_op_and_i64, { R, RI, RI } },

>   #if TCG_TARGET_HAS_andc_i64

>       { INDEX_op_andc_i64, { R, RI, RI } },

> @@ -678,6 +676,10 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,

>       case INDEX_op_sar_i64:

>       case INDEX_op_rotl_i64:     /* Optional (TCG_TARGET_HAS_rot_i64). */

>       case INDEX_op_rotr_i64:     /* Optional (TCG_TARGET_HAS_rot_i64). */

> +    case INDEX_op_div_i64:      /* Optional (TCG_TARGET_HAS_div_i64). */

> +    case INDEX_op_divu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */

> +    case INDEX_op_rem_i64:      /* Optional (TCG_TARGET_HAS_div_i64). */

> +    case INDEX_op_remu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */

>           tcg_out_r(s, args[0]);

>           tcg_out_ri64(s, const_args[1], args[1]);

>           tcg_out_ri64(s, const_args[2], args[2]);

> @@ -691,12 +693,6 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,

>           tcg_debug_assert(args[4] <= UINT8_MAX);

>           tcg_out8(s, args[4]);

>           break;

> -    case INDEX_op_div_i64:      /* Optional (TCG_TARGET_HAS_div_i64). */

> -    case INDEX_op_divu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */

> -    case INDEX_op_rem_i64:      /* Optional (TCG_TARGET_HAS_div_i64). */

> -    case INDEX_op_remu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */

> -        TODO();

> -        break;

>       case INDEX_op_brcond_i64:

>           tcg_out_r(s, args[0]);

>           tcg_out_ri64(s, const_args[1], args[1]);



Thanks. See my comment above where I suggest a slight modification.

Reviewed-by: Stefan Weil <sw@weilnetz.de>
Alex Bennée Jan. 28, 2021, 3:38 p.m. UTC | #2 
Richard Henderson <richard.henderson@linaro.org> writes:

> Trivially implemented like other arithmetic.

> Tested via check-tcg and the ppc64 target.

>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée
Richard Henderson Jan. 28, 2021, 5:56 p.m. UTC | #3 
On 1/28/21 12:04 AM, Stefan Weil wrote:
> Am 28.01.21 um 09:23 schrieb Richard Henderson:

> 

>> Trivially implemented like other arithmetic.

>> Tested via check-tcg and the ppc64 target.

>>

>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>> ---

>>   tcg/tci/tcg-target.h     |  4 ++--

>>   tcg/tci.c                | 28 ++++++++++++++++++++++------

>>   tcg/tci/tcg-target.c.inc | 12 ++++--------

>>   3 files changed, 28 insertions(+), 16 deletions(-)

>>

>> diff --git a/tcg/tci/tcg-target.h b/tcg/tci/tcg-target.h

>> index bb784e018e..7fc349a3de 100644

>> --- a/tcg/tci/tcg-target.h

>> +++ b/tcg/tci/tcg-target.h

>> @@ -100,8 +100,8 @@

>>   #define TCG_TARGET_HAS_extract_i64      0

>>   #define TCG_TARGET_HAS_sextract_i64     0

>>   #define TCG_TARGET_HAS_extract2_i64     0

>> -#define TCG_TARGET_HAS_div_i64          0

>> -#define TCG_TARGET_HAS_rem_i64          0

>> +#define TCG_TARGET_HAS_div_i64          1

>> +#define TCG_TARGET_HAS_rem_i64          1

>>   #define TCG_TARGET_HAS_ext8s_i64        1

>>   #define TCG_TARGET_HAS_ext16s_i64       1

>>   #define TCG_TARGET_HAS_ext32s_i64       1

>> diff --git a/tcg/tci.c b/tcg/tci.c

>> index 32931ea611..0065c854a4 100644

>> --- a/tcg/tci.c

>> +++ b/tcg/tci.c

>> @@ -889,14 +889,30 @@ uintptr_t QEMU_DISABLE_CFI

>> tcg_qemu_tb_exec(CPUArchState *env,

>>               t2 = tci_read_ri64(regs, &tb_ptr);

>>               tci_write_reg(regs, t0, t1 * t2);

>>               break;

>> -#if TCG_TARGET_HAS_div_i64

> 

> 

> I suggest to keep this and other identical #if TCG_TARGET_HAS_div_i64 and the

> matching #endif in the code.


As before, no ifdefs required.


r~
diff mbox series

Patch

diff --git a/tcg/tci/tcg-target.h b/tcg/tci/tcg-target.h
index bb784e018e..7fc349a3de 100644
--- a/tcg/tci/tcg-target.h
+++ b/tcg/tci/tcg-target.h
@@ -100,8 +100,8 @@ 
 #define TCG_TARGET_HAS_extract_i64      0
 #define TCG_TARGET_HAS_sextract_i64     0
 #define TCG_TARGET_HAS_extract2_i64     0
-#define TCG_TARGET_HAS_div_i64          0
-#define TCG_TARGET_HAS_rem_i64          0
+#define TCG_TARGET_HAS_div_i64          1
+#define TCG_TARGET_HAS_rem_i64          1
 #define TCG_TARGET_HAS_ext8s_i64        1
 #define TCG_TARGET_HAS_ext16s_i64       1
 #define TCG_TARGET_HAS_ext32s_i64       1
diff --git a/tcg/tci.c b/tcg/tci.c
index 32931ea611..0065c854a4 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -889,14 +889,30 @@  uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t2 = tci_read_ri64(regs, &tb_ptr);
             tci_write_reg(regs, t0, t1 * t2);
             break;
-#if TCG_TARGET_HAS_div_i64
         case INDEX_op_div_i64:
-        case INDEX_op_divu_i64:
-        case INDEX_op_rem_i64:
-        case INDEX_op_remu_i64:
-            TODO();
+            t0 = *tb_ptr++;
+            t1 = tci_read_ri64(regs, &tb_ptr);
+            t2 = tci_read_ri64(regs, &tb_ptr);
+            tci_write_reg(regs, t0, (int64_t)t1 / (int64_t)t2);
+            break;
+        case INDEX_op_divu_i64:
+            t0 = *tb_ptr++;
+            t1 = tci_read_ri64(regs, &tb_ptr);
+            t2 = tci_read_ri64(regs, &tb_ptr);
+            tci_write_reg(regs, t0, (uint64_t)t1 / (uint64_t)t2);
+            break;
+        case INDEX_op_rem_i64:
+            t0 = *tb_ptr++;
+            t1 = tci_read_ri64(regs, &tb_ptr);
+            t2 = tci_read_ri64(regs, &tb_ptr);
+            tci_write_reg(regs, t0, (int64_t)t1 % (int64_t)t2);
+            break;
+        case INDEX_op_remu_i64:
+            t0 = *tb_ptr++;
+            t1 = tci_read_ri64(regs, &tb_ptr);
+            t2 = tci_read_ri64(regs, &tb_ptr);
+            tci_write_reg(regs, t0, (uint64_t)t1 % (uint64_t)t2);
             break;
-#endif
         case INDEX_op_and_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(regs, &tb_ptr);
diff --git a/tcg/tci/tcg-target.c.inc b/tcg/tci/tcg-target.c.inc
index 842807ff2e..8c0e77a0be 100644
--- a/tcg/tci/tcg-target.c.inc
+++ b/tcg/tci/tcg-target.c.inc
@@ -146,12 +146,10 @@  static const TCGTargetOpDef tcg_target_op_defs[] = {
     { INDEX_op_add_i64, { R, RI, RI } },
     { INDEX_op_sub_i64, { R, RI, RI } },
     { INDEX_op_mul_i64, { R, RI, RI } },
-#if TCG_TARGET_HAS_div_i64
     { INDEX_op_div_i64, { R, R, R } },
     { INDEX_op_divu_i64, { R, R, R } },
     { INDEX_op_rem_i64, { R, R, R } },
     { INDEX_op_remu_i64, { R, R, R } },
-#endif
     { INDEX_op_and_i64, { R, RI, RI } },
 #if TCG_TARGET_HAS_andc_i64
     { INDEX_op_andc_i64, { R, RI, RI } },
@@ -678,6 +676,10 @@  static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
     case INDEX_op_sar_i64:
     case INDEX_op_rotl_i64:     /* Optional (TCG_TARGET_HAS_rot_i64). */
     case INDEX_op_rotr_i64:     /* Optional (TCG_TARGET_HAS_rot_i64). */
+    case INDEX_op_div_i64:      /* Optional (TCG_TARGET_HAS_div_i64). */
+    case INDEX_op_divu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */
+    case INDEX_op_rem_i64:      /* Optional (TCG_TARGET_HAS_div_i64). */
+    case INDEX_op_remu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */
         tcg_out_r(s, args[0]);
         tcg_out_ri64(s, const_args[1], args[1]);
         tcg_out_ri64(s, const_args[2], args[2]);
@@ -691,12 +693,6 @@  static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
         tcg_debug_assert(args[4] <= UINT8_MAX);
         tcg_out8(s, args[4]);
         break;
-    case INDEX_op_div_i64:      /* Optional (TCG_TARGET_HAS_div_i64). */
-    case INDEX_op_divu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */
-    case INDEX_op_rem_i64:      /* Optional (TCG_TARGET_HAS_div_i64). */
-    case INDEX_op_remu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */
-        TODO();
-        break;
     case INDEX_op_brcond_i64:
         tcg_out_r(s, args[0]);
         tcg_out_ri64(s, const_args[1], args[1]);