[PATCH 00/23] TCI fixes and cleanups

[PATCH 00/23] TCI fixes and cleanups

[Richard Henderson]

The first patch I believe is queued by Paolo, but is not yet
upstream; copied here for convenience.  Then, fill in all of
the TODO blanks in TCI.

The tci_write_reg* functions are redundant with tcg_write_reg.
Just pass in the properly truncated result to begin.  In the
cases of the loads, we've automatically done that with the
type of the indirection.  For all of the other arithmetic,
we don't actually have to do anything -- the value is either
right, or the high bits are undefined.  And in fact will
currently be ignored by the extension on read.


r~


Richard Henderson (21):
  configure: Fix --enable-tcg-interpreter
  tcg: Manage splitwx in tc_ptr_to_region_tree by hand
  exec: Make tci_tb_ptr thread-local
  tcg/tci: Inline tci_write_reg32s into the only caller
  tcg/tci: Inline tci_write_reg8 into its callers
  tcg/tci: Inline tci_write_reg16 into the only caller
  tcg/tci: Inline tci_write_reg32 into all callers
  tcg/tci: Inline tci_write_reg64 into 64-bit callers
  tcg/tci: Merge INDEX_op_ld8u_{i32,i64}
  tcg/tci: Merge INDEX_op_ld8s_{i32,i64}
  tcg/tci: Merge INDEX_op_ld16u_{i32,i64}
  tcg/tci: Merge INDEX_op_ld16s_{i32,i64}
  tcg/tci: Merge INDEX_op_{ld_i32,ld32u_i64}
  tcg/tci: Merge INDEX_op_st8_{i32,i64}
  tcg/tci: Merge INDEX_op_st16_{i32,i64}
  tcg/tci: Move stack bounds check to compile-time
  tcg/tci: Merge INDEX_op_{st_i32,st32_i64}
  tcg/tci: Use g_assert_not_reached
  tcg/tci: Remove dead code for TCG_TARGET_HAS_div2_*
  tcg/tci: Implement 64-bit division
  tcg/tci: Remove TODO as unused

Stefan Weil (2):
  tcg/tci: Implement INDEX_op_ld16s_i32
  tcg/tci: Implement INDEX_op_ld8s_i64

 configure                |   5 +-
 include/exec/exec-all.h  |   2 +-
 tcg/tci/tcg-target.h     |   4 +-
 tcg/tcg-common.c         |   2 +-
 tcg/tcg.c                |  23 +++-
 tcg/tci.c                | 283 +++++++++++++++------------------------
 tcg/tci/tcg-target.c.inc |  41 +++---
 7 files changed, 154 insertions(+), 206 deletions(-)

-- 
2.25.1

[PATCH 01/23] configure: Fix --enable-tcg-interpreter

[Richard Henderson]

The configure option was backward, and we failed to
pass the value on to meson.

Fixes: 23a77b2d18b
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 configure | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index dcc5ea7d63..ad35e26168 100755
--- a/configure
+++ b/configure
@@ -1119,9 +1119,9 @@ for opt do
   ;;
   --enable-whpx) whpx="enabled"
   ;;
-  --disable-tcg-interpreter) tcg_interpreter="true"
+  --disable-tcg-interpreter) tcg_interpreter="false"
   ;;
-  --enable-tcg-interpreter) tcg_interpreter="false"
+  --enable-tcg-interpreter) tcg_interpreter="true"
   ;;
   --disable-cap-ng)  cap_ng="disabled"
   ;;
@@ -6374,6 +6374,7 @@ NINJA=$ninja $meson setup \
         -Dvhost_user_blk_server=$vhost_user_blk_server \
         -Dfuse=$fuse -Dfuse_lseek=$fuse_lseek -Dguest_agent_msi=$guest_agent_msi \
         $(if test "$default_features" = no; then echo "-Dauto_features=disabled"; fi) \
+	-Dtcg_interpreter=$tcg_interpreter \
         $cross_arg \
         "$PWD" "$source_path"
 
-- 
2.25.1

Re: [PATCH 01/23] configure: Fix --enable-tcg-interpreter

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> The configure option was backward, and we failed to
> pass the value on to meson.
>
> Fixes: 23a77b2d18b
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

I was going to say I'd seen this before, and indeed I had so here are
all the tags you can add from 20210125144530.2837481-2-philmd@redhat.com:

Fixes: 23a77b2d18b ("build-system: clean up TCG/TCI configury")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tested-by: Stefan Weil <sw@weilnetz.de>


-- 
Alex Bennée

[PATCH 02/23] tcg: Manage splitwx in tc_ptr_to_region_tree by hand

[Richard Henderson]

The use in tcg_tb_lookup is given a random pc that comes from the pc
of a signal handler.  Do not assert that the pointer is already within
the code gen buffer at all, much less the writable mirror of it.

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

For TCI, this indicates a bug in handle_cpu_signal, in that we
are taking PC from the host signal frame.  Which is, nearly,
unrelated to TCI at all.

The TCI "pc" is tci_tb_ptr (fixed in the next patch to at least
be thread-local).  We update this only on calls, since we don't
expect SEGV during the interpretation loop.  Which works ok for
softmmu, in which we pass down pc by hand to the helpers, but
is not ok for user-only, where we simply perform the raw memory
operation.

I don't know how to fix this, exactly.  Probably by storing to
tci_tb_ptr before each qemu_ld/qemu_st operation, with barriers.
Then Doing the Right Thing in handle_cpu_signal.  And perhaps
by clearing tci_tb_ptr whenever we're not expecting a SEGV on
behalf of the guest (and thus anything left is a qemu host bug).


r~

---
 tcg/tcg.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 9e1b0d73c7..78701cf359 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -407,11 +407,21 @@ static void tcg_region_trees_init(void)
     }
 }
 
