[Qemu-devel] [PATCH 0/2] tcg constant pool fixups

[Qemu-devel] [PATCH 0/2] tcg constant pool fixups

[Richard Henderson]

As noted by Laurent in private mail, foo-linux-user is currently
broken on aarch64 host in unususal circumstances.  By inspection,
the same thing could happen for armv6 host.  My testing is always
on armv7 hardware, so I wouldn't have seen this.

With that fixed, the s390x code can be simplified a bit to take
advantage of this.

If the s390x maintainers could test this, I would appreciate it. 
The test case is

  ./i386-linux-user/qemu-i386 -B 0x111231000 ./some-static-i386

e.g. using the binary in the busyboxes tarball linked from

  https://wiki.qemu.org/Testing#User_mode_emulation

although any staticly linked binary would work.


r~


Richard Henderson (2):
  tcg: Allow constant pool entries in the prologue
  tcg/s390x: Use constant pool for prologue

 tcg/s390/tcg-target.inc.c | 44 ++++++++++++------------------------------
 tcg/tcg.c                 | 49 ++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 54 insertions(+), 39 deletions(-)

-- 
2.13.6

[Qemu-devel] [PATCH 1/2] tcg: Allow constant pool entries in the prologue

[Richard Henderson]

Both ARMv6 and AArch64 currently may drop complex guest_base values
into the constant pool.  But generic code wasn't expecting that, and
the pool is not emitted.  Correct that.

Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tcg.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 7 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 683ff4abb7..c22f1c4441 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -771,12 +771,32 @@ void tcg_prologue_init(TCGContext *s)
 
     /* Put the prologue at the beginning of code_gen_buffer.  */
     buf0 = s->code_gen_buffer;
+    total_size = s->code_gen_buffer_size;
     s->code_ptr = buf0;
     s->code_buf = buf0;
+    s->data_gen_ptr = NULL;
     s->code_gen_prologue = buf0;
 
+    /* Compute a high-water mark, at which we voluntarily flush the buffer
+       and start over.  The size here is arbitrary, significantly larger
+       than we expect the code generation for any one opcode to require.  */
+    s->code_gen_highwater = s->code_gen_buffer + (total_size - TCG_HIGHWATER);
+
+#ifdef TCG_TARGET_NEED_POOL_LABELS
+    s->pool_labels = NULL;
+#endif
+
     /* Generate the prologue.  */
     tcg_target_qemu_prologue(s);
+
+#ifdef TCG_TARGET_NEED_POOL_LABELS
+    /* Allow the prologue to put e.g. guest_base into a pool entry.  */
+    {
+        bool ok = tcg_out_pool_finalize(s);
+        tcg_debug_assert(ok);
+    }
+#endif
+
     buf1 = s->code_ptr;
     flush_icache_range((uintptr_t)buf0, (uintptr_t)buf1);
 
@@ -785,21 +805,36 @@ void tcg_prologue_init(TCGContext *s)
     s->code_gen_ptr = buf1;
     s->code_gen_buffer = buf1;
     s->code_buf = buf1;
-    total_size = s->code_gen_buffer_size - prologue_size;
+    total_size -= prologue_size;
     s->code_gen_buffer_size = total_size;
 
