All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/8] tcg queued patches
@ 2016-02-23 18:33 Richard Henderson
  2016-02-23 18:33 ` [Qemu-devel] [PULL 1/8] tcg: Work around clang bug wrt enum ranges, part 2 Richard Henderson
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: Richard Henderson @ 2016-02-23 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

A revision of the Feb 15 pull, with patch 5 adjusted to work around
the reported Werror bug in gcc 4.8.2, and Peter's renaming patch
set included.


r~


The following changes since commit 90ce6e2644db2c47d72f364b4de57342e50bd10a:

  include: Clean up includes (2016-02-23 12:43:05 +0000)

are available in the git repository at:

  git://github.com/rth7680/qemu.git tags/pull-tcg-20160223

for you to fetch changes up to c3b7f66800fbf9f47fddbcf2e2cd30ea932e0aae:

  tcg: Remove unnecessary osdep.h includes from tcg-target.inc.c (2016-02-23 08:31:03 -0800)

----------------------------------------------------------------
Queued TCG patches

----------------------------------------------------------------
Peter Maydell (3):
      tcg: Rename tcg-target.c to tcg-target.inc.c
      scripts/clean-includes: Ignore .inc.c files
      tcg: Remove unnecessary osdep.h includes from tcg-target.inc.c

Richard Henderson (5):
      tcg: Work around clang bug wrt enum ranges, part 2
      tcg: Implement indirect memory registers
      tcg: Allocate indirect_base temporaries in a different order
      target-sparc: Tidy global register initialization
      target-sparc: Use global registers for the register window

 qemu-tech.texi                                 |   2 +-
 scripts/clean-includes                         |   5 +
 target-sparc/translate.c                       | 204 ++++++++++++-------------
 tcg/README                                     |   5 +-
 tcg/aarch64/{tcg-target.c => tcg-target.inc.c} |   1 -
 tcg/arm/{tcg-target.c => tcg-target.inc.c}     |   1 -
 tcg/i386/{tcg-target.c => tcg-target.inc.c}    |   1 -
 tcg/ia64/{tcg-target.c => tcg-target.inc.c}    |   0
 tcg/mips/{tcg-target.c => tcg-target.inc.c}    |   1 -
 tcg/ppc/{tcg-target.c => tcg-target.inc.c}     |   1 -
 tcg/s390/{tcg-target.c => tcg-target.inc.c}    |   1 -
 tcg/sparc/{tcg-target.c => tcg-target.inc.c}   |   1 -
 tcg/tcg.c                                      | 145 ++++++++++++------
 tcg/tcg.h                                      |   4 +-
 tcg/tci/README                                 |   4 +-
 tcg/tci/{tcg-target.c => tcg-target.inc.c}     |   1 -
 16 files changed, 214 insertions(+), 163 deletions(-)
 rename tcg/aarch64/{tcg-target.c => tcg-target.inc.c} (99%)
 rename tcg/arm/{tcg-target.c => tcg-target.inc.c} (99%)
 rename tcg/i386/{tcg-target.c => tcg-target.inc.c} (99%)
 rename tcg/ia64/{tcg-target.c => tcg-target.inc.c} (100%)
 rename tcg/mips/{tcg-target.c => tcg-target.inc.c} (99%)
 rename tcg/ppc/{tcg-target.c => tcg-target.inc.c} (99%)
 rename tcg/s390/{tcg-target.c => tcg-target.inc.c} (99%)
 rename tcg/sparc/{tcg-target.c => tcg-target.inc.c} (99%)
 rename tcg/tci/{tcg-target.c => tcg-target.inc.c} (99%)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 1/8] tcg: Work around clang bug wrt enum ranges, part 2
  2016-02-23 18:33 [Qemu-devel] [PULL 0/8] tcg queued patches Richard Henderson
@ 2016-02-23 18:33 ` Richard Henderson
  2016-02-23 18:33 ` [Qemu-devel] [PULL 2/8] tcg: Implement indirect memory registers Richard Henderson
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2016-02-23 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

A previous patch patch changed the type of REG from int
to enum TCGReg, which provokes the following bug in clang:

  https://llvm.org/bugs/show_bug.cgi?id=16154

Signed-off-by: Richard Henderson  <rth@twiddle.net>
---
 tcg/tcg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 0317c9e..664e8e1 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1602,7 +1602,7 @@ static void dump_regs(TCGContext *s)
 
 static void check_regs(TCGContext *s)
 {
-    TCGReg reg;
+    int reg;
     int k;
     TCGTemp *ts;
     char buf[64];
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 2/8] tcg: Implement indirect memory registers
  2016-02-23 18:33 [Qemu-devel] [PULL 0/8] tcg queued patches Richard Henderson
  2016-02-23 18:33 ` [Qemu-devel] [PULL 1/8] tcg: Work around clang bug wrt enum ranges, part 2 Richard Henderson
@ 2016-02-23 18:33 ` Richard Henderson
  2016-02-23 18:33 ` [Qemu-devel] [PULL 3/8] tcg: Allocate indirect_base temporaries in a different order Richard Henderson
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2016-02-23 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