-static struct tcg_region_tree *tc_ptr_to_region_tree(const void *cp)
+static struct tcg_region_tree *tc_ptr_to_region_tree(const void *p)
 {
-    void *p = tcg_splitwx_to_rw(cp);
     size_t region_idx;
 
+    /*
+     * Like tcg_splitwx_to_rw, with no assert.  The pc may come from
+     * a signal handler over which the caller has no control.
+     */
+    if (!in_code_gen_buffer(p)) {
+        p -= tcg_splitwx_diff;
+        if (!in_code_gen_buffer(p)) {
+            return NULL;
+        }
+    }
+
     if (p < region.start_aligned) {
         region_idx = 0;
     } else {
@@ -430,6 +440,7 @@ void tcg_tb_insert(TranslationBlock *tb)
 {
     struct tcg_region_tree *rt = tc_ptr_to_region_tree(tb->tc.ptr);
 
+    g_assert(rt != NULL);
     qemu_mutex_lock(&rt->lock);
     g_tree_insert(rt->tree, &tb->tc, tb);
     qemu_mutex_unlock(&rt->lock);
@@ -439,6 +450,7 @@ void tcg_tb_remove(TranslationBlock *tb)
 {
     struct tcg_region_tree *rt = tc_ptr_to_region_tree(tb->tc.ptr);
 
+    g_assert(rt != NULL);
     qemu_mutex_lock(&rt->lock);
     g_tree_remove(rt->tree, &tb->tc);
     qemu_mutex_unlock(&rt->lock);
@@ -453,8 +465,13 @@ TranslationBlock *tcg_tb_lookup(uintptr_t tc_ptr)
 {
     struct tcg_region_tree *rt = tc_ptr_to_region_tree((void *)tc_ptr);
     TranslationBlock *tb;
-    struct tb_tc s = { .ptr = (void *)tc_ptr };
+    struct tb_tc s;
 
+    if (rt == NULL) {
+        return NULL;
+    }
+
+    s.ptr = (void *)tc_ptr;
     qemu_mutex_lock(&rt->lock);
     tb = g_tree_lookup(rt->tree, &s);
     qemu_mutex_unlock(&rt->lock);
-- 
2.25.1

Re: [PATCH 02/23] tcg: Manage splitwx in tc_ptr_to_region_tree by hand

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> The use in tcg_tb_lookup is given a random pc that comes from the pc
> of a signal handler.  Do not assert that the pointer is already within
> the code gen buffer at all, much less the writable mirror of it.
>
> Fixes: db0c51a3803
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

OK I bisected to this regression while running:

  "cd builds/bisect && rm -rf * && ../../configure --disable-docs --target-list=m68k-linux-user && make -j30 && make check-tcg"

which ultimately fails during the threadcount test for m68k-linux-user.
I'm just testing now to see if that also broke my ARM system test
images.

> ---
>
> For TCI, this indicates a bug in handle_cpu_signal, in that we
> are taking PC from the host signal frame.  Which is, nearly,
> unrelated to TCI at all.
>
> The TCI "pc" is tci_tb_ptr (fixed in the next patch to at least
> be thread-local).  We update this only on calls, since we don't
> expect SEGV during the interpretation loop.  Which works ok for
> softmmu, in which we pass down pc by hand to the helpers, but
> is not ok for user-only, where we simply perform the raw memory
> operation.
>
> I don't know how to fix this, exactly.  Probably by storing to
> tci_tb_ptr before each qemu_ld/qemu_st operation, with barriers.
> Then Doing the Right Thing in handle_cpu_signal.  And perhaps
> by clearing tci_tb_ptr whenever we're not expecting a SEGV on
> behalf of the guest (and thus anything left is a qemu host bug).
>
>
> r~
>
> ---
>  tcg/tcg.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 9e1b0d73c7..78701cf359 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -407,11 +407,21 @@ static void tcg_region_trees_init(void)
>      }
>  }
>  
> -static struct tcg_region_tree *tc_ptr_to_region_tree(const void *cp)
> +static struct tcg_region_tree *tc_ptr_to_region_tree(const void *p)
>  {
> -    void *p = tcg_splitwx_to_rw(cp);
>      size_t region_idx;
>  
> +    /*
> +     * Like tcg_splitwx_to_rw, with no assert.  The pc may come from
> +     * a signal handler over which the caller has no control.
> +     */
> +    if (!in_code_gen_buffer(p)) {
> +        p -= tcg_splitwx_diff;
> +        if (!in_code_gen_buffer(p)) {
> +            return NULL;
> +        }
> +    }
> +
>      if (p < region.start_aligned) {
>          region_idx = 0;
>      } else {
> @@ -430,6 +440,7 @@ void tcg_tb_insert(TranslationBlock *tb)
>  {
>      struct tcg_region_tree *rt = tc_ptr_to_region_tree(tb->tc.ptr);
>  
> +    g_assert(rt != NULL);
>      qemu_mutex_lock(&rt->lock);
>      g_tree_insert(rt->tree, &tb->tc, tb);
>      qemu_mutex_unlock(&rt->lock);
> @@ -439,6 +450,7 @@ void tcg_tb_remove(TranslationBlock *tb)
>  {
>      struct tcg_region_tree *rt = tc_ptr_to_region_tree(tb->tc.ptr);
>  
> +    g_assert(rt != NULL);
>      qemu_mutex_lock(&rt->lock);
>      g_tree_remove(rt->tree, &tb->tc);
>      qemu_mutex_unlock(&rt->lock);
> @@ -453,8 +465,13 @@ TranslationBlock *tcg_tb_lookup(uintptr_t tc_ptr)
>  {
>      struct tcg_region_tree *rt = tc_ptr_to_region_tree((void *)tc_ptr);
>      TranslationBlock *tb;
> -    struct tb_tc s = { .ptr = (void *)tc_ptr };
> +    struct tb_tc s;
>  
> +    if (rt == NULL) {
> +        return NULL;
> +    }
> +
> +    s.ptr = (void *)tc_ptr;
>      qemu_mutex_lock(&rt->lock);
>      tb = g_tree_lookup(rt->tree, &s);
>      qemu_mutex_unlock(&rt->lock);


-- 
Alex Bennée

Re: [PATCH 02/23] tcg: Manage splitwx in tc_ptr_to_region_tree by hand

[Alex Bennée]

Alex Bennée <alex.bennee@linaro.org> writes:

> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> The use in tcg_tb_lookup is given a random pc that comes from the pc
>> of a signal handler.  Do not assert that the pointer is already within
>> the code gen buffer at all, much less the writable mirror of it.
>>
>> Fixes: db0c51a3803
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
> OK I bisected to this regression while running:
>
>   "cd builds/bisect && rm -rf * && ../../configure --disable-docs --target-list=m68k-linux-user && make -j30 && make check-tcg"
>
> which ultimately fails during the threadcount test for m68k-linux-user.
> I'm just testing now to see if that also broke my ARM system test
> images.

For my ARM test system:

./qemu-system-aarch64 -machine virt,virtualization=on -cpu cortex-a57
-serial mon:stdio -nic user,model=virtio-net-pci,hostfwd=tcp::2222-:22
-device virtio-scsi-pci -drive
file=/dev/zvol/hackpool-0/debian-bullseye-arm64,id=hd0,index=0,if=none,format=raw
-device scsi-hd,drive=hd0 -display none -m 4096 -smp 4 -drive
if=pflash,file=/usr/share/AAVMF/AAVMF_CODE.fd,format=raw,readonly -drive
if=pflash,file=$HOME/images/AAVMF_VARS.fd,format=raw


Yep with this:

  [    2.948980] Checked W+X mappings: passed, no W+X pages found
  [    2.949443] Run /init as init process
  [    2.959082] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
  [    2.959563] CPU: 3 PID: 1 Comm: init Not tainted 5.10.0-1-arm64 #1 Debian 5.10.4-1
  [    2.959768] Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015
  [    2.960144] Call trace:
  [    2.960267]  dump_backtrace+0x0/0x1e4
  [    2.960491]  show_stack+0x24/0x6c
  [    2.960699]  dump_stack+0xd0/0x12c
  [    2.960862]  panic+0x168/0x370
  [    2.960990]  do_exit+0x9a8/0x9c0
  [    2.961072]  do_group_exit+0x44/0xac
  [    2.961163]  get_signal+0x174/0x910
  [    2.961256]  do_notify_resume+0x22c/0x9a0
  [    2.961355]  work_pending+0xc/0x39c
  [    2.961849] SMP: stopping secondary CPUs
  [    2.962341] Kernel Offset: disabled
  [    2.962585] CPU features: 0x0240022,61006082
  [    2.962729] Memory Limit: none
  [    2.963158] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
  QEMU 5.2.50 monitor - type 'help' for more information
  (qemu) quit

Where as I can boot from the commit before....

>
>> ---
>>
>> For TCI, this indicates a bug in handle_cpu_signal, in that we
>> are taking PC from the host signal frame.  Which is, nearly,
>> unrelated to TCI at all.
>>
>> The TCI "pc" is tci_tb_ptr (fixed in the next patch to at least
>> be thread-local).  We update this only on calls, since we don't
>> expect SEGV during the interpretation loop.  Which works ok for
>> softmmu, in which we pass down pc by hand to the helpers, but
>> is not ok for user-only, where we simply perform the raw memory
>> operation.
>>
>> I don't know how to fix this, exactly.  Probably by storing to
>> tci_tb_ptr before each qemu_ld/qemu_st operation, with barriers.
>> Then Doing the Right Thing in handle_cpu_signal.  And perhaps
>> by clearing tci_tb_ptr whenever we're not expecting a SEGV on
>> behalf of the guest (and thus anything left is a qemu host bug).
>>
>>
>> r~
>>
>> ---
>>  tcg/tcg.c | 23 ++++++++++++++++++++---
>>  1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/tcg/tcg.c b/tcg/tcg.c
>> index 9e1b0d73c7..78701cf359 100644
>> --- a/tcg/tcg.c
>> +++ b/tcg/tcg.c
>> @@ -407,11 +407,21 @@ static void tcg_region_trees_init(void)
>>      }
>>  }
>>  
>> -static struct tcg_region_tree *tc_ptr_to_region_tree(const void *cp)
>> +static struct tcg_region_tree *tc_ptr_to_region_tree(const void *p)
>>  {
>> -    void *p = tcg_splitwx_to_rw(cp);
>>      size_t region_idx;
>>  
>> +    /*
>> +     * Like tcg_splitwx_to_rw, with no assert.  The pc may come from
>> +     * a signal handler over which the caller has no control.
>> +     */
>> +    if (!in_code_gen_buffer(p)) {
>> +        p -= tcg_splitwx_diff;
>> +        if (!in_code_gen_buffer(p)) {
>> +            return NULL;
>> +        }
>> +    }
>> +
>>      if (p < region.start_aligned) {
>>          region_idx = 0;
>>      } else {
>> @@ -430,6 +440,7 @@ void tcg_tb_insert(TranslationBlock *tb)
>>  {
>>      struct tcg_region_tree *rt = tc_ptr_to_region_tree(tb->tc.ptr);
>>  
>> +    g_assert(rt != NULL);
>>      qemu_mutex_lock(&rt->lock);
>>      g_tree_insert(rt->tree, &tb->tc, tb);
>>      qemu_mutex_unlock(&rt->lock);
>> @@ -439,6 +450,7 @@ void tcg_tb_remove(TranslationBlock *tb)
>>  {
>>      struct tcg_region_tree *rt = tc_ptr_to_region_tree(tb->tc.ptr);
>>  
>> +    g_assert(rt != NULL);
>>      qemu_mutex_lock(&rt->lock);
>>      g_tree_remove(rt->tree, &tb->tc);
>>      qemu_mutex_unlock(&rt->lock);
>> @@ -453,8 +465,13 @@ TranslationBlock *tcg_tb_lookup(uintptr_t tc_ptr)
>>  {
>>      struct tcg_region_tree *rt = tc_ptr_to_region_tree((void *)tc_ptr);
>>      TranslationBlock *tb;
>> -    struct tb_tc s = { .ptr = (void *)tc_ptr };
>> +    struct tb_tc s;
>>  
>> +    if (rt == NULL) {
>> +        return NULL;
>> +    }
>> +
>> +    s.ptr = (void *)tc_ptr;
>>      qemu_mutex_lock(&rt->lock);
>>      tb = g_tree_lookup(rt->tree, &s);
>>      qemu_mutex_unlock(&rt->lock);


-- 
Alex Bennée

[PATCH 03/23] exec: Make tci_tb_ptr thread-local

[Richard Henderson]

Each thread must have its own pc, even under TCI.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/exec-all.h | 2 +-
 tcg/tcg-common.c        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 125000bcf7..f933c74c44 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -544,7 +544,7 @@ void tb_set_jmp_target(TranslationBlock *tb, int n, uintptr_t addr);
 
 /* GETPC is the true target of the return instruction that we'll execute.  */
 #if defined(CONFIG_TCG_INTERPRETER)
-extern uintptr_t tci_tb_ptr;
+extern __thread uintptr_t tci_tb_ptr;
 # define GETPC() tci_tb_ptr
 #else
 # define GETPC() \
diff --git a/tcg/tcg-common.c b/tcg/tcg-common.c
index 7e1992e79e..b183db84c7 100644
--- a/tcg/tcg-common.c
+++ b/tcg/tcg-common.c
@@ -26,7 +26,7 @@
 #include "tcg/tcg.h"
 
 #if defined(CONFIG_TCG_INTERPRETER)
-uintptr_t tci_tb_ptr;
+__thread uintptr_t tci_tb_ptr;
 #endif
 
 TCGOpDef tcg_op_defs[] = {
-- 
2.25.1

[PATCH 04/23] tcg/tci: Implement INDEX_op_ld16s_i32

[Richard Henderson]

From: Stefan Weil <sw@weilnetz.de>

That TCG opcode is used by debian-buster (arm64) running ffmpeg:

    qemu-aarch64 /usr/bin/ffmpeg -i theora.mkv theora.webm

Reported-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Message-Id: <20210128024814.2056958-1-sw@weilnetz.de>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 2311aa7d3a..2edb47506e 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -614,7 +614,10 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             TODO();
             break;
         case INDEX_op_ld16s_i32:
-            TODO();
+            t0 = *tb_ptr++;
+            t1 = tci_read_r(regs, &tb_ptr);
+            t2 = tci_read_s32(&tb_ptr);
+            tci_write_reg(regs, t0, *(int16_t *)(t1 + t2));
             break;
         case INDEX_op_ld_i32:
             t0 = *tb_ptr++;
-- 
2.25.1

Re: [PATCH 04/23] tcg/tci: Implement INDEX_op_ld16s_i32

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> From: Stefan Weil <sw@weilnetz.de>
>
> That TCG opcode is used by debian-buster (arm64) running ffmpeg:
>
>     qemu-aarch64 /usr/bin/ffmpeg -i theora.mkv theora.webm
>
> Reported-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> Message-Id: <20210128024814.2056958-1-sw@weilnetz.de>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

> ---
>  tcg/tci.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tcg/tci.c b/tcg/tci.c
> index 2311aa7d3a..2edb47506e 100644
> --- a/tcg/tci.c
> +++ b/tcg/tci.c
> @@ -614,7 +614,10 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
>              TODO();
>              break;
>          case INDEX_op_ld16s_i32:
> -            TODO();
> +            t0 = *tb_ptr++;
> +            t1 = tci_read_r(regs, &tb_ptr);
> +            t2 = tci_read_s32(&tb_ptr);
> +            tci_write_reg(regs, t0, *(int16_t *)(t1 + t2));
>              break;
>          case INDEX_op_ld_i32:
>              t0 = *tb_ptr++;


-- 
Alex Bennée

[PATCH 05/23] tcg/tci: Implement INDEX_op_ld8s_i64

[Richard Henderson]

From: Stefan Weil <sw@weilnetz.de>

That TCG opcode is used by debian-buster (arm64) running ffmpeg:

    qemu-aarch64 /usr/bin/ffmpeg -i theora.mkv theora.webm

Reported-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Message-Id: <20210128020425.2055454-1-sw@weilnetz.de>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 2edb47506e..0e1b8e8383 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -882,7 +882,10 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             tci_write_reg8(regs, t0, *(uint8_t *)(t1 + t2));
             break;
         case INDEX_op_ld8s_i64:
-            TODO();
+            t0 = *tb_ptr++;
+            t1 = tci_read_r(regs, &tb_ptr);
+            t2 = tci_read_s32(&tb_ptr);
+            tci_write_reg(regs, t0, *(int8_t *)(t1 + t2));
             break;
         case INDEX_op_ld16u_i64:
             t0 = *tb_ptr++;
-- 
2.25.1

Re: [PATCH 05/23] tcg/tci: Implement INDEX_op_ld8s_i64

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> From: Stefan Weil <sw@weilnetz.de>
>
> That TCG opcode is used by debian-buster (arm64) running ffmpeg:
>
>     qemu-aarch64 /usr/bin/ffmpeg -i theora.mkv theora.webm
>
> Reported-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> Message-Id: <20210128020425.2055454-1-sw@weilnetz.de>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

> ---
>  tcg/tci.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tcg/tci.c b/tcg/tci.c
> index 2edb47506e..0e1b8e8383 100644
> --- a/tcg/tci.c
> +++ b/tcg/tci.c
> @@ -882,7 +882,10 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
>              tci_write_reg8(regs, t0, *(uint8_t *)(t1 + t2));
>              break;
>          case INDEX_op_ld8s_i64:
> -            TODO();
> +            t0 = *tb_ptr++;
> +            t1 = tci_read_r(regs, &tb_ptr);
> +            t2 = tci_read_s32(&tb_ptr);
> +            tci_write_reg(regs, t0, *(int8_t *)(t1 + t2));
>              break;
>          case INDEX_op_ld16u_i64:
>              t0 = *tb_ptr++;


-- 
Alex Bennée

[PATCH 06/23] tcg/tci: Inline tci_write_reg32s into the only caller

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 0e1b8e8383..438d712ea8 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -115,14 +115,6 @@ tci_write_reg(tcg_target_ulong *regs, TCGReg index, tcg_target_ulong value)
     regs[index] = value;
 }
 
-#if TCG_TARGET_REG_BITS == 64
-static void
-tci_write_reg32s(tcg_target_ulong *regs, TCGReg index, int32_t value)
-{
-    tci_write_reg(regs, index, value);
-}
-#endif
-
 static void tci_write_reg8(tcg_target_ulong *regs, TCGReg index, uint8_t value)
 {
     tci_write_reg(regs, index, value);
@@ -906,7 +898,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
-            tci_write_reg32s(regs, t0, *(int32_t *)(t1 + t2));
+            tci_write_reg(regs, t0, *(int32_t *)(t1 + t2));
             break;
         case INDEX_op_ld_i64:
             t0 = *tb_ptr++;
-- 
2.25.1

Re: [PATCH 06/23] tcg/tci: Inline tci_write_reg32s into the only caller

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

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

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

-- 
Alex Bennée

[PATCH 07/23] tcg/tci: Inline tci_write_reg8 into its callers

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 438d712ea8..7797558b2a 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -115,11 +115,6 @@ tci_write_reg(tcg_target_ulong *regs, TCGReg index, tcg_target_ulong value)
     regs[index] = value;
 }
 
-static void tci_write_reg8(tcg_target_ulong *regs, TCGReg index, uint8_t value)
-{
-    tci_write_reg(regs, index, value);
-}
-
 static void
 tci_write_reg16(tcg_target_ulong *regs, TCGReg index, uint16_t value)
 {
@@ -597,7 +592,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
-            tci_write_reg8(regs, t0, *(uint8_t *)(t1 + t2));
+            tci_write_reg(regs, t0, *(uint8_t *)(t1 + t2));
             break;
         case INDEX_op_ld8s_i32:
             TODO();
@@ -871,7 +866,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
-            tci_write_reg8(regs, t0, *(uint8_t *)(t1 + t2));
+            tci_write_reg(regs, t0, *(uint8_t *)(t1 + t2));
             break;
         case INDEX_op_ld8s_i64:
             t0 = *tb_ptr++;
-- 
2.25.1

Re: [PATCH 07/23] tcg/tci: Inline tci_write_reg8 into its callers

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

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

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

-- 
Alex Bennée

[PATCH 08/23] tcg/tci: Inline tci_write_reg16 into the only caller

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 7797558b2a..0b27f26cfb 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -115,12 +115,6 @@ tci_write_reg(tcg_target_ulong *regs, TCGReg index, tcg_target_ulong value)
     regs[index] = value;
 }
 
-static void
-tci_write_reg16(tcg_target_ulong *regs, TCGReg index, uint16_t value)
-{
-    tci_write_reg(regs, index, value);
-}
-
 static void
 tci_write_reg32(tcg_target_ulong *regs, TCGReg index, uint32_t value)
 {
@@ -878,7 +872,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
-            tci_write_reg16(regs, t0, *(uint16_t *)(t1 + t2));
+            tci_write_reg(regs, t0, *(uint16_t *)(t1 + t2));
             break;
         case INDEX_op_ld16s_i64:
             TODO();
-- 
2.25.1

Re: [PATCH 08/23] tcg/tci: Inline tci_write_reg16 into the only caller

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

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

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

-- 
Alex Bennée

[PATCH 09/23] tcg/tci: Inline tci_write_reg32 into all callers

[Richard Henderson]

For a 64-bit TCI, the upper bits of a 32-bit operation are
undefined (much like a native ppc64 32-bit operation).  It
simplifies everything if we don't force-extend the result.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 66 +++++++++++++++++++++++++------------------------------
 1 file changed, 30 insertions(+), 36 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 0b27f26cfb..f75971dd5e 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -115,12 +115,6 @@ tci_write_reg(tcg_target_ulong *regs, TCGReg index, tcg_target_ulong value)
     regs[index] = value;
 }
 
-static void
-tci_write_reg32(tcg_target_ulong *regs, TCGReg index, uint32_t value)
-{
-    tci_write_reg(regs, index, value);
-}
-
 #if TCG_TARGET_REG_BITS == 32
 static void tci_write_reg64(tcg_target_ulong *regs, uint32_t high_index,
                             uint32_t low_index, uint64_t value)
@@ -550,7 +544,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t1 = tci_read_r32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
             condition = *tb_ptr++;
-            tci_write_reg32(regs, t0, tci_compare32(t1, t2, condition));
+            tci_write_reg(regs, t0, tci_compare32(t1, t2, condition));
             break;
 #if TCG_TARGET_REG_BITS == 32
         case INDEX_op_setcond2_i32:
@@ -558,7 +552,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             tmp64 = tci_read_r64(regs, &tb_ptr);
             v64 = tci_read_ri64(regs, &tb_ptr);
             condition = *tb_ptr++;
-            tci_write_reg32(regs, t0, tci_compare64(tmp64, v64, condition));
+            tci_write_reg(regs, t0, tci_compare64(tmp64, v64, condition));
             break;
 #elif TCG_TARGET_REG_BITS == 64
         case INDEX_op_setcond_i64:
@@ -572,12 +566,12 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
         case INDEX_op_mov_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_r32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
         case INDEX_op_tci_movi_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_i32(&tb_ptr);
-            tci_write_reg32(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
 
             /* Load/store operations (32 bit). */
@@ -604,7 +598,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
-            tci_write_reg32(regs, t0, *(uint32_t *)(t1 + t2));
+            tci_write_reg(regs, t0, *(uint32_t *)(t1 + t2));
             break;
         case INDEX_op_st8_i32:
             t0 = tci_read_r8(regs, &tb_ptr);
@@ -632,44 +626,44 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1 + t2);
+            tci_write_reg(regs, t0, t1 + t2);
             break;
         case INDEX_op_sub_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1 - t2);
+            tci_write_reg(regs, t0, t1 - t2);
             break;
         case INDEX_op_mul_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1 * t2);
+            tci_write_reg(regs, t0, t1 * t2);
             break;
 #if TCG_TARGET_HAS_div_i32
         case INDEX_op_div_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, (int32_t)t1 / (int32_t)t2);
+            tci_write_reg(regs, t0, (int32_t)t1 / (int32_t)t2);
             break;
         case INDEX_op_divu_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1 / t2);
+            tci_write_reg(regs, t0, t1 / t2);
             break;
         case INDEX_op_rem_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, (int32_t)t1 % (int32_t)t2);
+            tci_write_reg(regs, t0, (int32_t)t1 % (int32_t)t2);
             break;
         case INDEX_op_remu_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1 % t2);
+            tci_write_reg(regs, t0, t1 % t2);
             break;
 #elif TCG_TARGET_HAS_div2_i32
         case INDEX_op_div2_i32:
@@ -681,19 +675,19 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1 & t2);
+            tci_write_reg(regs, t0, t1 & t2);
             break;
         case INDEX_op_or_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1 | t2);
+            tci_write_reg(regs, t0, t1 | t2);
             break;
         case INDEX_op_xor_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1 ^ t2);
+            tci_write_reg(regs, t0, t1 ^ t2);
             break;
 
             /* Shift/rotate operations (32 bit). */
@@ -702,32 +696,32 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1 << (t2 & 31));
+            tci_write_reg(regs, t0, t1 << (t2 & 31));
             break;
         case INDEX_op_shr_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1 >> (t2 & 31));