-    /* Compute a high-water mark, at which we voluntarily flush the buffer
-       and start over.  The size here is arbitrary, significantly larger
-       than we expect the code generation for any one opcode to require.  */
-    s->code_gen_highwater = s->code_gen_buffer + (total_size - TCG_HIGHWATER);
-
     tcg_register_jit(s->code_gen_buffer, total_size);
 
 #ifdef DEBUG_DISAS
     if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM)) {
         qemu_log_lock();
         qemu_log("PROLOGUE: [size=%zu]\n", prologue_size);
-        log_disas(buf0, prologue_size);
+        if (s->data_gen_ptr) {
+            size_t code_size = s->data_gen_ptr - buf0;
+            size_t data_size = prologue_size - code_size;
+            size_t i;
+
+            log_disas(buf0, code_size);
+
+            for (i = 0; i < data_size; i += sizeof(tcg_target_ulong)) {
+                if (sizeof(tcg_target_ulong) == 8) {
+                    qemu_log("0x%08" PRIxPTR ":  .quad  0x%016" PRIx64 "\n",
+                             (uintptr_t)s->data_gen_ptr + i,
+                             *(uint64_t *)(s->data_gen_ptr + i));
+                } else {
+                    qemu_log("0x%08" PRIxPTR ":  .long  0x%08x\n",
+                             (uintptr_t)s->data_gen_ptr + i,
+                             *(uint32_t *)(s->data_gen_ptr + i));
+                }
+            }
+        } else {
+            log_disas(buf0, prologue_size);
+        }
         qemu_log("\n");
         qemu_log_flush();
         qemu_log_unlock();
-- 
2.13.6

Re: [Qemu-devel] [PATCH 1/2] tcg: Allow constant pool entries in the prologue

[Emilio G. Cota]

On Thu, Oct 26, 2017 at 17:27:03 +0200, Richard Henderson wrote:
> Both ARMv6 and AArch64 currently may drop complex guest_base values
> into the constant pool.  But generic code wasn't expecting that, and
> the pool is not emitted.  Correct that.
> 
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Tested-by: Emilio G. Cota <cota@braap.org>

on an aarch64 host.

		Emilio

Re: [Qemu-devel] [PATCH 1/2] tcg: Allow constant pool entries in the prologue

[Laurent Desnogues]

Hello,

On Thu, Oct 26, 2017 at 5:27 PM, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Both ARMv6 and AArch64 currently may drop complex guest_base values
> into the constant pool.  But generic code wasn't expecting that, and
> the pool is not emitted.  Correct that.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com>

on an AArch64 host and with running qemu on qemu.

Thanks for taking care of the issue,

Laurent

> ---
>  tcg/tcg.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 42 insertions(+), 7 deletions(-)
>
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 683ff4abb7..c22f1c4441 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -771,12 +771,32 @@ void tcg_prologue_init(TCGContext *s)
>
>      /* Put the prologue at the beginning of code_gen_buffer.  */
>      buf0 = s->code_gen_buffer;
> +    total_size = s->code_gen_buffer_size;
>      s->code_ptr = buf0;
>      s->code_buf = buf0;
> +    s->data_gen_ptr = NULL;
>      s->code_gen_prologue = buf0;
>
> +    /* Compute a high-water mark, at which we voluntarily flush the buffer
> +       and start over.  The size here is arbitrary, significantly larger
> +       than we expect the code generation for any one opcode to require.  */
> +    s->code_gen_highwater = s->code_gen_buffer + (total_size - TCG_HIGHWATER);
> +
> +#ifdef TCG_TARGET_NEED_POOL_LABELS
> +    s->pool_labels = NULL;
> +#endif
> +
>      /* Generate the prologue.  */
>      tcg_target_qemu_prologue(s);
> +
> +#ifdef TCG_TARGET_NEED_POOL_LABELS
> +    /* Allow the prologue to put e.g. guest_base into a pool entry.  */
> +    {
> +        bool ok = tcg_out_pool_finalize(s);
> +        tcg_debug_assert(ok);
> +    }
> +#endif
> +
>      buf1 = s->code_ptr;
>      flush_icache_range((uintptr_t)buf0, (uintptr_t)buf1);
>
> @@ -785,21 +805,36 @@ void tcg_prologue_init(TCGContext *s)
>      s->code_gen_ptr = buf1;
>      s->code_gen_buffer = buf1;
>      s->code_buf = buf1;
> -    total_size = s->code_gen_buffer_size - prologue_size;
> +    total_size -= prologue_size;
>      s->code_gen_buffer_size = total_size;
>
> -    /* Compute a high-water mark, at which we voluntarily flush the buffer
> -       and start over.  The size here is arbitrary, significantly larger
> -       than we expect the code generation for any one opcode to require.  */
> -    s->code_gen_highwater = s->code_gen_buffer + (total_size - TCG_HIGHWATER);
> -
>      tcg_register_jit(s->code_gen_buffer, total_size);
>
>  #ifdef DEBUG_DISAS
>      if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM)) {
>          qemu_log_lock();
>          qemu_log("PROLOGUE: [size=%zu]\n", prologue_size);
> -        log_disas(buf0, prologue_size);
> +        if (s->data_gen_ptr) {
> +            size_t code_size = s->data_gen_ptr - buf0;
> +            size_t data_size = prologue_size - code_size;
> +            size_t i;
> +
> +            log_disas(buf0, code_size);
> +
> +            for (i = 0; i < data_size; i += sizeof(tcg_target_ulong)) {
> +                if (sizeof(tcg_target_ulong) == 8) {
> +                    qemu_log("0x%08" PRIxPTR ":  .quad  0x%016" PRIx64 "\n",
> +                             (uintptr_t)s->data_gen_ptr + i,
> +                             *(uint64_t *)(s->data_gen_ptr + i));
> +                } else {
> +                    qemu_log("0x%08" PRIxPTR ":  .long  0x%08x\n",
> +                             (uintptr_t)s->data_gen_ptr + i,
> +                             *(uint32_t *)(s->data_gen_ptr + i));
> +                }
> +            }
> +        } else {
> +            log_disas(buf0, prologue_size);
> +        }
>          qemu_log("\n");
>          qemu_log_flush();
>          qemu_log_unlock();
> --
> 2.13.6
>

[Qemu-devel] [PATCH 2/2] tcg/s390x: Use constant pool for prologue

[Richard Henderson]

Rather than have separate code only used for guest_base,
rely on a recent change to handle constant pool entries.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/s390/tcg-target.inc.c | 44 ++++++++++++--------------------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

diff --git a/tcg/s390/tcg-target.inc.c b/tcg/s390/tcg-target.inc.c
index 38a7cdab75..9af6dcef05 100644
--- a/tcg/s390/tcg-target.inc.c
+++ b/tcg/s390/tcg-target.inc.c
@@ -555,9 +555,6 @@ static void tcg_out_mov(TCGContext *s, TCGType type, TCGReg dst, TCGReg src)
 static const S390Opcode lli_insns[4] = {
     RI_LLILL, RI_LLILH, RI_LLIHL, RI_LLIHH
 };
-static const S390Opcode ii_insns[4] = {
-    RI_IILL, RI_IILH, RI_IIHL, RI_IIHH
-};
 
 static bool maybe_out_small_movi(TCGContext *s, TCGType type,
                                  TCGReg ret, tcg_target_long sval)
@@ -647,36 +644,19 @@ static void tcg_out_movi_int(TCGContext *s, TCGType type, TCGReg ret,
         return;
     }
 
-    /* When allowed, stuff it in the constant pool.  */
-    if (!in_prologue) {
-        if (USE_REG_TB) {
-            tcg_out_insn(s, RXY, LG, ret, TCG_REG_TB, TCG_REG_NONE, 0);
-            new_pool_label(s, sval, R_390_20, s->code_ptr - 2,
-                           -(intptr_t)s->code_gen_ptr);
-        } else {
-            tcg_out_insn(s, RIL, LGRL, ret, 0);
-            new_pool_label(s, sval, R_390_PC32DBL, s->code_ptr - 2, 2);
-        }
-        return;
-    }
-
-    /* What's left is for the prologue, loading GUEST_BASE, and because
-       it failed to match above, is known to be a full 64-bit quantity.
-       We could try more than this, but it probably wouldn't pay off.  */
-    if (s390_facilities & FACILITY_EXT_IMM) {
-        tcg_out_insn(s, RIL, LLILF, ret, uval);
-        tcg_out_insn(s, RIL, IIHF, ret, uval >> 32);
+    /* Otherwise, stuff it in the constant pool.  */
+    if (s390_facilities & FACILITY_GEN_INST_EXT) {
+        tcg_out_insn(s, RIL, LGRL, ret, 0);
+        new_pool_label(s, sval, R_390_PC32DBL, s->code_ptr - 2, 2);
+    } else if (USE_REG_TB && !in_prologue) {
+        tcg_out_insn(s, RXY, LG, ret, TCG_REG_TB, TCG_REG_NONE, 0);
+        new_pool_label(s, sval, R_390_20, s->code_ptr - 2,
+                       -(intptr_t)s->code_gen_ptr);
     } else {
-        const S390Opcode *insns = lli_insns;
-        int i;
-
-        for (i = 0; i < 4; i++) {
-            uint16_t part = uval >> (16 * i);
-            if (part) {
-                tcg_out_insn_RI(s, insns[i], ret, part);
-                insns = ii_insns;
-            }
-        }
+        TCGReg base = ret ? ret : TCG_TMP0;
+        tcg_out_insn(s, RIL, LARL, base, 0);
+        new_pool_label(s, sval, R_390_PC32DBL, s->code_ptr - 2, 2);
+        tcg_out_insn(s, RXY, LG, ret, base, TCG_REG_NONE, 0);
     }
 }
 
-- 
2.13.6