On 07/08/2017 09:50 PM, Emilio G. Cota wrote:
In order to use glib's binary search tree we embed a helper struct
in TranslationBlock to allow us to compare tb's based on their
tc_ptr as well as their tc_size fields.
Using an anon struct really doesn't help. You're effectively using two
different structs when you pass them in. It "works" simply because in both
cases we happen to know that a given pointer is followed by a uint32_t.
But with that in mind, and knowing that qemu builds with -fno-strict-aliasing,
you could just as well *not* use the anon struct and address the pointer and
the uint32_t within the un-partitioned TranslationBlock. With appropriate
comments in both places, of course.
+ /*
+ * When both sizes are set, we know this isn't a lookup and therefore
+ * the two buffers are non-overlapping: a pointer comparison will do.
+ * This is the most likely case: every TB must be inserted; lookups
+ * are a lot less frequent.
+ */
+ if (likely(a->size && b->size)) {
+ return a->ptr - b->ptr;
+ }
sizeof(gint) < sizeof(ptrdiff_t) on 64-bit hosts. You wouldn't have seen a
problem on x86_64 because of MAX_CODE_GEN_BUFFER_SIZE. I think it would be
better to reduce this to -1/0/1.
static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
{
+ int nb_tbs __attribute__((unused));
+
tb_lock();
/* If it is already been done on request of another CPU,
@@ -894,11 +913,12 @@ static void do_tb_flush(CPUState *cpu, run_on_cpu_data
tb_flush_count)
}
#if defined(DEBUG_TB_FLUSH)
+ nb_tbs = g_tree_nnodes(tcg_ctx.tb_ctx.tb_tree);
printf("qemu: flush code_size=%ld nb_tbs=%d avg_tb_size=%ld\n",
(unsigned long)(tcg_ctx.code_gen_ptr - tcg_ctx.code_gen_buffer),
- tcg_ctx.tb_ctx.nb_tbs, tcg_ctx.tb_ctx.nb_tbs > 0 ?
+ nb_tbs, nb_tbs > 0 ?
((unsigned long)(tcg_ctx.code_gen_ptr - tcg_ctx.code_gen_buffer)) /
- tcg_ctx.tb_ctx.nb_tbs : 0);
+ nb_tbs : 0);
#endif
Variable declaration within braces within the ifdef. Better as size_t or
unsigned long. Using int to count thing on 64-bit hosts always seems like a bug.
r~