+            tci_write_reg(regs, t0, t1 >> (t2 & 31));
             break;
         case INDEX_op_sar_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, ((int32_t)t1 >> (t2 & 31)));
+            tci_write_reg(regs, t0, ((int32_t)t1 >> (t2 & 31)));
             break;
 #if TCG_TARGET_HAS_rot_i32
         case INDEX_op_rotl_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, rol32(t1, t2 & 31));
+            tci_write_reg(regs, t0, rol32(t1, t2 & 31));
             break;
         case INDEX_op_rotr_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
             t2 = tci_read_ri32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, ror32(t1, t2 & 31));
+            tci_write_reg(regs, t0, ror32(t1, t2 & 31));
             break;
 #endif
 #if TCG_TARGET_HAS_deposit_i32
@@ -738,7 +732,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             tmp16 = *tb_ptr++;
             tmp8 = *tb_ptr++;
             tmp32 = (((1 << tmp8) - 1) << tmp16);
-            tci_write_reg32(regs, t0, (t1 & ~tmp32) | ((t2 << tmp16) & tmp32));
+            tci_write_reg(regs, t0, (t1 & ~tmp32) | ((t2 << tmp16) & tmp32));
             break;
 #endif
         case INDEX_op_brcond_i32:
@@ -790,56 +784,56 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
         case INDEX_op_ext8s_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_r8s(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
 #endif
 #if TCG_TARGET_HAS_ext16s_i32
         case INDEX_op_ext16s_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_r16s(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
 #endif
 #if TCG_TARGET_HAS_ext8u_i32
         case INDEX_op_ext8u_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_r8(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
 #endif
 #if TCG_TARGET_HAS_ext16u_i32
         case INDEX_op_ext16u_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_r16(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
 #endif
 #if TCG_TARGET_HAS_bswap16_i32
         case INDEX_op_bswap16_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_r16(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, bswap16(t1));
+            tci_write_reg(regs, t0, bswap16(t1));
             break;
 #endif
 #if TCG_TARGET_HAS_bswap32_i32
         case INDEX_op_bswap32_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_r32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, bswap32(t1));
+            tci_write_reg(regs, t0, bswap32(t1));
             break;
 #endif
 #if TCG_TARGET_HAS_not_i32
         case INDEX_op_not_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_r32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, ~t1);
+            tci_write_reg(regs, t0, ~t1);
             break;
 #endif
 #if TCG_TARGET_HAS_neg_i32
         case INDEX_op_neg_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_r32(regs, &tb_ptr);
-            tci_write_reg32(regs, t0, -t1);
+            tci_write_reg(regs, t0, -t1);
             break;
 #endif
 #if TCG_TARGET_REG_BITS == 64
@@ -881,7 +875,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
-            tci_write_reg32(regs, t0, *(uint32_t *)(t1 + t2));
+            tci_write_reg(regs, t0, *(uint32_t *)(t1 + t2));
             break;
         case INDEX_op_ld32s_i64:
             t0 = *tb_ptr++;
-- 
2.25.1

Re: [PATCH 09/23] tcg/tci: Inline tci_write_reg32 into all callers

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> For a 64-bit TCI, the upper bits of a 32-bit operation are
> undefined (much like a native ppc64 32-bit operation).  It
> simplifies everything if we don't force-extend the result.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

-- 
Alex Bennée

[PATCH 10/23] tcg/tci: Inline tci_write_reg64 into 64-bit callers

[Richard Henderson]

Note that we had two functions of the same name: a 32-bit version
which took two register numbers and a 64-bit version which was a
no-op wrapper for tcg_write_reg.  After this, we are left with
only the 32-bit version.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 60 +++++++++++++++++++++++++------------------------------
 1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index f75971dd5e..864771d91b 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -122,12 +122,6 @@ static void tci_write_reg64(tcg_target_ulong *regs, uint32_t high_index,
     tci_write_reg(regs, low_index, value);
     tci_write_reg(regs, high_index, value >> 32);
 }
-#elif TCG_TARGET_REG_BITS == 64
-static void
-tci_write_reg64(tcg_target_ulong *regs, TCGReg index, uint64_t value)
-{
-    tci_write_reg(regs, index, value);
-}
 #endif
 
 #if TCG_TARGET_REG_BITS == 32
@@ -560,7 +554,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t1 = tci_read_r64(regs, &tb_ptr);
             t2 = tci_read_ri64(regs, &tb_ptr);
             condition = *tb_ptr++;
-            tci_write_reg64(regs, t0, tci_compare64(t1, t2, condition));
+            tci_write_reg(regs, t0, tci_compare64(t1, t2, condition));
             break;
 #endif
         case INDEX_op_mov_i32:
@@ -840,12 +834,12 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
         case INDEX_op_mov_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
         case INDEX_op_tci_movi_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_i64(&tb_ptr);
-            tci_write_reg64(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
 
             /* Load/store operations (64 bit). */
@@ -887,7 +881,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
-            tci_write_reg64(regs, t0, *(uint64_t *)(t1 + t2));
+            tci_write_reg(regs, t0, *(uint64_t *)(t1 + t2));
             break;
         case INDEX_op_st8_i64:
             t0 = tci_read_r8(regs, &tb_ptr);
@@ -921,19 +915,19 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(regs, &tb_ptr);
             t2 = tci_read_ri64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1 + t2);
+            tci_write_reg(regs, t0, t1 + t2);
             break;
         case INDEX_op_sub_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(regs, &tb_ptr);
             t2 = tci_read_ri64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1 - t2);
+            tci_write_reg(regs, t0, t1 - t2);
             break;
         case INDEX_op_mul_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(regs, &tb_ptr);
             t2 = tci_read_ri64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1 * t2);