That is, global_mem registers whose base is another global_mem
register, rather than a fixed register.

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg.c | 95 ++++++++++++++++++++++++++++++++++++++++++++-------------------
 tcg/tcg.h |  2 ++
 2 files changed, 68 insertions(+), 29 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 664e8e1..ba2491f 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -506,17 +506,23 @@ int tcg_global_mem_new_internal(TCGType type, TCGv_ptr base,
     TCGContext *s = &tcg_ctx;
     TCGTemp *base_ts = &s->temps[GET_TCGV_PTR(base)];
     TCGTemp *ts = tcg_global_alloc(s);
-    int bigendian = 0;
+    int indirect_reg = 0, bigendian = 0;
 #ifdef HOST_WORDS_BIGENDIAN
     bigendian = 1;
 #endif
 
+    if (!base_ts->fixed_reg) {
+        indirect_reg = 1;
+        base_ts->indirect_base = 1;
+    }
+
     if (TCG_TARGET_REG_BITS == 32 && type == TCG_TYPE_I64) {
         TCGTemp *ts2 = tcg_global_alloc(s);
         char buf[64];
 
         ts->base_type = TCG_TYPE_I64;
         ts->type = TCG_TYPE_I32;
+        ts->indirect_reg = indirect_reg;
         ts->mem_allocated = 1;
         ts->mem_base = base_ts;
         ts->mem_offset = offset + bigendian * 4;
@@ -527,6 +533,7 @@ int tcg_global_mem_new_internal(TCGType type, TCGv_ptr base,
         tcg_debug_assert(ts2 == ts + 1);
         ts2->base_type = TCG_TYPE_I64;
         ts2->type = TCG_TYPE_I32;
+        ts2->indirect_reg = indirect_reg;
         ts2->mem_allocated = 1;
         ts2->mem_base = base_ts;
         ts2->mem_offset = offset + (1 - bigendian) * 4;
@@ -536,6 +543,7 @@ int tcg_global_mem_new_internal(TCGType type, TCGv_ptr base,
     } else {
         ts->base_type = type;
         ts->type = type;
+        ts->indirect_reg = indirect_reg;
         ts->mem_allocated = 1;
         ts->mem_base = base_ts;
         ts->mem_offset = offset;
@@ -1652,8 +1660,10 @@ static void temp_allocate_frame(TCGContext *s, int temp)
     s->current_frame_offset += sizeof(tcg_target_long);
 }
 
+static void temp_load(TCGContext *, TCGTemp *, TCGRegSet, TCGRegSet);
+
 /* sync register 'reg' by saving it to the corresponding temporary */
-static inline void tcg_reg_sync(TCGContext *s, TCGReg reg)
+static void tcg_reg_sync(TCGContext *s, TCGReg reg, TCGRegSet allocated_regs)
 {
     TCGTemp *ts = s->reg_to_temp[reg];
 
@@ -1661,6 +1671,11 @@ static inline void tcg_reg_sync(TCGContext *s, TCGReg reg)
     if (!ts->mem_coherent && !ts->fixed_reg) {
         if (!ts->mem_allocated) {
             temp_allocate_frame(s, temp_idx(s, ts));
+        } else if (ts->indirect_reg) {
+            tcg_regset_set_reg(allocated_regs, ts->reg);
+            temp_load(s, ts->mem_base,
+                      tcg_target_available_regs[TCG_TYPE_PTR],
+                      allocated_regs);
         }
         tcg_out_st(s, ts->type, reg, ts->mem_base->reg, ts->mem_offset);
     }
@@ -1668,25 +1683,26 @@ static inline void tcg_reg_sync(TCGContext *s, TCGReg reg)
 }
 
 /* free register 'reg' by spilling the corresponding temporary if necessary */
-static void tcg_reg_free(TCGContext *s, TCGReg reg)
+static void tcg_reg_free(TCGContext *s, TCGReg reg, TCGRegSet allocated_regs)
 {
     TCGTemp *ts = s->reg_to_temp[reg];
 
     if (ts != NULL) {
-        tcg_reg_sync(s, reg);
+        tcg_reg_sync(s, reg, allocated_regs);
         ts->val_type = TEMP_VAL_MEM;
         s->reg_to_temp[reg] = NULL;
     }
 }
 
 /* Allocate a register belonging to reg1 & ~reg2 */
-static TCGReg tcg_reg_alloc(TCGContext *s, TCGRegSet reg1, TCGRegSet reg2)
+static TCGReg tcg_reg_alloc(TCGContext *s, TCGRegSet desired_regs,
+                            TCGRegSet allocated_regs)
 {
     int i;
     TCGReg reg;
     TCGRegSet reg_ct;
 
-    tcg_regset_andnot(reg_ct, reg1, reg2);
+    tcg_regset_andnot(reg_ct, desired_regs, allocated_regs);
 
     /* first try free registers */
     for(i = 0; i < ARRAY_SIZE(tcg_target_reg_alloc_order); i++) {
@@ -1699,7 +1715,7 @@ static TCGReg tcg_reg_alloc(TCGContext *s, TCGRegSet reg1, TCGRegSet reg2)
     for(i = 0; i < ARRAY_SIZE(tcg_target_reg_alloc_order); i++) {
         reg = tcg_target_reg_alloc_order[i];
         if (tcg_regset_test_reg(reg_ct, reg)) {
-            tcg_reg_free(s, reg);
+            tcg_reg_free(s, reg, allocated_regs);
             return reg;
         }
     }
@@ -1724,6 +1740,12 @@ static void temp_load(TCGContext *s, TCGTemp *ts, TCGRegSet desired_regs,
         break;
     case TEMP_VAL_MEM:
         reg = tcg_reg_alloc(s, desired_regs, allocated_regs);
+        if (ts->indirect_reg) {
+            tcg_regset_set_reg(allocated_regs, reg);
+            temp_load(s, ts->mem_base,
+                      tcg_target_available_regs[TCG_TYPE_PTR],
+                      allocated_regs);
+        }
         tcg_out_ld(s, ts->type, reg, ts->mem_base->reg, ts->mem_offset);
         ts->mem_coherent = 1;
         break;
@@ -1761,7 +1783,7 @@ static void temp_sync(TCGContext *s, TCGTemp *ts, TCGRegSet allocated_regs)
         temp_load(s, ts, tcg_target_available_regs[ts->type], allocated_regs);
         /* fallthrough */
     case TEMP_VAL_REG:
-        tcg_reg_sync(s, ts->reg);
+        tcg_reg_sync(s, ts->reg, allocated_regs);
         break;
     case TEMP_VAL_DEAD:
     case TEMP_VAL_MEM:
@@ -1777,13 +1799,16 @@ static inline void temp_save(TCGContext *s, TCGTemp *ts,
                              TCGRegSet allocated_regs)
 {
 #ifdef USE_LIVENESS_ANALYSIS
-    /* The liveness analysis already ensures that globals are back
-       in memory. Keep an assert for safety. */
-    tcg_debug_assert(ts->val_type == TEMP_VAL_MEM || ts->fixed_reg);
-#else
+    /* ??? Liveness does not yet incorporate indirect bases.  */
+    if (!ts->indirect_base) {
+        /* The liveness analysis already ensures that globals are back
+           in memory. Keep an assert for safety. */
+        tcg_debug_assert(ts->val_type == TEMP_VAL_MEM || ts->fixed_reg);
+        return;
+    }
+#endif
     temp_sync(s, ts, allocated_regs);
     temp_dead(s, ts);
-#endif
 }
 
 /* save globals to their canonical location and assume they can be
@@ -1808,12 +1833,15 @@ static void sync_globals(TCGContext *s, TCGRegSet allocated_regs)
     for (i = 0; i < s->nb_globals; i++) {
         TCGTemp *ts = &s->temps[i];
 #ifdef USE_LIVENESS_ANALYSIS
-        tcg_debug_assert(ts->val_type != TEMP_VAL_REG
-                         || ts->fixed_reg
-                         || ts->mem_coherent);
-#else
-        temp_sync(s, ts, allocated_regs);
+        /* ??? Liveness does not yet incorporate indirect bases.  */
+        if (!ts->indirect_base) {
+            tcg_debug_assert(ts->val_type != TEMP_VAL_REG
+                             || ts->fixed_reg
+                             || ts->mem_coherent);
+            continue;
+        }
 #endif
+        temp_sync(s, ts, allocated_regs);
     }
 }
 
@@ -1829,12 +1857,15 @@ static void tcg_reg_alloc_bb_end(TCGContext *s, TCGRegSet allocated_regs)
             temp_save(s, ts, allocated_regs);
         } else {
 #ifdef USE_LIVENESS_ANALYSIS
-            /* The liveness analysis already ensures that temps are dead.
-               Keep an assert for safety. */
-            assert(ts->val_type == TEMP_VAL_DEAD);
-#else
-            temp_dead(s, ts);
+            /* ??? Liveness does not yet incorporate indirect bases.  */
+            if (!ts->indirect_base) {
+                /* The liveness analysis already ensures that temps are dead.
+                   Keep an assert for safety. */
+                assert(ts->val_type == TEMP_VAL_DEAD);
+                continue;
+            }
 #endif
+            temp_dead(s, ts);
         }
     }
 
@@ -1907,6 +1938,12 @@ static void tcg_reg_alloc_mov(TCGContext *s, const TCGOpDef *def,
         if (!ots->mem_allocated) {
             temp_allocate_frame(s, args[0]);
         }
+        if (ots->indirect_reg) {
+            tcg_regset_set_reg(allocated_regs, ts->reg);
+            temp_load(s, ots->mem_base,
+                      tcg_target_available_regs[TCG_TYPE_PTR],
+                      allocated_regs);
+        }
         tcg_out_st(s, otype, ts->reg, ots->mem_base->reg, ots->mem_offset);
         if (IS_DEAD_ARG(1)) {
             temp_dead(s, ts);
@@ -1947,7 +1984,7 @@ static void tcg_reg_alloc_mov(TCGContext *s, const TCGOpDef *def,
         ots->mem_coherent = 0;
         s->reg_to_temp[ots->reg] = ots;
         if (NEED_SYNC_ARG(0)) {
-            tcg_reg_sync(s, ots->reg);
+            tcg_reg_sync(s, ots->reg, allocated_regs);
         }
     }
 }
@@ -2047,7 +2084,7 @@ static void tcg_reg_alloc_op(TCGContext *s,
             /* XXX: permit generic clobber register list ? */ 
             for (i = 0; i < TCG_TARGET_NB_REGS; i++) {
                 if (tcg_regset_test_reg(tcg_target_call_clobber_regs, i)) {
-                    tcg_reg_free(s, i);
+                    tcg_reg_free(s, i, allocated_regs);
                 }
             }
         }
@@ -2104,7 +2141,7 @@ static void tcg_reg_alloc_op(TCGContext *s,
             tcg_out_mov(s, ts->type, ts->reg, reg);
         }
         if (NEED_SYNC_ARG(i)) {
-            tcg_reg_sync(s, reg);
+            tcg_reg_sync(s, reg, allocated_regs);
         }
         if (IS_DEAD_ARG(i)) {
             temp_dead(s, ts);
@@ -2175,7 +2212,7 @@ static void tcg_reg_alloc_call(TCGContext *s, int nb_oargs, int nb_iargs,
         if (arg != TCG_CALL_DUMMY_ARG) {
             ts = &s->temps[arg];
             reg = tcg_target_call_iarg_regs[i];
-            tcg_reg_free(s, reg);
+            tcg_reg_free(s, reg, allocated_regs);
 
             if (ts->val_type == TEMP_VAL_REG) {
                 if (ts->reg != reg) {
@@ -2203,7 +2240,7 @@ static void tcg_reg_alloc_call(TCGContext *s, int nb_oargs, int nb_iargs,
     /* clobber call registers */
     for (i = 0; i < TCG_TARGET_NB_REGS; i++) {
         if (tcg_regset_test_reg(tcg_target_call_clobber_regs, i)) {
-            tcg_reg_free(s, i);
+            tcg_reg_free(s, i, allocated_regs);
         }
     }
 
@@ -2239,7 +2276,7 @@ static void tcg_reg_alloc_call(TCGContext *s, int nb_oargs, int nb_iargs,
             ts->mem_coherent = 0;
             s->reg_to_temp[reg] = ts;
             if (NEED_SYNC_ARG(i)) {
-                tcg_reg_sync(s, reg);
+                tcg_reg_sync(s, reg, allocated_regs);
             }
             if (IS_DEAD_ARG(i)) {
                 temp_dead(s, ts);
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 83da5fb..d181694 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -453,6 +453,8 @@ typedef struct TCGTemp {
     TCGType base_type:8;
     TCGType type:8;
     unsigned int fixed_reg:1;
+    unsigned int indirect_reg:1;
+    unsigned int indirect_base:1;
     unsigned int mem_coherent:1;
     unsigned int mem_allocated:1;
     unsigned int temp_local:1; /* If true, the temp is saved across
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 3/8] tcg: Allocate indirect_base temporaries in a different order
  2016-02-23 18:33 [Qemu-devel] [PULL 0/8] tcg queued patches Richard Henderson
  2016-02-23 18:33 ` [Qemu-devel] [PULL 1/8] tcg: Work around clang bug wrt enum ranges, part 2 Richard Henderson
  2016-02-23 18:33 ` [Qemu-devel] [PULL 2/8] tcg: Implement indirect memory registers Richard Henderson
@ 2016-02-23 18:33 ` Richard Henderson
  2016-02-23 18:33 ` [Qemu-devel] [PULL 4/8] target-sparc: Tidy global register initialization Richard Henderson
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2016-02-23 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Since we've not got liveness analysis for indirect bases,
placing them at the end of the call-saved registers makes
it more likely that it'll stay live.

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg.c | 43 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index ba2491f..86208a8 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -318,6 +318,8 @@ static const TCGHelperInfo all_helpers[] = {
 #include "exec/helper-tcg.h"
 };
 
+static int indirect_reg_alloc_order[ARRAY_SIZE(tcg_target_reg_alloc_order)];
+
 void tcg_context_init(TCGContext *s)
 {
     int op, total_args, n, i;
@@ -360,6 +362,21 @@ void tcg_context_init(TCGContext *s)
     }
 
     tcg_target_init(s);
+
+    /* Reverse the order of the saved registers, assuming they're all at
+       the start of tcg_target_reg_alloc_order.  */
+    for (n = 0; n < ARRAY_SIZE(tcg_target_reg_alloc_order); ++n) {
+        int r = tcg_target_reg_alloc_order[n];
+        if (tcg_regset_test_reg(tcg_target_call_clobber_regs, r)) {
+            break;
+        }
+    }
+    for (i = 0; i < n; ++i) {
+        indirect_reg_alloc_order[i] = tcg_target_reg_alloc_order[n - 1 - i];
+    }
+    for (; i < ARRAY_SIZE(tcg_target_reg_alloc_order); ++i) {
+        indirect_reg_alloc_order[i] = tcg_target_reg_alloc_order[i];
+    }
 }
 
 void tcg_prologue_init(TCGContext *s)
@@ -1696,24 +1713,26 @@ static void tcg_reg_free(TCGContext *s, TCGReg reg, TCGRegSet allocated_regs)
 
 /* Allocate a register belonging to reg1 & ~reg2 */
 static TCGReg tcg_reg_alloc(TCGContext *s, TCGRegSet desired_regs,
-                            TCGRegSet allocated_regs)
+                            TCGRegSet allocated_regs, bool rev)
 {
-    int i;
+    int i, n = ARRAY_SIZE(tcg_target_reg_alloc_order);
+    const int *order;
     TCGReg reg;
     TCGRegSet reg_ct;
 
     tcg_regset_andnot(reg_ct, desired_regs, allocated_regs);
+    order = rev ? indirect_reg_alloc_order : tcg_target_reg_alloc_order;
 
     /* first try free registers */
-    for(i = 0; i < ARRAY_SIZE(tcg_target_reg_alloc_order); i++) {
-        reg = tcg_target_reg_alloc_order[i];
+    for(i = 0; i < n; i++) {
+        reg = order[i];
         if (tcg_regset_test_reg(reg_ct, reg) && s->reg_to_temp[reg] == NULL)
             return reg;
     }
 
     /* XXX: do better spill choice */
-    for(i = 0; i < ARRAY_SIZE(tcg_target_reg_alloc_order); i++) {
-        reg = tcg_target_reg_alloc_order[i];
+    for(i = 0; i < n; i++) {
+        reg = order[i];
         if (tcg_regset_test_reg(reg_ct, reg)) {
             tcg_reg_free(s, reg, allocated_regs);
             return reg;
@@ -1734,12 +1753,12 @@ static void temp_load(TCGContext *s, TCGTemp *ts, TCGRegSet desired_regs,
     case TEMP_VAL_REG:
         return;
     case TEMP_VAL_CONST:
-        reg = tcg_reg_alloc(s, desired_regs, allocated_regs);
+        reg = tcg_reg_alloc(s, desired_regs, allocated_regs, ts->indirect_base);
         tcg_out_movi(s, ts->type, reg, ts->val);
         ts->mem_coherent = 0;
         break;
     case TEMP_VAL_MEM:
-        reg = tcg_reg_alloc(s, desired_regs, allocated_regs);
+        reg = tcg_reg_alloc(s, desired_regs, allocated_regs, ts->indirect_base);
         if (ts->indirect_reg) {
             tcg_regset_set_reg(allocated_regs, reg);
             temp_load(s, ts->mem_base,
@@ -1976,7 +1995,7 @@ static void tcg_reg_alloc_mov(TCGContext *s, const TCGOpDef *def,
                    input one. */
                 tcg_regset_set_reg(allocated_regs, ts->reg);
                 ots->reg = tcg_reg_alloc(s, tcg_target_available_regs[otype],
-                                         allocated_regs);
+                                         allocated_regs, ots->indirect_base);
             }
             tcg_out_mov(s, otype, ots->reg, ts->reg);
         }
@@ -2061,7 +2080,8 @@ static void tcg_reg_alloc_op(TCGContext *s,
         allocate_in_reg:
             /* allocate a new register matching the constraint 
                and move the temporary register into it */
-            reg = tcg_reg_alloc(s, arg_ct->u.regs, allocated_regs);
+            reg = tcg_reg_alloc(s, arg_ct->u.regs, allocated_regs,
+                                ts->indirect_base);
             tcg_out_mov(s, ts->type, reg, ts->reg);
         }
         new_args[i] = reg;
@@ -2110,7 +2130,8 @@ static void tcg_reg_alloc_op(TCGContext *s,
                     tcg_regset_test_reg(arg_ct->u.regs, reg)) {
                     goto oarg_end;
                 }
-                reg = tcg_reg_alloc(s, arg_ct->u.regs, allocated_regs);
+                reg = tcg_reg_alloc(s, arg_ct->u.regs, allocated_regs,
+                                    ts->indirect_base);
             }
             tcg_regset_set_reg(allocated_regs, reg);
             /* if a fixed register is used, then a move will be done afterwards */
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 4/8] target-sparc: Tidy global register initialization
  2016-02-23 18:33 [Qemu-devel] [PULL 0/8] tcg queued patches Richard Henderson
                   ` (2 preceding siblings ...)
  2016-02-23 18:33 ` [Qemu-devel] [PULL 3/8] tcg: Allocate indirect_base temporaries in a different order Richard Henderson
@ 2016-02-23 18:33 ` Richard Henderson
  2016-02-23 18:33 ` [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window Richard Henderson
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2016-02-23 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Create tables for the various global registers that need allocation.
Remove one level of indirection from  gregnames and fregnames.

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-sparc/translate.c | 157 +++++++++++++++++++++--------------------------
 1 file changed, 70 insertions(+), 87 deletions(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 536c4b5..4be56dd 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -5329,106 +5329,89 @@ void gen_intermediate_code(CPUSPARCState * env, TranslationBlock * tb)
 
 void gen_intermediate_code_init(CPUSPARCState *env)
 {
-    unsigned int i;
     static int inited;
-    static const char * const gregnames[8] = {
-        NULL, // g0 not used
-        "g1",
-        "g2",
-        "g3",
-        "g4",
-        "g5",
-        "g6",
-        "g7",
+    static const char gregnames[8][4] = {
+        "g0", "g1", "g2", "g3", "g4", "g5", "g6", "g7",
     };
-    static const char * const fregnames[32] = {
+    static const char fregnames[32][4] = {
         "f0", "f2", "f4", "f6", "f8", "f10", "f12", "f14",
         "f16", "f18", "f20", "f22", "f24", "f26", "f28", "f30",
         "f32", "f34", "f36", "f38", "f40", "f42", "f44", "f46",
         "f48", "f50", "f52", "f54", "f56", "f58", "f60", "f62",
     };
 
-    /* init various static tables */
-    if (!inited) {
-        inited = 1;
-
-        cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
-        cpu_regwptr = tcg_global_mem_new_ptr(cpu_env,
-                                             offsetof(CPUSPARCState, regwptr),
-                                             "regwptr");
+    static const struct { TCGv_i32 *ptr; int off; const char *name; } r32[] = {
 #ifdef TARGET_SPARC64
-        cpu_xcc = tcg_global_mem_new_i32(cpu_env, offsetof(CPUSPARCState, xcc),
-                                         "xcc");
-        cpu_asi = tcg_global_mem_new_i32(cpu_env, offsetof(CPUSPARCState, asi),
-                                         "asi");
-        cpu_fprs = tcg_global_mem_new_i32(cpu_env,
-                                          offsetof(CPUSPARCState, fprs),
-                                          "fprs");
-        cpu_gsr = tcg_global_mem_new(cpu_env, offsetof(CPUSPARCState, gsr),
-                                     "gsr");
-        cpu_tick_cmpr = tcg_global_mem_new(cpu_env,
-                                           offsetof(CPUSPARCState, tick_cmpr),
-                                           "tick_cmpr");
-        cpu_stick_cmpr = tcg_global_mem_new(cpu_env,
-                                            offsetof(CPUSPARCState, stick_cmpr),
-                                            "stick_cmpr");
-        cpu_hstick_cmpr = tcg_global_mem_new(cpu_env,
-                                             offsetof(CPUSPARCState, hstick_cmpr),
-                                             "hstick_cmpr");
-        cpu_hintp = tcg_global_mem_new(cpu_env, offsetof(CPUSPARCState, hintp),
-                                       "hintp");
-        cpu_htba = tcg_global_mem_new(cpu_env, offsetof(CPUSPARCState, htba),
-                                      "htba");
-        cpu_hver = tcg_global_mem_new(cpu_env, offsetof(CPUSPARCState, hver),
-                                      "hver");
-        cpu_ssr = tcg_global_mem_new(cpu_env,
-                                     offsetof(CPUSPARCState, ssr), "ssr");
-        cpu_ver = tcg_global_mem_new(cpu_env,
-                                     offsetof(CPUSPARCState, version), "ver");
-        cpu_softint = tcg_global_mem_new_i32(cpu_env,
-                                             offsetof(CPUSPARCState, softint),
-                                             "softint");
+        { &cpu_xcc, offsetof(CPUSPARCState, xcc), "xcc" },
+        { &cpu_asi, offsetof(CPUSPARCState, asi), "asi" },
+        { &cpu_fprs, offsetof(CPUSPARCState, fprs), "fprs" },
+        { &cpu_softint, offsetof(CPUSPARCState, softint), "softint" },
 #else
-        cpu_wim = tcg_global_mem_new(cpu_env, offsetof(CPUSPARCState, wim),
-                                     "wim");
+        { &cpu_wim, offsetof(CPUSPARCState, wim), "wim" },
+#endif
+        { &cpu_cc_op, offsetof(CPUSPARCState, cc_op), "cc_op" },
+        { &cpu_psr, offsetof(CPUSPARCState, psr), "psr" },
+    };
+
+    static const struct { TCGv *ptr; int off; const char *name; } rtl[] = {
+#ifdef TARGET_SPARC64
+        { &cpu_gsr, offsetof(CPUSPARCState, gsr), "gsr" },
+        { &cpu_tick_cmpr, offsetof(CPUSPARCState, tick_cmpr), "tick_cmpr" },
+        { &cpu_stick_cmpr, offsetof(CPUSPARCState, stick_cmpr), "stick_cmpr" },
+        { &cpu_hstick_cmpr, offsetof(CPUSPARCState, hstick_cmpr),
+          "hstick_cmpr" },
+        { &cpu_hintp, offsetof(CPUSPARCState, hintp), "hintp" },
+        { &cpu_htba, offsetof(CPUSPARCState, htba), "htba" },
+        { &cpu_hver, offsetof(CPUSPARCState, hver), "hver" },
+        { &cpu_ssr, offsetof(CPUSPARCState, ssr), "ssr" },
+        { &cpu_ver, offsetof(CPUSPARCState, version), "ver" },
 #endif
-        cpu_cond = tcg_global_mem_new(cpu_env, offsetof(CPUSPARCState, cond),
-                                      "cond");
-        cpu_cc_src = tcg_global_mem_new(cpu_env,
-                                        offsetof(CPUSPARCState, cc_src),
-                                        "cc_src");
-        cpu_cc_src2 = tcg_global_mem_new(cpu_env,
-                                         offsetof(CPUSPARCState, cc_src2),
-                                         "cc_src2");
-        cpu_cc_dst = tcg_global_mem_new(cpu_env,
-                                        offsetof(CPUSPARCState, cc_dst),
-                                        "cc_dst");
-        cpu_cc_op = tcg_global_mem_new_i32(cpu_env,
-                                           offsetof(CPUSPARCState, cc_op),
-                                           "cc_op");
-        cpu_psr = tcg_global_mem_new_i32(cpu_env, offsetof(CPUSPARCState, psr),
-                                         "psr");
-        cpu_fsr = tcg_global_mem_new(cpu_env, offsetof(CPUSPARCState, fsr),
-                                     "fsr");
-        cpu_pc = tcg_global_mem_new(cpu_env, offsetof(CPUSPARCState, pc),
-                                    "pc");
-        cpu_npc = tcg_global_mem_new(cpu_env, offsetof(CPUSPARCState, npc),
-                                     "npc");
-        cpu_y = tcg_global_mem_new(cpu_env, offsetof(CPUSPARCState, y), "y");
+        { &cpu_cond, offsetof(CPUSPARCState, cond), "cond" },
+        { &cpu_cc_src, offsetof(CPUSPARCState, cc_src), "cc_src" },
+        { &cpu_cc_src2, offsetof(CPUSPARCState, cc_src2), "cc_src2" },
+        { &cpu_cc_dst, offsetof(CPUSPARCState, cc_dst), "cc_dst" },
+        { &cpu_fsr, offsetof(CPUSPARCState, fsr), "fsr" },
+        { &cpu_pc, offsetof(CPUSPARCState, pc), "pc" },
+        { &cpu_npc, offsetof(CPUSPARCState, npc), "npc" },
+        { &cpu_y, offsetof(CPUSPARCState, y), "y" },
 #ifndef CONFIG_USER_ONLY
-        cpu_tbr = tcg_global_mem_new(cpu_env, offsetof(CPUSPARCState, tbr),
-                                     "tbr");
+        { &cpu_tbr, offsetof(CPUSPARCState, tbr), "tbr" },
 #endif
-        for (i = 1; i < 8; i++) {
-            cpu_gregs[i] = tcg_global_mem_new(cpu_env,
-                                              offsetof(CPUSPARCState, gregs[i]),
-                                              gregnames[i]);
-        }
-        for (i = 0; i < TARGET_DPREGS; i++) {
-            cpu_fpr[i] = tcg_global_mem_new_i64(cpu_env,
-                                                offsetof(CPUSPARCState, fpr[i]),
-                                                fregnames[i]);
-        }
+    };
+
+    unsigned int i;
+
+    /* init various static tables */
+    if (inited) {
+        return;
+    }
+    inited = 1;
+
+    cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
+
+    cpu_regwptr = tcg_global_mem_new_ptr(cpu_env,
+                                         offsetof(CPUSPARCState, regwptr),
+                                         "regwptr");
+
+    for (i = 0; i < ARRAY_SIZE(r32); ++i) {
+        *r32[i].ptr = tcg_global_mem_new_i32(cpu_env, r32[i].off, r32[i].name);
+    }
+
+    for (i = 0; i < ARRAY_SIZE(rtl); ++i) {
+        *rtl[i].ptr = tcg_global_mem_new(cpu_env, rtl[i].off, rtl[i].name);
+    }
+
+    TCGV_UNUSED(cpu_gregs[0]);
+    for (i = 1; i < 8; ++i) {
+        cpu_gregs[i] = tcg_global_mem_new(cpu_env,
+                                          offsetof(CPUSPARCState, gregs[i]),
+                                          gregnames[i]);
+    }
+
+    for (i = 0; i < TARGET_DPREGS; i++) {
+        cpu_fpr[i] = tcg_global_mem_new_i64(cpu_env,
+                                            offsetof(CPUSPARCState, fpr[i]),
+                                            fregnames[i]);
     }
 }
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window
  2016-02-23 18:33 [Qemu-devel] [PULL 0/8] tcg queued patches Richard Henderson
                   ` (3 preceding siblings ...)
  2016-02-23 18:33 ` [Qemu-devel] [PULL 4/8] target-sparc: Tidy global register initialization Richard Henderson
@ 2016-02-23 18:33 ` Richard Henderson
  2016-06-14 21:52   ` Mark Cave-Ayland
  2016-02-23 18:33 ` [Qemu-devel] [PULL 6/8] tcg: Rename tcg-target.c to tcg-target.inc.c Richard Henderson
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2016-02-23 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Via indirection off cpu_regwptr.

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-sparc/translate.c | 57 ++++++++++++++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 24 deletions(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 4be56dd..00d61ee 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -43,7 +43,8 @@ static TCGv_ptr cpu_env, cpu_regwptr;
 static TCGv cpu_cc_src, cpu_cc_src2, cpu_cc_dst;
 static TCGv_i32 cpu_cc_op;
 static TCGv_i32 cpu_psr;
-static TCGv cpu_fsr, cpu_pc, cpu_npc, cpu_gregs[8];
+static TCGv cpu_fsr, cpu_pc, cpu_npc;
+static TCGv cpu_regs[32];
 static TCGv cpu_y;
 #ifndef CONFIG_USER_ONLY
 static TCGv cpu_tbr;
@@ -273,36 +274,31 @@ static inline void gen_address_mask(DisasContext *dc, TCGv addr)
 
 static inline TCGv gen_load_gpr(DisasContext *dc, int reg)
 {
-    if (reg == 0 || reg >= 8) {
+    if (reg > 0) {
+        assert(reg < 32);
+        return cpu_regs[reg];
+    } else {
         TCGv t = get_temp_tl(dc);
-        if (reg == 0) {
-            tcg_gen_movi_tl(t, 0);
-        } else {
-            tcg_gen_ld_tl(t, cpu_regwptr, (reg - 8) * sizeof(target_ulong));
-        }
+        tcg_gen_movi_tl(t, 0);
         return t;
-    } else {
-        return cpu_gregs[reg];
     }
 }
 
 static inline void gen_store_gpr(DisasContext *dc, int reg, TCGv v)
 {
     if (reg > 0) {
-        if (reg < 8) {
-            tcg_gen_mov_tl(cpu_gregs[reg], v);
-        } else {
-            tcg_gen_st_tl(v, cpu_regwptr, (reg - 8) * sizeof(target_ulong));
-        }
+        assert(reg < 32);
+        tcg_gen_mov_tl(cpu_regs[reg], v);
     }
 }
 
 static inline TCGv gen_dest_gpr(DisasContext *dc, int reg)
 {
-    if (reg == 0 || reg >= 8) {
-        return get_temp_tl(dc);
+    if (reg > 0) {
+        assert(reg < 32);
+        return cpu_regs[reg];
     } else {
-        return cpu_gregs[reg];
+        return get_temp_tl(dc);
     }
 }
 
@@ -2158,9 +2154,13 @@ static inline void gen_ldda_asi(DisasContext *dc, TCGv hi, TCGv addr,
     tcg_temp_free_i32(r_size);
     tcg_temp_free_i32(r_asi);
 
-    t = gen_dest_gpr(dc, rd + 1);
+    /* ??? Work around an apparent bug in Ubuntu gcc 4.8.2-10ubuntu2+12,
+       whereby "rd + 1" elicits "error: array subscript is above array".
+       Since we have already asserted that rd is even, the semantics
+       are unchanged.  */
+    t = gen_dest_gpr(dc, rd | 1);
     tcg_gen_trunc_i64_tl(t, t64);
-    gen_store_gpr(dc, rd + 1, t);
+    gen_store_gpr(dc, rd | 1, t);
 
     tcg_gen_shri_i64(t64, t64, 32);
     tcg_gen_trunc_i64_tl(hi, t64);
@@ -5330,8 +5330,11 @@ void gen_intermediate_code(CPUSPARCState * env, TranslationBlock * tb)
 void gen_intermediate_code_init(CPUSPARCState *env)
 {
     static int inited;
-    static const char gregnames[8][4] = {
+    static const char gregnames[32][4] = {
         "g0", "g1", "g2", "g3", "g4", "g5", "g6", "g7",
+        "o0", "o1", "o2", "o3", "o4", "o5", "o6", "o7",
+        "l0", "l1", "l2", "l3", "l4", "l5", "l6", "l7",
+        "i0", "i1", "i2", "i3", "i4", "i5", "i6", "i7",
     };
     static const char fregnames[32][4] = {
         "f0", "f2", "f4", "f6", "f8", "f10", "f12", "f14",
@@ -5401,11 +5404,17 @@ void gen_intermediate_code_init(CPUSPARCState *env)
         *rtl[i].ptr = tcg_global_mem_new(cpu_env, rtl[i].off, rtl[i].name);
     }
 
-    TCGV_UNUSED(cpu_gregs[0]);
+    TCGV_UNUSED(cpu_regs[0]);
     for (i = 1; i < 8; ++i) {
-        cpu_gregs[i] = tcg_global_mem_new(cpu_env,
-                                          offsetof(CPUSPARCState, gregs[i]),
-                                          gregnames[i]);
+        cpu_regs[i] = tcg_global_mem_new(cpu_env,
+                                         offsetof(CPUSPARCState, gregs[i]),
+                                         gregnames[i]);
+    }
+
+    for (i = 8; i < 32; ++i) {
+        cpu_regs[i] = tcg_global_mem_new(cpu_regwptr,
+                                         (i - 8) * sizeof(target_ulong),
+                                         gregnames[i]);
     }
 
     for (i = 0; i < TARGET_DPREGS; i++) {
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 6/8] tcg: Rename tcg-target.c to tcg-target.inc.c
  2016-02-23 18:33 [Qemu-devel] [PULL 0/8] tcg queued patches Richard Henderson
                   ` (4 preceding siblings ...)
  2016-02-23 18:33 ` [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window Richard Henderson
@ 2016-02-23 18:33 ` Richard Henderson
  2016-02-23 18:33 ` [Qemu-devel] [PULL 7/8] scripts/clean-includes: Ignore .inc.c files Richard Henderson
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2016-02-23 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

From: Peter Maydell <peter.maydell@linaro.org>

Rename the per-architecture tcg-target.c files to tcg-target.inc.c.
This makes it clearer that they are not intended to be standalone
C files, but are instead #included into another source file.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <1456238983-10160-2-git-send-email-peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 qemu-tech.texi                                 | 2 +-
 tcg/README                                     | 5 +++--
 tcg/aarch64/{tcg-target.c => tcg-target.inc.c} | 0
 tcg/arm/{tcg-target.c => tcg-target.inc.c}     | 0
 tcg/i386/{tcg-target.c => tcg-target.inc.c}    | 0
 tcg/ia64/{tcg-target.c => tcg-target.inc.c}    | 0
 tcg/mips/{tcg-target.c => tcg-target.inc.c}    | 0
 tcg/ppc/{tcg-target.c => tcg-target.inc.c}     | 0
 tcg/s390/{tcg-target.c => tcg-target.inc.c}    | 0
 tcg/sparc/{tcg-target.c => tcg-target.inc.c}   | 0
 tcg/tcg.c                                      | 7 ++++---
 tcg/tcg.h                                      | 2 +-
 tcg/tci/README                                 | 4 ++--
 tcg/tci/{tcg-target.c => tcg-target.inc.c}     | 0
 14 files changed, 11 insertions(+), 9 deletions(-)
 rename tcg/aarch64/{tcg-target.c => tcg-target.inc.c} (100%)
 rename tcg/arm/{tcg-target.c => tcg-target.inc.c} (100%)
 rename tcg/i386/{tcg-target.c => tcg-target.inc.c} (100%)
 rename tcg/ia64/{tcg-target.c => tcg-target.inc.c} (100%)
 rename tcg/mips/{tcg-target.c => tcg-target.inc.c} (100%)
 rename tcg/ppc/{tcg-target.c => tcg-target.inc.c} (100%)
 rename tcg/s390/{tcg-target.c => tcg-target.inc.c} (100%)
 rename tcg/sparc/{tcg-target.c => tcg-target.inc.c} (100%)
 rename tcg/tci/{tcg-target.c => tcg-target.inc.c} (100%)

diff --git a/qemu-tech.texi b/qemu-tech.texi
index 022017d..bdb2285 100644
--- a/qemu-tech.texi
+++ b/qemu-tech.texi
@@ -385,7 +385,7 @@ ops (see @code{target-i386/translate.c}). Some optimizations can be
 performed at this stage, including liveness analysis and trivial
 constant expression evaluation. TCG ops are then implemented in the
 host CPU back end, also known as TCG target (see
-@code{tcg/i386/tcg-target.c}). For more information, please take a
+@code{tcg/i386/tcg-target.inc.c}). For more information, please take a
 look at @code{tcg/README}.
 
 @node Condition code optimisations
diff --git a/tcg/README b/tcg/README
index 34c0775..f4a8ac1 100644
--- a/tcg/README
+++ b/tcg/README
@@ -460,8 +460,9 @@ function tcg_gen_xxx(args).
 
 4) Backend
 
-tcg-target.h contains the target specific definitions. tcg-target.c
-contains the target specific code.
+tcg-target.h contains the target specific definitions. tcg-target.inc.c
+contains the target specific code; it is #included by tcg/tcg.c, rather
+than being a standalone C file.
 
 4.1) Assumptions
 
diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.inc.c
similarity index 100%
rename from tcg/aarch64/tcg-target.c
rename to tcg/aarch64/tcg-target.inc.c
diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.inc.c
similarity index 100%
rename from tcg/arm/tcg-target.c
rename to tcg/arm/tcg-target.inc.c
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.inc.c
similarity index 100%
rename from tcg/i386/tcg-target.c
rename to tcg/i386/tcg-target.inc.c
diff --git a/tcg/ia64/tcg-target.c b/tcg/ia64/tcg-target.inc.c
similarity index 100%
rename from tcg/ia64/tcg-target.c
rename to tcg/ia64/tcg-target.inc.c
diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.inc.c
similarity index 100%
rename from tcg/mips/tcg-target.c
rename to tcg/mips/tcg-target.inc.c
diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.inc.c
similarity index 100%
rename from tcg/ppc/tcg-target.c
rename to tcg/ppc/tcg-target.inc.c
diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.inc.c
similarity index 100%
rename from tcg/s390/tcg-target.c
rename to tcg/s390/tcg-target.inc.c
diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.inc.c
similarity index 100%
rename from tcg/sparc/tcg-target.c
rename to tcg/sparc/tcg-target.inc.c
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 86208a8..550671b 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -62,7 +62,8 @@
 #include "elf.h"
 #include "exec/log.h"
 
-/* Forward declarations for functions declared in tcg-target.c and used here. */
+/* Forward declarations for functions declared in tcg-target.inc.c and
+   used here. */
 static void tcg_target_init(TCGContext *s);
 static void tcg_target_qemu_prologue(TCGContext *s);
 static void patch_reloc(tcg_insn_unit *code_ptr, int type,
@@ -96,7 +97,7 @@ static void tcg_register_jit_int(void *buf, size_t size,
                                  size_t debug_frame_size)
     __attribute__((unused));
 
-/* Forward declarations for functions declared and used in tcg-target.c. */
+/* Forward declarations for functions declared and used in tcg-target.inc.c. */
 static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str);
 static void tcg_out_ld(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg1,
                        intptr_t arg2);
@@ -250,7 +251,7 @@ TCGLabel *gen_new_label(void)
     return l;
 }
 
-#include "tcg-target.c"
+#include "tcg-target.inc.c"
 
 /* pool based memory allocation */
 void *tcg_malloc_internal(TCGContext *s, int size)
diff --git a/tcg/tcg.h b/tcg/tcg.h
index d181694..c45329a 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -568,7 +568,7 @@ struct TCGContext {
 
     TBContext tb_ctx;
 
-    /* The TCGBackendData structure is private to tcg-target.c.  */
+    /* The TCGBackendData structure is private to tcg-target.inc.c.  */
     struct TCGBackendData *be;
 
     TCGTempSet free_temps[TCG_TYPE_COUNT * 2];
diff --git a/tcg/tci/README b/tcg/tci/README
index dc57f07..3786b09 100644
--- a/tcg/tci/README
+++ b/tcg/tci/README
@@ -21,7 +21,7 @@ This is what TCI (Tiny Code Interpreter) does.
 2) Implementation
 
 Like each TCG host frontend, TCI implements the code generator in
-tcg-target.c, tcg-target.h. Both files are in directory tcg/tci.
+tcg-target.inc.c, tcg-target.h. Both files are in directory tcg/tci.
 
 The additional file tcg/tci.c adds the interpreter.
 
@@ -123,7 +123,7 @@ u1 = linux-user-test works
   would also improve speed for hosts which support byte alignment).
 
 * A better disassembler for the pseudo code would be nice (a very primitive
-  disassembler is included in tcg-target.c).
+  disassembler is included in tcg-target.inc.c).
 
 * It might be useful to have a runtime option which selects the native TCG
   or TCI, so QEMU would have to include two TCGs. Today, selecting TCI
diff --git a/tcg/tci/tcg-target.c b/tcg/tci/tcg-target.inc.c
similarity index 100%
rename from tcg/tci/tcg-target.c
rename to tcg/tci/tcg-target.inc.c
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 7/8] scripts/clean-includes: Ignore .inc.c files
  2016-02-23 18:33 [Qemu-devel] [PULL 0/8] tcg queued patches Richard Henderson
                   ` (5 preceding siblings ...)
  2016-02-23 18:33 ` [Qemu-devel] [PULL 6/8] tcg: Rename tcg-target.c to tcg-target.inc.c Richard Henderson
@ 2016-02-23 18:33 ` Richard Henderson
  2016-02-23 18:33 ` [Qemu-devel] [PULL 8/8] tcg: Remove unnecessary osdep.h includes from tcg-target.inc.c Richard Henderson
  2016-02-25  9:47 ` [Qemu-devel] [PULL 0/8] tcg queued patches Peter Maydell
  8 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2016-02-23 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

From: Peter Maydell <peter.maydell@linaro.org>

Ignore files which have a .inc.c extension -- these are not headers
but they are not standalone C source files either, so we can't make
any automated decisions about what #include directives they should
have.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <1456238983-10160-3-git-send-email-peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 scripts/clean-includes | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/scripts/clean-includes b/scripts/clean-includes
index d2dd7ae..a1faa60 100755
--- a/scripts/clean-includes
+++ b/scripts/clean-includes
@@ -94,6 +94,11 @@ EOT
 
 for f in "$@"; do
   case "$f" in
+    *.inc.c)
+      # These aren't standalone C source files
+      echo "SKIPPING $f (not a standalone source file)"
+      continue
+      ;;
     *.c)
       MODE=c
       ;;
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 8/8] tcg: Remove unnecessary osdep.h includes from tcg-target.inc.c
  2016-02-23 18:33 [Qemu-devel] [PULL 0/8] tcg queued patches Richard Henderson
                   ` (6 preceding siblings ...)
  2016-02-23 18:33 ` [Qemu-devel] [PULL 7/8] scripts/clean-includes: Ignore .inc.c files Richard Henderson
@ 2016-02-23 18:33 ` Richard Henderson
  2016-02-25  9:47 ` [Qemu-devel] [PULL 0/8] tcg queued patches Peter Maydell
  8 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2016-02-23 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

From: Peter Maydell <peter.maydell@linaro.org>

Commit 757e725b58c57d added a number of #include "qemu/osdep.h"
files to the tcg-target.c files (as they were named at the time).
These are unnecessary because these files are not standalone C
files, and the tcg/tcg.c file which includes them will have
already included osdep.h on their behalf. Remove the unneeded
include directives.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <1456238983-10160-4-git-send-email-peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/aarch64/tcg-target.inc.c | 1 -
 tcg/arm/tcg-target.inc.c     | 1 -
 tcg/i386/tcg-target.inc.c    | 1 -
 tcg/mips/tcg-target.inc.c    | 1 -
 tcg/ppc/tcg-target.inc.c     | 1 -
 tcg/s390/tcg-target.inc.c    | 1 -
 tcg/sparc/tcg-target.inc.c   | 1 -
 tcg/tci/tcg-target.inc.c     | 1 -
 8 files changed, 8 deletions(-)

diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
index 8467d5d..0ed10a9 100644
--- a/tcg/aarch64/tcg-target.inc.c
+++ b/tcg/aarch64/tcg-target.inc.c
@@ -10,7 +10,6 @@
  * See the COPYING file in the top-level directory for details.
  */
 
-#include "qemu/osdep.h"
 #include "tcg-be-ldst.h"
 #include "qemu/bitops.h"
 
diff --git a/tcg/arm/tcg-target.inc.c b/tcg/arm/tcg-target.inc.c
index 146ac00..3edf6a6 100644
--- a/tcg/arm/tcg-target.inc.c
+++ b/tcg/arm/tcg-target.inc.c
@@ -22,7 +22,6 @@
  * THE SOFTWARE.
  */
 
-#include "qemu/osdep.h"
 #include "elf.h"
 #include "tcg-be-ldst.h"
 
diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index d90636c..9187d34 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -22,7 +22,6 @@
  * THE SOFTWARE.
  */
 
-#include "qemu/osdep.h"
 #include "tcg-be-ldst.h"
 
 #ifndef NDEBUG
diff --git a/tcg/mips/tcg-target.inc.c b/tcg/mips/tcg-target.inc.c
index 2dc4998..297bd00 100644
--- a/tcg/mips/tcg-target.inc.c
+++ b/tcg/mips/tcg-target.inc.c
@@ -24,7 +24,6 @@
  * THE SOFTWARE.
  */
 
-#include "qemu/osdep.h"
 #include "tcg-be-ldst.h"
 
 #ifdef HOST_WORDS_BIGENDIAN
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index c593344..8c1c2df 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -22,7 +22,6 @@
  * THE SOFTWARE.
  */
 
-#include "qemu/osdep.h"
 #include "tcg-be-ldst.h"
 
 #if defined _CALL_DARWIN || defined __APPLE__
diff --git a/tcg/s390/tcg-target.inc.c b/tcg/s390/tcg-target.inc.c
index 58520fa..fbf97bb 100644
--- a/tcg/s390/tcg-target.inc.c
+++ b/tcg/s390/tcg-target.inc.c
@@ -24,7 +24,6 @@
  * THE SOFTWARE.
  */
 
-#include "qemu/osdep.h"
 #include "tcg-be-ldst.h"
 
 /* We only support generating code for 64-bit mode.  */
diff --git a/tcg/sparc/tcg-target.inc.c b/tcg/sparc/tcg-target.inc.c
index d3100ab..54df1bc 100644
--- a/tcg/sparc/tcg-target.inc.c
+++ b/tcg/sparc/tcg-target.inc.c
@@ -22,7 +22,6 @@
  * THE SOFTWARE.
  */
 
-#include "qemu/osdep.h"
 #include "tcg-be-null.h"
 
 #ifndef NDEBUG
diff --git a/tcg/tci/tcg-target.inc.c b/tcg/tci/tcg-target.inc.c
index 16ce048..4afe4d7 100644
--- a/tcg/tci/tcg-target.inc.c
+++ b/tcg/tci/tcg-target.inc.c
@@ -22,7 +22,6 @@
  * THE SOFTWARE.
  */
 
-#include "qemu/osdep.h"
 #include "tcg-be-null.h"
 
 /* TODO list:
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 0/8] tcg queued patches
  2016-02-23 18:33 [Qemu-devel] [PULL 0/8] tcg queued patches Richard Henderson
                   ` (7 preceding siblings ...)
  2016-02-23 18:33 ` [Qemu-devel] [PULL 8/8] tcg: Remove unnecessary osdep.h includes from tcg-target.inc.c Richard Henderson
@ 2016-02-25  9:47 ` Peter Maydell
  8 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2016-02-25  9:47 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers

On 23 February 2016 at 18:33, Richard Henderson <rth@twiddle.net> wrote:
> A revision of the Feb 15 pull, with patch 5 adjusted to work around
> the reported Werror bug in gcc 4.8.2, and Peter's renaming patch
> set included.
>
>
> r~
>
>
> The following changes since commit 90ce6e2644db2c47d72f364b4de57342e50bd10a:
>
>   include: Clean up includes (2016-02-23 12:43:05 +0000)
>
> are available in the git repository at:
>
>   git://github.com/rth7680/qemu.git tags/pull-tcg-20160223
>
> for you to fetch changes up to c3b7f66800fbf9f47fddbcf2e2cd30ea932e0aae:
>
>   tcg: Remove unnecessary osdep.h includes from tcg-target.inc.c (2016-02-23 08:31:03 -0800)
>
> ----------------------------------------------------------------
> Queued TCG patches
>

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window
  2016-02-23 18:33 ` [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window Richard Henderson
@ 2016-06-14 21:52   ` Mark Cave-Ayland
  2016-06-16 20:26     ` Richard Henderson
  0 siblings, 1 reply; 20+ messages in thread
From: Mark Cave-Ayland @ 2016-06-14 21:52 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On 23/02/16 18:33, Richard Henderson wrote:

> Via indirection off cpu_regwptr.
> 
> Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target-sparc/translate.c | 57 ++++++++++++++++++++++++++++--------------------
>  1 file changed, 33 insertions(+), 24 deletions(-)
> 
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index 4be56dd..00d61ee 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -43,7 +43,8 @@ static TCGv_ptr cpu_env, cpu_regwptr;
>  static TCGv cpu_cc_src, cpu_cc_src2, cpu_cc_dst;
>  static TCGv_i32 cpu_cc_op;
>  static TCGv_i32 cpu_psr;
> -static TCGv cpu_fsr, cpu_pc, cpu_npc, cpu_gregs[8];
> +static TCGv cpu_fsr, cpu_pc, cpu_npc;
> +static TCGv cpu_regs[32];
>  static TCGv cpu_y;
>  #ifndef CONFIG_USER_ONLY
>  static TCGv cpu_tbr;
> @@ -273,36 +274,31 @@ static inline void gen_address_mask(DisasContext *dc, TCGv addr)
>  
>  static inline TCGv gen_load_gpr(DisasContext *dc, int reg)
>  {
> -    if (reg == 0 || reg >= 8) {
> +    if (reg > 0) {
> +        assert(reg < 32);
> +        return cpu_regs[reg];
> +    } else {
>          TCGv t = get_temp_tl(dc);
> -        if (reg == 0) {
> -            tcg_gen_movi_tl(t, 0);
> -        } else {
> -            tcg_gen_ld_tl(t, cpu_regwptr, (reg - 8) * sizeof(target_ulong));
> -        }
> +        tcg_gen_movi_tl(t, 0);
>          return t;
> -    } else {
> -        return cpu_gregs[reg];
>      }
>  }
>  
>  static inline void gen_store_gpr(DisasContext *dc, int reg, TCGv v)
>  {
>      if (reg > 0) {
> -        if (reg < 8) {
> -            tcg_gen_mov_tl(cpu_gregs[reg], v);
> -        } else {
> -            tcg_gen_st_tl(v, cpu_regwptr, (reg - 8) * sizeof(target_ulong));
> -        }
> +        assert(reg < 32);
> +        tcg_gen_mov_tl(cpu_regs[reg], v);
>      }
>  }
>  
>  static inline TCGv gen_dest_gpr(DisasContext *dc, int reg)
>  {
> -    if (reg == 0 || reg >= 8) {
> -        return get_temp_tl(dc);
> +    if (reg > 0) {
> +        assert(reg < 32);
> +        return cpu_regs[reg];
>      } else {
> -        return cpu_gregs[reg];
> +        return get_temp_tl(dc);
>      }
>  }
>  
> @@ -2158,9 +2154,13 @@ static inline void gen_ldda_asi(DisasContext *dc, TCGv hi, TCGv addr,
>      tcg_temp_free_i32(r_size);
>      tcg_temp_free_i32(r_asi);
>  
> -    t = gen_dest_gpr(dc, rd + 1);
> +    /* ??? Work around an apparent bug in Ubuntu gcc 4.8.2-10ubuntu2+12,
> +       whereby "rd + 1" elicits "error: array subscript is above array".
> +       Since we have already asserted that rd is even, the semantics
> +       are unchanged.  */
> +    t = gen_dest_gpr(dc, rd | 1);
>      tcg_gen_trunc_i64_tl(t, t64);
> -    gen_store_gpr(dc, rd + 1, t);
> +    gen_store_gpr(dc, rd | 1, t);
>  
>      tcg_gen_shri_i64(t64, t64, 32);
>      tcg_gen_trunc_i64_tl(hi, t64);
> @@ -5330,8 +5330,11 @@ void gen_intermediate_code(CPUSPARCState * env, TranslationBlock * tb)
>  void gen_intermediate_code_init(CPUSPARCState *env)
>  {
>      static int inited;
> -    static const char gregnames[8][4] = {
> +    static const char gregnames[32][4] = {
>          "g0", "g1", "g2", "g3", "g4", "g5", "g6", "g7",
> +        "o0", "o1", "o2", "o3", "o4", "o5", "o6", "o7",
> +        "l0", "l1", "l2", "l3", "l4", "l5", "l6", "l7",
> +        "i0", "i1", "i2", "i3", "i4", "i5", "i6", "i7",
>      };
>      static const char fregnames[32][4] = {
>          "f0", "f2", "f4", "f6", "f8", "f10", "f12", "f14",
> @@ -5401,11 +5404,17 @@ void gen_intermediate_code_init(CPUSPARCState *env)
>          *rtl[i].ptr = tcg_global_mem_new(cpu_env, rtl[i].off, rtl[i].name);
>      }
>  
> -    TCGV_UNUSED(cpu_gregs[0]);
> +    TCGV_UNUSED(cpu_regs[0]);
>      for (i = 1; i < 8; ++i) {
> -        cpu_gregs[i] = tcg_global_mem_new(cpu_env,
> -                                          offsetof(CPUSPARCState, gregs[i]),
> -                                          gregnames[i]);
> +        cpu_regs[i] = tcg_global_mem_new(cpu_env,
> +                                         offsetof(CPUSPARCState, gregs[i]),
> +                                         gregnames[i]);
> +    }
> +
> +    for (i = 8; i < 32; ++i) {
> +        cpu_regs[i] = tcg_global_mem_new(cpu_regwptr,
> +                                         (i - 8) * sizeof(target_ulong),
> +                                         gregnames[i]);
>      }
>  
>      for (i = 0; i < TARGET_DPREGS; i++) {
> 

Hi Richard,

Following up the bug report at
https://bugs.launchpad.net/qemu/+bug/1588328, I bisected the regression
down to this particular commit. I can't see anything obvious here, so
perhaps this is exposing another bug somewhere else?


ATB,

Mark.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window
  2016-06-14 21:52   ` Mark Cave-Ayland
@ 2016-06-16 20:26     ` Richard Henderson
  2016-06-16 21:53       ` Mark Cave-Ayland
  0 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2016-06-16 20:26 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel

On 06/14/2016 02:52 PM, Mark Cave-Ayland wrote:
> Following up the bug report at
> https://bugs.launchpad.net/qemu/+bug/1588328, I bisected the regression
> down to this particular commit. I can't see anything obvious here, so
> perhaps this is exposing another bug somewhere else?
> 

Probably.  I'm downloading the solaris image now.


r~

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window
  2016-06-16 20:26     ` Richard Henderson
@ 2016-06-16 21:53       ` Mark Cave-Ayland
  2016-06-24  3:57         ` Richard Henderson
  0 siblings, 1 reply; 20+ messages in thread
From: Mark Cave-Ayland @ 2016-06-16 21:53 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On 16/06/16 21:26, Richard Henderson wrote:

> On 06/14/2016 02:52 PM, Mark Cave-Ayland wrote:
>> Following up the bug report at
>> https://bugs.launchpad.net/qemu/+bug/1588328, I bisected the regression
>> down to this particular commit. I can't see anything obvious here, so
>> perhaps this is exposing another bug somewhere else?
>>
> 
> Probably.  I'm downloading the solaris image now.
> 
> 
> r~

Thanks for taking a look - otherwise I won't be able to get to this
until next week. My thinking was that since the code makes access to
regwptr direct instead of copied into a temporary, something is
accidentally clobbering a destination register...


ATB,

Mark.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window
  2016-06-16 21:53       ` Mark Cave-Ayland
@ 2016-06-24  3:57         ` Richard Henderson
  2016-06-24  6:36           ` Paolo Bonzini
  0 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2016-06-24  3:57 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel

On 06/16/2016 02:53 PM, Mark Cave-Ayland wrote:
> On 16/06/16 21:26, Richard Henderson wrote:
>
>> On 06/14/2016 02:52 PM, Mark Cave-Ayland wrote:
>>> Following up the bug report at
>>> https://bugs.launchpad.net/qemu/+bug/1588328, I bisected the regression
>>> down to this particular commit. I can't see anything obvious here, so
>>> perhaps this is exposing another bug somewhere else?
>>>
>>
>> Probably.  I'm downloading the solaris image now.
>>
>>
>> r~
>
> Thanks for taking a look - otherwise I won't be able to get to this
> until next week. My thinking was that since the code makes access to
> regwptr direct instead of copied into a temporary, something is
> accidentally clobbering a destination register...

I've been unable to find this.

Whatever happens, it happens after 10GB of logs, which is simply too much to 
sift through.  I've tried to narrow it down, but the lack of a hardware tlb 
refill means that we get hundreds of thousands of Data Access Faults that are 
simply TLB misses and not the actual Segmentation Fault in question.

It doesn't seem to affect other OSes, so I can't imagine what quirk is being 
exercised in this case.

As loath as I am to suggest it, we may have to revert the sparc indirect 
register patch for the release.

I do now ping the rest of my sparc improvements patchset.  It's completely 
independent of the use of indirect registers.


r~

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window
  2016-06-24  3:57         ` Richard Henderson
@ 2016-06-24  6:36           ` Paolo Bonzini
  2016-06-24  8:12             ` Peter Maydell
  2016-06-24 10:42             ` Mark Cave-Ayland
  0 siblings, 2 replies; 20+ messages in thread
From: Paolo Bonzini @ 2016-06-24  6:36 UTC (permalink / raw)
  To: Richard Henderson, Mark Cave-Ayland; +Cc: qemu-devel



On 24/06/2016 05:57, Richard Henderson wrote:
> 
> Whatever happens, it happens after 10GB of logs, which is simply too
> much to sift through.  I've tried to narrow it down, but the lack of a
> hardware tlb refill means that we get hundreds of thousands of Data
> Access Faults that are simply TLB misses and not the actual Segmentation
> Fault in question.
> 
> It doesn't seem to affect other OSes, so I can't imagine what quirk is
> being exercised in this case.
> 
> As loath as I am to suggest it, we may have to revert the sparc indirect
> register patch for the release.

We have more than a month.  If it's reproducible, it can be fixed. :)

> I do now ping the rest of my sparc improvements patchset.  It's
> completely independent of the use of indirect registers.

Mark, perhaps you can try to use migration to reduce the amount of
logging?  (Start QEMU with -snapshot, try to stop the vm before it
fails.  If you succeed, do a "migrate exec:cat>foo.sav" followed by
"commit"; if you fail, try again).

It would be nice to have a mechanism to stop the VM after executing N
basic blocks.  Binary search on this value then can help with coming up
with a more easily debuggable snapshot, possibly to a point where the
difference between pre-patch and post-patch becomes deterministic.

Paolo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window
  2016-06-24  6:36           ` Paolo Bonzini
@ 2016-06-24  8:12             ` Peter Maydell
  2016-06-24  8:16               ` Paolo Bonzini
  2016-06-24 10:42             ` Mark Cave-Ayland
  1 sibling, 1 reply; 20+ messages in thread
From: Peter Maydell @ 2016-06-24  8:12 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Richard Henderson, Mark Cave-Ayland, QEMU Developers

On 24 June 2016 at 07:36, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Mark, perhaps you can try to use migration to reduce the amount of
> logging?  (Start QEMU with -snapshot, try to stop the vm before it
> fails.  If you succeed, do a "migrate exec:cat>foo.sav" followed by
> "commit"; if you fail, try again).

Why drag migration into it? I usually use 'savevm' and then
the -loadvm command line argument for this. (You need a
qcow2 disk image.)

> It would be nice to have a mechanism to stop the VM after executing N
> basic blocks.  Binary search on this value then can help with coming up
> with a more easily debuggable snapshot, possibly to a point where the
> difference between pre-patch and post-patch becomes deterministic.

You can use the monitor and an expect script to say "take a
snapshot 0.7 seconds into boot", which I've found to be
a good enough approximation:

https://translatedcode.wordpress.com/2015/07/06/tricks-for-debugging-qemu-savevm-snapshots/

thanks
-- PMM

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window
  2016-06-24  8:12             ` Peter Maydell
@ 2016-06-24  8:16               ` Paolo Bonzini
  0 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2016-06-24  8:16 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Richard Henderson, Mark Cave-Ayland, QEMU Developers



On 24/06/2016 10:12, Peter Maydell wrote:
> On 24 June 2016 at 07:36, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Mark, perhaps you can try to use migration to reduce the amount of
>> logging?  (Start QEMU with -snapshot, try to stop the vm before it
>> fails.  If you succeed, do a "migrate exec:cat>foo.sav" followed by
>> "commit"; if you fail, try again).
> 
> Why drag migration into it? I usually use 'savevm' and then
> the -loadvm command line argument for this. (You need a
> qcow2 disk image.)

Well, migration and savevm are the same. :)  In this case IIUC the
failure happens from a CDROM so migration lets you avoid the qcow2
image.  In general I find it easier to manage migration files on disk
than saved snapshots.

Paolo

>> It would be nice to have a mechanism to stop the VM after executing N
>> basic blocks.  Binary search on this value then can help with coming up
>> with a more easily debuggable snapshot, possibly to a point where the
>> difference between pre-patch and post-patch becomes deterministic.
> 
> You can use the monitor and an expect script to say "take a
> snapshot 0.7 seconds into boot", which I've found to be
> a good enough approximation:
> 
> https://translatedcode.wordpress.com/2015/07/06/tricks-for-debugging-qemu-savevm-snapshots/
> 
> thanks
> -- PMM
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window
  2016-06-24  6:36           ` Paolo Bonzini
  2016-06-24  8:12             ` Peter Maydell
@ 2016-06-24 10:42             ` Mark Cave-Ayland
  2016-06-24 12:03               ` Paolo Bonzini
  1 sibling, 1 reply; 20+ messages in thread
From: Mark Cave-Ayland @ 2016-06-24 10:42 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson; +Cc: qemu-devel

On 24/06/16 07:36, Paolo Bonzini wrote:

> On 24/06/2016 05:57, Richard Henderson wrote:
>>
>> Whatever happens, it happens after 10GB of logs, which is simply too
>> much to sift through.  I've tried to narrow it down, but the lack of a
>> hardware tlb refill means that we get hundreds of thousands of Data
>> Access Faults that are simply TLB misses and not the actual Segmentation
>> Fault in question.
>>
>> It doesn't seem to affect other OSes, so I can't imagine what quirk is
>> being exercised in this case.
>>
>> As loath as I am to suggest it, we may have to revert the sparc indirect
>> register patch for the release.
>
> We have more than a month.  If it's reproducible, it can be fixed. :)
>
>> I do now ping the rest of my sparc improvements patchset.  It's
>> completely independent of the use of indirect registers.
>
> Mark, perhaps you can try to use migration to reduce the amount of
> logging?  (Start QEMU with -snapshot, try to stop the vm before it
> fails.  If you succeed, do a "migrate exec:cat>foo.sav" followed by
> "commit"; if you fail, try again).

Yeah, given the improvements that Richard has made, I'd prefer not to 
revert if at all possible. Finally I have some spare time today so I'll 
try and get this down to an easily-testable qcow2 image that can 
reproduce the issue.


ATB,

Mark.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window
  2016-06-24 10:42             ` Mark Cave-Ayland
@ 2016-06-24 12:03               ` Paolo Bonzini
  2016-06-24 12:35                 ` Artyom Tarasenko
  0 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2016-06-24 12:03 UTC (permalink / raw)
  To: Mark Cave-Ayland, Richard Henderson; +Cc: qemu-devel



On 24/06/2016 12:42, Mark Cave-Ayland wrote:
> On 24/06/16 07:36, Paolo Bonzini wrote:
> 
>> On 24/06/2016 05:57, Richard Henderson wrote:
>>>
>>> Whatever happens, it happens after 10GB of logs, which is simply too
>>> much to sift through.  I've tried to narrow it down, but the lack of a
>>> hardware tlb refill means that we get hundreds of thousands of Data
>>> Access Faults that are simply TLB misses and not the actual Segmentation
>>> Fault in question.
>>>
>>> It doesn't seem to affect other OSes, so I can't imagine what quirk is
>>> being exercised in this case.
>>>
>>> As loath as I am to suggest it, we may have to revert the sparc indirect
>>> register patch for the release.
>>
>> We have more than a month.  If it's reproducible, it can be fixed. :)
>>
>>> I do now ping the rest of my sparc improvements patchset.  It's
>>> completely independent of the use of indirect registers.
>>
>> Mark, perhaps you can try to use migration to reduce the amount of
>> logging?  (Start QEMU with -snapshot, try to stop the vm before it
>> fails.  If you succeed, do a "migrate exec:cat>foo.sav" followed by
>> "commit"; if you fail, try again).
> 
> Yeah, given the improvements that Richard has made, I'd prefer not to
> revert if at all possible. Finally I have some spare time today so I'll
> try and get this down to an easily-testable qcow2 image that can
> reproduce the issue.

I've gotten an image that reaches the segmentation fault in about 1
second but I cannot upload it anywhere in the next few hours.  The good
news is that it fails even without a hard disk (so it's a stateless vm)
and with -d nochain -singlestep.  The bad news is that the dump is not
very deterministic and that I failed to create images closer to the failure.

Paolo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window
  2016-06-24 12:03               ` Paolo Bonzini
@ 2016-06-24 12:35                 ` Artyom Tarasenko
  0 siblings, 0 replies; 20+ messages in thread
From: Artyom Tarasenko @ 2016-06-24 12:35 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson; +Cc: Mark Cave-Ayland, qemu-devel

On Fri, Jun 24, 2016 at 2:03 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 24/06/2016 12:42, Mark Cave-Ayland wrote:
>> On 24/06/16 07:36, Paolo Bonzini wrote:
>>
>>> On 24/06/2016 05:57, Richard Henderson wrote:
>>>>
>>>> Whatever happens, it happens after 10GB of logs, which is simply too
>>>> much to sift through.  I've tried to narrow it down, but the lack of a
>>>> hardware tlb refill means that we get hundreds of thousands of Data
>>>> Access Faults that are simply TLB misses and not the actual Segmentation
>>>> Fault in question.
>>>>
>>>> It doesn't seem to affect other OSes, so I can't imagine what quirk is
>>>> being exercised in this case.
>>>>
>>>> As loath as I am to suggest it, we may have to revert the sparc indirect
>>>> register patch for the release.
>>>
>>> We have more than a month.  If it's reproducible, it can be fixed. :)
>>>
>>>> I do now ping the rest of my sparc improvements patchset.  It's
>>>> completely independent of the use of indirect registers.
>>>
>>> Mark, perhaps you can try to use migration to reduce the amount of
>>> logging?  (Start QEMU with -snapshot, try to stop the vm before it
>>> fails.  If you succeed, do a "migrate exec:cat>foo.sav" followed by
>>> "commit"; if you fail, try again).
>>
>> Yeah, given the improvements that Richard has made, I'd prefer not to
>> revert if at all possible. Finally I have some spare time today so I'll
>> try and get this down to an easily-testable qcow2 image that can
>> reproduce the issue.
>
> I've gotten an image that reaches the segmentation fault in about 1
> second but I cannot upload it anywhere in the next few hours.  The good
> news is that it fails even without a hard disk (so it's a stateless vm)
> and with -d nochain -singlestep.  The bad news is that the dump is not
> very deterministic and that I failed to create images closer to the failure.

I have a fix for the bug, will post it shortly. This patch does reveal a bug in
the ldstub implementation, but I'm really surprised we haven't hit it before.

 I observe the following sequence under Solaris:

1. a memory page is mapped without the write permission.
2. the ldstub instruction is executed on this page.
3. ldstub raises an access exception
4b. Because of the bug in the ldstub, a register gets corrupted.
5b. Solaris kernel re-maps the page to have the write access
6b. ldstub is executed again with the corrupted register content
7b. Segmentation fault

With the ldstub fix it goes:
4a. Solaris kernel re-maps the page to have the write access
5a. ldstub is executed again, this time all is good.

There must be another bug in memory access handling.
Maybe cached TBs can be executed with the wrong mem idx?

Artyom



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-06-24 12:36 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-23 18:33 [Qemu-devel] [PULL 0/8] tcg queued patches Richard Henderson
2016-02-23 18:33 ` [Qemu-devel] [PULL 1/8] tcg: Work around clang bug wrt enum ranges, part 2 Richard Henderson
2016-02-23 18:33 ` [Qemu-devel] [PULL 2/8] tcg: Implement indirect memory registers Richard Henderson
2016-02-23 18:33 ` [Qemu-devel] [PULL 3/8] tcg: Allocate indirect_base temporaries in a different order Richard Henderson
2016-02-23 18:33 ` [Qemu-devel] [PULL 4/8] target-sparc: Tidy global register initialization Richard Henderson
2016-02-23 18:33 ` [Qemu-devel] [PULL 5/8] target-sparc: Use global registers for the register window Richard Henderson
2016-06-14 21:52   ` Mark Cave-Ayland
2016-06-16 20:26     ` Richard Henderson
2016-06-16 21:53       ` Mark Cave-Ayland
2016-06-24  3:57         ` Richard Henderson
2016-06-24  6:36           ` Paolo Bonzini
2016-06-24  8:12             ` Peter Maydell
2016-06-24  8:16               ` Paolo Bonzini
2016-06-24 10:42             ` Mark Cave-Ayland
2016-06-24 12:03               ` Paolo Bonzini
2016-06-24 12:35                 ` Artyom Tarasenko
2016-02-23 18:33 ` [Qemu-devel] [PULL 6/8] tcg: Rename tcg-target.c to tcg-target.inc.c Richard Henderson
2016-02-23 18:33 ` [Qemu-devel] [PULL 7/8] scripts/clean-includes: Ignore .inc.c files Richard Henderson
2016-02-23 18:33 ` [Qemu-devel] [PULL 8/8] tcg: Remove unnecessary osdep.h includes from tcg-target.inc.c Richard Henderson
2016-02-25  9:47 ` [Qemu-devel] [PULL 0/8] tcg queued patches Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.