Re: [Qemu-devel] [PATCH v9 07/26] target: [tcg, i386] Refactor init_disas_context

2017-06-26 Thread Lluís Vilanova
Richard Henderson writes:

> On 06/25/2017 02:12 AM, Lluís Vilanova wrote:
>> +DisasContext *dc = container_of(db, DisasContext, base);
>> CPUX86State *env = cpu->env_ptr;
>> -DisasContext dc1, *dc = &dc1;
>> -DisasContextBase *db = &dc1.base;
>> -uint32_t flags;
>> -target_ulong cs_base;
>> -int num_insns;
>> -int max_insns;
>> -
>> -/* generate intermediate code */
>> -db->pc_first = tb->pc;
>> -cs_base = tb->cs_base;
>> -flags = tb->flags;
>> +uint32_t flags = db->tb->flags;
>> +target_ulong cs_base = db->tb->cs_base;

> As a nit, it would be better for the compiler if you keep only one of the two
> pointers {dc,db} live.  That is, once you've used container_of, always use
> dc-> base instead of db.

That's what the previous version did, but Emilio proposed to use both to keep
diffs more readable.

Still, if using both dc/db will confuse the compiler's alias analysis, I can
revert it back to dc->base.

Thanks,
  Lluis



Re: [Qemu-devel] [PATCH v9 07/26] target: [tcg, i386] Refactor init_disas_context

2017-06-26 Thread Richard Henderson

On 06/25/2017 02:12 AM, Lluís Vilanova wrote:

+DisasContext *dc = container_of(db, DisasContext, base);
  CPUX86State *env = cpu->env_ptr;
-DisasContext dc1, *dc = &dc1;
-DisasContextBase *db = &dc1.base;
-uint32_t flags;
-target_ulong cs_base;
-int num_insns;
-int max_insns;
-
-/* generate intermediate code */
-db->pc_first = tb->pc;
-cs_base = tb->cs_base;
-flags = tb->flags;
+uint32_t flags = db->tb->flags;
+target_ulong cs_base = db->tb->cs_base;


As a nit, it would be better for the compiler if you keep only one of the two 
pointers {dc,db} live.  That is, once you've used container_of, always use 
dc->base instead of db.



r~