+            tci_write_reg(regs, t0, t1 * t2);
             break;
 #if TCG_TARGET_HAS_div_i64
         case INDEX_op_div_i64:
@@ -952,19 +946,19 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(regs, &tb_ptr);
             t2 = tci_read_ri64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1 & t2);
+            tci_write_reg(regs, t0, t1 & t2);
             break;
         case INDEX_op_or_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(regs, &tb_ptr);
             t2 = tci_read_ri64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1 | t2);
+            tci_write_reg(regs, t0, t1 | t2);
             break;
         case INDEX_op_xor_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(regs, &tb_ptr);
             t2 = tci_read_ri64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1 ^ t2);
+            tci_write_reg(regs, t0, t1 ^ t2);
             break;
 
             /* Shift/rotate operations (64 bit). */
@@ -973,32 +967,32 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(regs, &tb_ptr);
             t2 = tci_read_ri64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1 << (t2 & 63));
+            tci_write_reg(regs, t0, t1 << (t2 & 63));
             break;
         case INDEX_op_shr_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(regs, &tb_ptr);
             t2 = tci_read_ri64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1 >> (t2 & 63));
+            tci_write_reg(regs, t0, t1 >> (t2 & 63));
             break;
         case INDEX_op_sar_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(regs, &tb_ptr);
             t2 = tci_read_ri64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, ((int64_t)t1 >> (t2 & 63)));
+            tci_write_reg(regs, t0, ((int64_t)t1 >> (t2 & 63)));
             break;
 #if TCG_TARGET_HAS_rot_i64
         case INDEX_op_rotl_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(regs, &tb_ptr);
             t2 = tci_read_ri64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, rol64(t1, t2 & 63));
+            tci_write_reg(regs, t0, rol64(t1, t2 & 63));
             break;
         case INDEX_op_rotr_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(regs, &tb_ptr);
             t2 = tci_read_ri64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, ror64(t1, t2 & 63));
+            tci_write_reg(regs, t0, ror64(t1, t2 & 63));
             break;
 #endif
 #if TCG_TARGET_HAS_deposit_i64
@@ -1009,7 +1003,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             tmp16 = *tb_ptr++;
             tmp8 = *tb_ptr++;
             tmp64 = (((1ULL << tmp8) - 1) << tmp16);
-            tci_write_reg64(regs, t0, (t1 & ~tmp64) | ((t2 << tmp16) & tmp64));
+            tci_write_reg(regs, t0, (t1 & ~tmp64) | ((t2 << tmp16) & tmp64));
             break;
 #endif
         case INDEX_op_brcond_i64:
@@ -1027,28 +1021,28 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
         case INDEX_op_ext8u_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r8(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
 #endif
 #if TCG_TARGET_HAS_ext8s_i64
         case INDEX_op_ext8s_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r8s(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
 #endif
 #if TCG_TARGET_HAS_ext16s_i64
         case INDEX_op_ext16s_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r16s(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
 #endif
 #if TCG_TARGET_HAS_ext16u_i64
         case INDEX_op_ext16u_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r16(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
 #endif
 #if TCG_TARGET_HAS_ext32s_i64
@@ -1057,7 +1051,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
         case INDEX_op_ext_i32_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r32s(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
 #if TCG_TARGET_HAS_ext32u_i64
         case INDEX_op_ext32u_i64:
@@ -1065,41 +1059,41 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
         case INDEX_op_extu_i32_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r32(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, t1);
+            tci_write_reg(regs, t0, t1);
             break;
 #if TCG_TARGET_HAS_bswap16_i64
         case INDEX_op_bswap16_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r16(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, bswap16(t1));
+            tci_write_reg(regs, t0, bswap16(t1));
             break;
 #endif
 #if TCG_TARGET_HAS_bswap32_i64
         case INDEX_op_bswap32_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r32(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, bswap32(t1));
+            tci_write_reg(regs, t0, bswap32(t1));
             break;
 #endif
 #if TCG_TARGET_HAS_bswap64_i64
         case INDEX_op_bswap64_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, bswap64(t1));
+            tci_write_reg(regs, t0, bswap64(t1));
             break;
 #endif
 #if TCG_TARGET_HAS_not_i64
         case INDEX_op_not_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, ~t1);
+            tci_write_reg(regs, t0, ~t1);
             break;
 #endif
 #if TCG_TARGET_HAS_neg_i64
         case INDEX_op_neg_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r64(regs, &tb_ptr);
-            tci_write_reg64(regs, t0, -t1);
+            tci_write_reg(regs, t0, -t1);
             break;
 #endif
 #endif /* TCG_TARGET_REG_BITS == 64 */
-- 
2.25.1

Re: [PATCH 10/23] tcg/tci: Inline tci_write_reg64 into 64-bit callers

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Note that we had two functions of the same name: a 32-bit version
> which took two register numbers and a 64-bit version which was a
> no-op wrapper for tcg_write_reg.  After this, we are left with
> only the 32-bit version.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

-- 
Alex Bennée

[PATCH 11/23] tcg/tci: Merge INDEX_op_ld8u_{i32,i64}

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 864771d91b..019035d52f 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -571,6 +571,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             /* Load/store operations (32 bit). */
 
         case INDEX_op_ld8u_i32:
+        case INDEX_op_ld8u_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
@@ -844,12 +845,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
 
             /* Load/store operations (64 bit). */
 
-        case INDEX_op_ld8u_i64:
-            t0 = *tb_ptr++;
-            t1 = tci_read_r(regs, &tb_ptr);
-            t2 = tci_read_s32(&tb_ptr);
-            tci_write_reg(regs, t0, *(uint8_t *)(t1 + t2));
-            break;
         case INDEX_op_ld8s_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
-- 
2.25.1

Re: [PATCH 11/23] tcg/tci: Merge INDEX_op_ld8u_{i32,i64}

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

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

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

-- 
Alex Bennée

[PATCH 12/23] tcg/tci: Merge INDEX_op_ld8s_{i32,i64}

[Richard Henderson]

Eliminating a TODO for ld8s_i32.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 019035d52f..7d11982eb2 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -578,7 +578,11 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             tci_write_reg(regs, t0, *(uint8_t *)(t1 + t2));
             break;
         case INDEX_op_ld8s_i32:
-            TODO();
+        case INDEX_op_ld8s_i64:
+            t0 = *tb_ptr++;
+            t1 = tci_read_r(regs, &tb_ptr);
+            t2 = tci_read_s32(&tb_ptr);
+            tci_write_reg(regs, t0, *(int8_t *)(t1 + t2));
             break;
         case INDEX_op_ld16u_i32:
             TODO();
@@ -845,12 +849,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
 
             /* Load/store operations (64 bit). */
 
-        case INDEX_op_ld8s_i64:
-            t0 = *tb_ptr++;
-            t1 = tci_read_r(regs, &tb_ptr);
-            t2 = tci_read_s32(&tb_ptr);
-            tci_write_reg(regs, t0, *(int8_t *)(t1 + t2));
-            break;
         case INDEX_op_ld16u_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
-- 
2.25.1

Re: [PATCH 12/23] tcg/tci: Merge INDEX_op_ld8s_{i32,i64}

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Eliminating a TODO for ld8s_i32.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

-- 
Alex Bennée

[PATCH 13/23] tcg/tci: Merge INDEX_op_ld16u_{i32,i64}

[Richard Henderson]

Eliminating a TODO for ld16u_i32.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 7d11982eb2..d197803dca 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -585,7 +585,11 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             tci_write_reg(regs, t0, *(int8_t *)(t1 + t2));
             break;
         case INDEX_op_ld16u_i32:
-            TODO();
+        case INDEX_op_ld16u_i64:
+            t0 = *tb_ptr++;
+            t1 = tci_read_r(regs, &tb_ptr);
+            t2 = tci_read_s32(&tb_ptr);
+            tci_write_reg(regs, t0, *(uint16_t *)(t1 + t2));
             break;
         case INDEX_op_ld16s_i32:
             t0 = *tb_ptr++;
@@ -849,12 +853,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
 
             /* Load/store operations (64 bit). */
 
-        case INDEX_op_ld16u_i64:
-            t0 = *tb_ptr++;
-            t1 = tci_read_r(regs, &tb_ptr);
-            t2 = tci_read_s32(&tb_ptr);
-            tci_write_reg(regs, t0, *(uint16_t *)(t1 + t2));
-            break;
         case INDEX_op_ld16s_i64:
             TODO();
             break;
-- 
2.25.1

Re: [PATCH 13/23] tcg/tci: Merge INDEX_op_ld16u_{i32,i64}

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Eliminating a TODO for ld16u_i32.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

-- 
Alex Bennée

[PATCH 14/23] tcg/tci: Merge INDEX_op_ld16s_{i32,i64}

[Richard Henderson]

Eliminating a TODO for ld16s_i64.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index d197803dca..95625701bb 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -592,6 +592,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             tci_write_reg(regs, t0, *(uint16_t *)(t1 + t2));
             break;
         case INDEX_op_ld16s_i32:
+        case INDEX_op_ld16s_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
@@ -853,9 +854,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
 
             /* Load/store operations (64 bit). */
 
-        case INDEX_op_ld16s_i64:
-            TODO();
-            break;
         case INDEX_op_ld32u_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
-- 
2.25.1

Re: [PATCH 14/23] tcg/tci: Merge INDEX_op_ld16s_{i32,i64}

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Eliminating a TODO for ld16s_i64.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

-- 
Alex Bennée

[PATCH 15/23] tcg/tci: Merge INDEX_op_{ld_i32,ld32u_i64}

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 95625701bb..233fc0604e 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -599,6 +599,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             tci_write_reg(regs, t0, *(int16_t *)(t1 + t2));
             break;
         case INDEX_op_ld_i32:
+        case INDEX_op_ld32u_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
@@ -854,12 +855,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
 
             /* Load/store operations (64 bit). */
 
-        case INDEX_op_ld32u_i64:
-            t0 = *tb_ptr++;
-            t1 = tci_read_r(regs, &tb_ptr);
-            t2 = tci_read_s32(&tb_ptr);
-            tci_write_reg(regs, t0, *(uint32_t *)(t1 + t2));
-            break;
         case INDEX_op_ld32s_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_r(regs, &tb_ptr);
-- 
2.25.1

Re: [PATCH 15/23] tcg/tci: Merge INDEX_op_{ld_i32,ld32u_i64}

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

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

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

-- 
Alex Bennée

[PATCH 16/23] tcg/tci: Merge INDEX_op_st8_{i32,i64}

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 233fc0604e..0978a5c554 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -606,6 +606,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             tci_write_reg(regs, t0, *(uint32_t *)(t1 + t2));
             break;
         case INDEX_op_st8_i32:
+        case INDEX_op_st8_i64:
             t0 = tci_read_r8(regs, &tb_ptr);
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
@@ -867,12 +868,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t2 = tci_read_s32(&tb_ptr);
             tci_write_reg(regs, t0, *(uint64_t *)(t1 + t2));
             break;
-        case INDEX_op_st8_i64:
-            t0 = tci_read_r8(regs, &tb_ptr);
-            t1 = tci_read_r(regs, &tb_ptr);
-            t2 = tci_read_s32(&tb_ptr);
-            *(uint8_t *)(t1 + t2) = t0;
-            break;
         case INDEX_op_st16_i64:
             t0 = tci_read_r16(regs, &tb_ptr);
             t1 = tci_read_r(regs, &tb_ptr);
-- 
2.25.1

Re: [PATCH 16/23] tcg/tci: Merge INDEX_op_st8_{i32,i64}

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

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

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

-- 
Alex Bennée

[PATCH 17/23] tcg/tci: Merge INDEX_op_st16_{i32,i64}

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 0978a5c554..67875636a5 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -613,6 +613,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             *(uint8_t *)(t1 + t2) = t0;
             break;
         case INDEX_op_st16_i32:
+        case INDEX_op_st16_i64:
             t0 = tci_read_r16(regs, &tb_ptr);
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
@@ -868,12 +869,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t2 = tci_read_s32(&tb_ptr);
             tci_write_reg(regs, t0, *(uint64_t *)(t1 + t2));
             break;
-        case INDEX_op_st16_i64:
-            t0 = tci_read_r16(regs, &tb_ptr);
-            t1 = tci_read_r(regs, &tb_ptr);
-            t2 = tci_read_s32(&tb_ptr);
-            *(uint16_t *)(t1 + t2) = t0;
-            break;
         case INDEX_op_st32_i64:
             t0 = tci_read_r32(regs, &tb_ptr);
             t1 = tci_read_r(regs, &tb_ptr);
-- 
2.25.1

Re: [PATCH 17/23] tcg/tci: Merge INDEX_op_st16_{i32,i64}

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

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

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

-- 
Alex Bennée

[PATCH 18/23] tcg/tci: Move stack bounds check to compile-time

[Richard Henderson]

The existing check was incomplete:
(1) Only applied to two of the 7 stores, and not to the loads at all.
(2) Only checked the upper, but not the lower bound of the stack.

Doing this at compile time means that we don't need to do it
at runtime as well.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c                |  2 --
 tcg/tci/tcg-target.c.inc | 13 +++++++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 67875636a5..c4c303f874 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -623,7 +623,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = tci_read_r32(regs, &tb_ptr);
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
-            tci_assert(t1 != sp_value || (int32_t)t2 < 0);
             *(uint32_t *)(t1 + t2) = t0;
             break;
 
@@ -879,7 +878,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t0 = tci_read_r64(regs, &tb_ptr);
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
-            tci_assert(t1 != sp_value || (int32_t)t2 < 0);
             *(uint64_t *)(t1 + t2) = t0;
             break;
 
diff --git a/tcg/tci/tcg-target.c.inc b/tcg/tci/tcg-target.c.inc
index 15981265db..a60fa524a4 100644
--- a/tcg/tci/tcg-target.c.inc
+++ b/tcg/tci/tcg-target.c.inc
@@ -484,10 +484,20 @@ static void tci_out_label(TCGContext *s, TCGLabel *label)
     }
 }
 
+static void stack_bounds_check(TCGReg base, target_long offset)
+{
+    if (base == TCG_REG_CALL_STACK) {
+        tcg_debug_assert(offset < 0);
+        tcg_debug_assert(offset >= -(CPU_TEMP_BUF_NLONGS * sizeof(long)));
+    }
+}
+
 static void tcg_out_ld(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg1,
                        intptr_t arg2)
 {
     uint8_t *old_code_ptr = s->code_ptr;
+
+    stack_bounds_check(arg1, arg2);
     if (type == TCG_TYPE_I32) {
         tcg_out_op_t(s, INDEX_op_ld_i32);
         tcg_out_r(s, ret);
@@ -623,6 +633,7 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
     case INDEX_op_st16_i64:
     case INDEX_op_st32_i64:
     case INDEX_op_st_i64:
+        stack_bounds_check(args[1], args[2]);
         tcg_out_r(s, args[0]);
         tcg_out_r(s, args[1]);
         tcg_debug_assert(args[2] == (int32_t)args[2]);
@@ -825,6 +836,8 @@ static void tcg_out_st(TCGContext *s, TCGType type, TCGReg arg, TCGReg arg1,
                        intptr_t arg2)
 {
     uint8_t *old_code_ptr = s->code_ptr;
+
+    stack_bounds_check(arg1, arg2);
     if (type == TCG_TYPE_I32) {
         tcg_out_op_t(s, INDEX_op_st_i32);
         tcg_out_r(s, arg);
-- 
2.25.1

Re: [PATCH 18/23] tcg/tci: Move stack bounds check to compile-time

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> The existing check was incomplete:
> (1) Only applied to two of the 7 stores, and not to the loads at all.
> (2) Only checked the upper, but not the lower bound of the stack.
>
> Doing this at compile time means that we don't need to do it
> at runtime as well.

You confused me with the statement compile time until I realised which
compile time it was ;-)

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

-- 
Alex Bennée

[PATCH 19/23] tcg/tci: Merge INDEX_op_{st_i32,st32_i64}

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index c4c303f874..66b90f8489 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -620,6 +620,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             *(uint16_t *)(t1 + t2) = t0;
             break;
         case INDEX_op_st_i32:
+        case INDEX_op_st32_i64:
             t0 = tci_read_r32(regs, &tb_ptr);
             t1 = tci_read_r(regs, &tb_ptr);
             t2 = tci_read_s32(&tb_ptr);
@@ -868,12 +869,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t2 = tci_read_s32(&tb_ptr);
             tci_write_reg(regs, t0, *(uint64_t *)(t1 + t2));
             break;
-        case INDEX_op_st32_i64:
-            t0 = tci_read_r32(regs, &tb_ptr);
-            t1 = tci_read_r(regs, &tb_ptr);
-            t2 = tci_read_s32(&tb_ptr);
-            *(uint32_t *)(t1 + t2) = t0;
-            break;
         case INDEX_op_st_i64:
             t0 = tci_read_r64(regs, &tb_ptr);
             t1 = tci_read_r(regs, &tb_ptr);
-- 
2.25.1

Re: [PATCH 19/23] tcg/tci: Merge INDEX_op_{st_i32,st32_i64}

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

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

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

-- 
Alex Bennée

[PATCH 20/23] tcg/tci: Use g_assert_not_reached

[Richard Henderson]

Three TODO instances are never happen cases.
Other uses of tcg_abort are also indicating unreachable cases.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 66b90f8489..2ce67a8fd3 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -360,7 +360,7 @@ static bool tci_compare32(uint32_t u0, uint32_t u1, TCGCond condition)
         result = (u0 > u1);
         break;
     default:
-        TODO();
+        g_assert_not_reached();
     }
     return result;
 }
@@ -402,7 +402,7 @@ static bool tci_compare64(uint64_t u0, uint64_t u1, TCGCond condition)
         result = (u0 > u1);
         break;
     default:
-        TODO();
+        g_assert_not_reached();
     }
     return result;
 }
@@ -1109,7 +1109,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
                 tmp32 = qemu_ld_beul;
                 break;
             default:
-                tcg_abort();
+                g_assert_not_reached();
             }
             tci_write_reg(regs, t0, tmp32);
             break;
@@ -1158,7 +1158,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
                 tmp64 = qemu_ld_beq;
                 break;
             default:
-                tcg_abort();
+                g_assert_not_reached();
             }
             tci_write_reg(regs, t0, tmp64);
             if (TCG_TARGET_REG_BITS == 32) {
@@ -1186,7 +1186,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
                 qemu_st_bel(t0);
                 break;
             default:
-                tcg_abort();
+                g_assert_not_reached();
             }
             break;
         case INDEX_op_qemu_st_i64:
@@ -1216,7 +1216,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
                 qemu_st_beq(tmp64);
                 break;
             default:
-                tcg_abort();
+                g_assert_not_reached();
             }
             break;
         case INDEX_op_mb:
@@ -1224,8 +1224,7 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             smp_mb();
             break;
         default:
-            TODO();
-            break;
+            g_assert_not_reached();
         }
         tci_assert(tb_ptr == old_code_ptr + op_size);
     }
-- 
2.25.1

Re: [PATCH 20/23] tcg/tci: Use g_assert_not_reached

[Stefan Weil]

Am 28.01.21 um 09:23 schrieb Richard Henderson:

> Three TODO instances are never happen cases.
> Other uses of tcg_abort are also indicating unreachable cases.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/tci.c | 15 +++++++--------
>   1 file changed, 7 insertions(+), 8 deletions(-)
>

Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [PATCH 20/23] tcg/tci: Use g_assert_not_reached

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Three TODO instances are never happen cases.
> Other uses of tcg_abort are also indicating unreachable cases.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

-- 
Alex Bennée

[PATCH 21/23] tcg/tci: Remove dead code for TCG_TARGET_HAS_div2_*

[Richard Henderson]

We do not simultaneously support div and div2 -- it's one
or the other.  TCI is already using div, so remove div2.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c                | 12 ------------
 tcg/tci/tcg-target.c.inc | 16 ----------------
 2 files changed, 28 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 2ce67a8fd3..32931ea611 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -647,7 +647,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t2 = tci_read_ri32(regs, &tb_ptr);
             tci_write_reg(regs, t0, t1 * t2);
             break;
-#if TCG_TARGET_HAS_div_i32
         case INDEX_op_div_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
@@ -672,12 +671,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
             t2 = tci_read_ri32(regs, &tb_ptr);
             tci_write_reg(regs, t0, t1 % t2);
             break;
-#elif TCG_TARGET_HAS_div2_i32
-        case INDEX_op_div2_i32:
-        case INDEX_op_divu2_i32:
-            TODO();
-            break;
-#endif
         case INDEX_op_and_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(regs, &tb_ptr);
@@ -903,11 +896,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
         case INDEX_op_remu_i64:
             TODO();
             break;
-#elif TCG_TARGET_HAS_div2_i64
-        case INDEX_op_div2_i64:
-        case INDEX_op_divu2_i64:
-            TODO();
-            break;
 #endif
         case INDEX_op_and_i64:
             t0 = *tb_ptr++;
diff --git a/tcg/tci/tcg-target.c.inc b/tcg/tci/tcg-target.c.inc
index a60fa524a4..842807ff2e 100644
--- a/tcg/tci/tcg-target.c.inc
+++ b/tcg/tci/tcg-target.c.inc
@@ -71,15 +71,10 @@ static const TCGTargetOpDef tcg_target_op_defs[] = {
     { INDEX_op_add_i32, { R, RI, RI } },
     { INDEX_op_sub_i32, { R, RI, RI } },
     { INDEX_op_mul_i32, { R, RI, RI } },
-#if TCG_TARGET_HAS_div_i32
     { INDEX_op_div_i32, { R, R, R } },
     { INDEX_op_divu_i32, { R, R, R } },
     { INDEX_op_rem_i32, { R, R, R } },
     { INDEX_op_remu_i32, { R, R, R } },
-#elif TCG_TARGET_HAS_div2_i32
-    { INDEX_op_div2_i32, { R, R, "0", "1", R } },
-    { INDEX_op_divu2_i32, { R, R, "0", "1", R } },
-#endif
     /* TODO: Does R, RI, RI result in faster code than R, R, RI?
        If both operands are constants, we can optimize. */
     { INDEX_op_and_i32, { R, RI, RI } },
@@ -156,9 +151,6 @@ static const TCGTargetOpDef tcg_target_op_defs[] = {
     { INDEX_op_divu_i64, { R, R, R } },
     { INDEX_op_rem_i64, { R, R, R } },
     { INDEX_op_remu_i64, { R, R, R } },
-#elif TCG_TARGET_HAS_div2_i64
-    { INDEX_op_div2_i64, { R, R, "0", "1", R } },
-    { INDEX_op_divu2_i64, { R, R, "0", "1", R } },
 #endif
     { INDEX_op_and_i64, { R, RI, RI } },
 #if TCG_TARGET_HAS_andc_i64
@@ -705,10 +697,6 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
     case INDEX_op_remu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */
         TODO();
         break;
-    case INDEX_op_div2_i64:     /* Optional (TCG_TARGET_HAS_div2_i64). */
-    case INDEX_op_divu2_i64:    /* Optional (TCG_TARGET_HAS_div2_i64). */
-        TODO();
-        break;
     case INDEX_op_brcond_i64:
         tcg_out_r(s, args[0]);
         tcg_out_ri64(s, const_args[1], args[1]);
@@ -748,10 +736,6 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
         tcg_out_ri32(s, const_args[1], args[1]);
         tcg_out_ri32(s, const_args[2], args[2]);
         break;
-    case INDEX_op_div2_i32:     /* Optional (TCG_TARGET_HAS_div2_i32). */
-    case INDEX_op_divu2_i32:    /* Optional (TCG_TARGET_HAS_div2_i32). */
-        TODO();
-        break;
 #if TCG_TARGET_REG_BITS == 32
     case INDEX_op_add2_i32:
     case INDEX_op_sub2_i32:
-- 
2.25.1

Re: [PATCH 21/23] tcg/tci: Remove dead code for TCG_TARGET_HAS_div2_*

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> We do not simultaneously support div and div2 -- it's one
> or the other.  TCI is already using div, so remove div2.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

-- 
Alex Bennée

Re: [PATCH 21/23] tcg/tci: Remove dead code for TCG_TARGET_HAS_div2_*

[Stefan Weil]

Am 28.01.21 um 09:23 schrieb Richard Henderson:

> We do not simultaneously support div and div2 -- it's one
> or the other.  TCI is already using div, so remove div2.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/tci.c                | 12 ------------
>   tcg/tci/tcg-target.c.inc | 16 ----------------
>   2 files changed, 28 deletions(-)
>
> diff --git a/tcg/tci.c b/tcg/tci.c
> index 2ce67a8fd3..32931ea611 100644
> --- a/tcg/tci.c
> +++ b/tcg/tci.c
> @@ -647,7 +647,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
>               t2 = tci_read_ri32(regs, &tb_ptr);
>               tci_write_reg(regs, t0, t1 * t2);
>               break;
> -#if TCG_TARGET_HAS_div_i32
>           case INDEX_op_div_i32:
>               t0 = *tb_ptr++;
>               t1 = tci_read_ri32(regs, &tb_ptr);
> @@ -672,12 +671,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
>               t2 = tci_read_ri32(regs, &tb_ptr);
>               tci_write_reg(regs, t0, t1 % t2);
>               break;
> -#elif TCG_TARGET_HAS_div2_i32
> -        case INDEX_op_div2_i32:
> -        case INDEX_op_divu2_i32:
> -            TODO();
> -            break;
> -#endif
>           case INDEX_op_and_i32:
>               t0 = *tb_ptr++;
>               t1 = tci_read_ri32(regs, &tb_ptr);
> @@ -903,11 +896,6 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
>           case INDEX_op_remu_i64:
>               TODO();
>               break;
> -#elif TCG_TARGET_HAS_div2_i64
> -        case INDEX_op_div2_i64:
> -        case INDEX_op_divu2_i64:
> -            TODO();
> -            break;
>   #endif
>           case INDEX_op_and_i64:
>               t0 = *tb_ptr++;
> diff --git a/tcg/tci/tcg-target.c.inc b/tcg/tci/tcg-target.c.inc
> index a60fa524a4..842807ff2e 100644
> --- a/tcg/tci/tcg-target.c.inc
> +++ b/tcg/tci/tcg-target.c.inc
> @@ -71,15 +71,10 @@ static const TCGTargetOpDef tcg_target_op_defs[] = {
>       { INDEX_op_add_i32, { R, RI, RI } },
>       { INDEX_op_sub_i32, { R, RI, RI } },
>       { INDEX_op_mul_i32, { R, RI, RI } },
> -#if TCG_TARGET_HAS_div_i32
>       { INDEX_op_div_i32, { R, R, R } },
>       { INDEX_op_divu_i32, { R, R, R } },
>       { INDEX_op_rem_i32, { R, R, R } },
>       { INDEX_op_remu_i32, { R, R, R } },
> -#elif TCG_TARGET_HAS_div2_i32
> -    { INDEX_op_div2_i32, { R, R, "0", "1", R } },
> -    { INDEX_op_divu2_i32, { R, R, "0", "1", R } },
> -#endif
>       /* TODO: Does R, RI, RI result in faster code than R, R, RI?
>          If both operands are constants, we can optimize. */
>       { INDEX_op_and_i32, { R, RI, RI } },
> @@ -156,9 +151,6 @@ static const TCGTargetOpDef tcg_target_op_defs[] = {
>       { INDEX_op_divu_i64, { R, R, R } },
>       { INDEX_op_rem_i64, { R, R, R } },
>       { INDEX_op_remu_i64, { R, R, R } },
> -#elif TCG_TARGET_HAS_div2_i64
> -    { INDEX_op_div2_i64, { R, R, "0", "1", R } },
> -    { INDEX_op_divu2_i64, { R, R, "0", "1", R } },
>   #endif
>       { INDEX_op_and_i64, { R, RI, RI } },
>   #if TCG_TARGET_HAS_andc_i64
> @@ -705,10 +697,6 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
>       case INDEX_op_remu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */
>           TODO();
>           break;
> -    case INDEX_op_div2_i64:     /* Optional (TCG_TARGET_HAS_div2_i64). */
> -    case INDEX_op_divu2_i64:    /* Optional (TCG_TARGET_HAS_div2_i64). */
> -        TODO();
> -        break;
>       case INDEX_op_brcond_i64:
>           tcg_out_r(s, args[0]);
>           tcg_out_ri64(s, const_args[1], args[1]);
> @@ -748,10 +736,6 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
>           tcg_out_ri32(s, const_args[1], args[1]);
>           tcg_out_ri32(s, const_args[2], args[2]);
>           break;
> -    case INDEX_op_div2_i32:     /* Optional (TCG_TARGET_HAS_div2_i32). */
> -    case INDEX_op_divu2_i32:    /* Optional (TCG_TARGET_HAS_div2_i32). */
> -        TODO();
> -        break;
>   #if TCG_TARGET_REG_BITS == 32
>       case INDEX_op_add2_i32:
>       case INDEX_op_sub2_i32:


One of the ideas for TCI is that it should ideally support any subset of 
TCG opcodes which is used by an existing TCG backend or a newly written 
backend.

This only requires copying the TCG_TARGET_HAS... defines.

So this patch should keep the preprocessor conditionals, and the TODO 
statements have to be replaced by code (or #error for the moment).

Patch 22 should also keep the preprocessor conditionals, see my comment 
there.

Thanks,

Stefan

Re: [PATCH 21/23] tcg/tci: Remove dead code for TCG_TARGET_HAS_div2_*

[Richard Henderson]

On 1/28/21 5:39 AM, Stefan Weil wrote:
>> -    case INDEX_op_div2_i32:     /* Optional (TCG_TARGET_HAS_div2_i32). */
>> -    case INDEX_op_divu2_i32:    /* Optional (TCG_TARGET_HAS_div2_i32). */
>> -        TODO();
>> -        break;
>>   #if TCG_TARGET_REG_BITS == 32
>>       case INDEX_op_add2_i32:
>>       case INDEX_op_sub2_i32:
> 
> 
> One of the ideas for TCI is that it should ideally support any subset of TCG
> opcodes which is used by an existing TCG backend or a newly written backend.
> 
> This only requires copying the TCG_TARGET_HAS... defines.
> 
> So this patch should keep the preprocessor conditionals, and the TODO
> statements have to be replaced by code (or #error for the moment).


If that's what you are after, easily configure different setups, you still
don't need the ifdefs.  Just change the TCG_TARGET_HAS_* definition in the
header file.  The ifdefs are not necessary.  They will be unreachable code if
you zero out the HAS, but they'll still compile.

As for div2, there is no code for it in tci.c and therefore there should be no
code for it in tcg-target.c.inc.  Those bits of code are a pair and must to be
added together.


r~

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

[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
         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]);
-- 
2.25.1

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

[Stefan Weil]

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>

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

[Richard Henderson]

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~

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

[Alex Bennée]

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

[PATCH 23/23] tcg/tci: Remove TODO as unused

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tci.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 0065c854a4..efc0ca20a6 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -33,14 +33,6 @@
 #include "tcg/tcg-op.h"
 #include "qemu/compiler.h"
 
-/* Marker for missing code. */
-#define TODO() \
-    do { \
-        fprintf(stderr, "TODO %s:%u: %s()\n", \
-                __FILE__, __LINE__, __func__); \
-        tcg_abort(); \
-    } while (0)
-
 #if MAX_OPC_PARAM_IARGS != 6
 # error Fix needed, number of supported input arguments changed!
 #endif
-- 
2.25.1

Re: [PATCH 23/23] tcg/tci: Remove TODO as unused

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

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

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

-- 
Alex Bennée

Re: [PATCH 00/23] TCI fixes and cleanups

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> The first patch I believe is queued by Paolo, but is not yet
> upstream; copied here for convenience.  Then, fill in all of
> the TODO blanks in TCI.
>
> The tci_write_reg* functions are redundant with tcg_write_reg.
> Just pass in the properly truncated result to begin.  In the
> cases of the loads, we've automatically done that with the
> type of the indirection.  For all of the other arithmetic,
> we don't actually have to do anything -- the value is either
> right, or the high bits are undefined.  And in fact will
> currently be ignored by the extension on read.

FWIW aside from the regression with 2/23 I've been testing this in TCI
mode all day on hackbox. So far ffmpeg has encoded a whole 153 frames of
video which is the furthest it has ever got in one of my tests ;-)

So for the TCI part itself:

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


>
>
> r~
>
>
> Richard Henderson (21):
>   configure: Fix --enable-tcg-interpreter
>   tcg: Manage splitwx in tc_ptr_to_region_tree by hand
>   exec: Make tci_tb_ptr thread-local
>   tcg/tci: Inline tci_write_reg32s into the only caller
>   tcg/tci: Inline tci_write_reg8 into its callers
>   tcg/tci: Inline tci_write_reg16 into the only caller
>   tcg/tci: Inline tci_write_reg32 into all callers
>   tcg/tci: Inline tci_write_reg64 into 64-bit callers
>   tcg/tci: Merge INDEX_op_ld8u_{i32,i64}
>   tcg/tci: Merge INDEX_op_ld8s_{i32,i64}
>   tcg/tci: Merge INDEX_op_ld16u_{i32,i64}
>   tcg/tci: Merge INDEX_op_ld16s_{i32,i64}
>   tcg/tci: Merge INDEX_op_{ld_i32,ld32u_i64}
>   tcg/tci: Merge INDEX_op_st8_{i32,i64}
>   tcg/tci: Merge INDEX_op_st16_{i32,i64}
>   tcg/tci: Move stack bounds check to compile-time
>   tcg/tci: Merge INDEX_op_{st_i32,st32_i64}
>   tcg/tci: Use g_assert_not_reached
>   tcg/tci: Remove dead code for TCG_TARGET_HAS_div2_*
>   tcg/tci: Implement 64-bit division
>   tcg/tci: Remove TODO as unused
>
> Stefan Weil (2):
>   tcg/tci: Implement INDEX_op_ld16s_i32
>   tcg/tci: Implement INDEX_op_ld8s_i64
>
>  configure                |   5 +-
>  include/exec/exec-all.h  |   2 +-
>  tcg/tci/tcg-target.h     |   4 +-
>  tcg/tcg-common.c         |   2 +-
>  tcg/tcg.c                |  23 +++-
>  tcg/tci.c                | 283 +++++++++++++++------------------------
>  tcg/tci/tcg-target.c.inc |  41 +++---
>  7 files changed, 154 insertions(+), 206 deletions(-)


-- 
Alex Bennée

Re: [PATCH 00/23] TCI fixes and cleanups

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> The first patch I believe is queued by Paolo, but is not yet
> upstream; copied here for convenience.  Then, fill in all of
> the TODO blanks in TCI.
>
> The tci_write_reg* functions are redundant with tcg_write_reg.
> Just pass in the properly truncated result to begin.  In the
> cases of the loads, we've automatically done that with the
> type of the indirection.  For all of the other arithmetic,
> we don't actually have to do anything -- the value is either
> right, or the high bits are undefined.  And in fact will
> currently be ignored by the extension on read.

Hmm it also failed CI pretty hard:

  https://gitlab.com/stsquad/qemu/-/pipelines/248074046/failures

-- 
Alex Bennée