Re: [Qemu-devel] [PATCH v5 1/9] tcg: pass down TranslationBlock to tcg_code_gen

2016-02-10 Thread Alex Bennée

Richard Henderson  writes:

> On 02/05/2016 01:56 AM, Alex Bennée wrote:
>> diff --git a/translate-all.c b/translate-all.c
>> index ab61fac..dce00d5 100644
>> --- a/translate-all.c
>> +++ b/translate-all.c
>> @@ -1055,7 +1055,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>>   TranslationBlock *tb;
>>   tb_page_addr_t phys_pc, phys_page2;
>>   target_ulong virt_page2;
>> -tcg_insn_unit *gen_code_buf;
>>   int gen_code_size, search_size;
>>   #ifdef CONFIG_PROFILER
>>   int64_t ti;
>> @@ -1078,8 +1077,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>>   tcg_ctx.tb_ctx.tb_invalidated_flag = 1;
>>   }
>>
>> -gen_code_buf = tcg_ctx.code_gen_ptr;
>> -tb->tc_ptr = gen_code_buf;
>> +tb->tc_ptr = tcg_ctx.code_gen_ptr;
>
> Why get rid of the gen_code_buf variable?  You're forcing the compiler to keep
> reloading the value from memory.
>
> Certainly that's not relevant to passing down TB to tcg_gen_code, and is a
> separate change that ought to be separately defended.

Fixed. Thanks.

>
>
> r~
>
>>   tb->cs_base = cs_base;
>>   tb->flags = flags;
>>   tb->cflags = cflags;
>> @@ -1119,11 +1117,11 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>>  the tcg optimization currently hidden inside tcg_gen_code.  All
>>  that should be required is to flush the TBs, allocate a new TB,
>>  re-initialize it per above, and re-do the actual code generation.  
>> */
>> -gen_code_size = tcg_gen_code(_ctx, gen_code_buf);
>> +gen_code_size = tcg_gen_code(_ctx, tb);
>>   if (unlikely(gen_code_size < 0)) {
>>   goto buffer_overflow;
>>   }
>> -search_size = encode_search(tb, (void *)gen_code_buf + gen_code_size);
>> +search_size = encode_search(tb, (void *)tb->tc_ptr + gen_code_size);
>>   if (unlikely(search_size < 0)) {
>>   goto buffer_overflow;
>>   }
>> @@ -1145,7 +1143,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>>   #endif
>>
>>   tcg_ctx.code_gen_ptr = (void *)
>> -ROUND_UP((uintptr_t)gen_code_buf + gen_code_size + search_size,
>> +ROUND_UP((uintptr_t)tb->tc_ptr + gen_code_size + search_size,
>>CODE_GEN_ALIGN);
>>
>>   /* check next page if needed */
>>


--
Alex Bennée



Re: [Qemu-devel] [PATCH v5 1/9] tcg: pass down TranslationBlock to tcg_code_gen

2016-02-04 Thread Richard Henderson

On 02/05/2016 01:56 AM, Alex Bennée wrote:

diff --git a/translate-all.c b/translate-all.c
index ab61fac..dce00d5 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1055,7 +1055,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
  TranslationBlock *tb;
  tb_page_addr_t phys_pc, phys_page2;
  target_ulong virt_page2;
-tcg_insn_unit *gen_code_buf;
  int gen_code_size, search_size;
  #ifdef CONFIG_PROFILER
  int64_t ti;
@@ -1078,8 +1077,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
  tcg_ctx.tb_ctx.tb_invalidated_flag = 1;
  }

-gen_code_buf = tcg_ctx.code_gen_ptr;
-tb->tc_ptr = gen_code_buf;
+tb->tc_ptr = tcg_ctx.code_gen_ptr;


Why get rid of the gen_code_buf variable?  You're forcing the compiler to keep 
reloading the value from memory.


Certainly that's not relevant to passing down TB to tcg_gen_code, and is a 
separate change that ought to be separately defended.



r~


  tb->cs_base = cs_base;
  tb->flags = flags;
  tb->cflags = cflags;
@@ -1119,11 +1117,11 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
 the tcg optimization currently hidden inside tcg_gen_code.  All
 that should be required is to flush the TBs, allocate a new TB,
 re-initialize it per above, and re-do the actual code generation.  */
-gen_code_size = tcg_gen_code(_ctx, gen_code_buf);
+gen_code_size = tcg_gen_code(_ctx, tb);
  if (unlikely(gen_code_size < 0)) {
  goto buffer_overflow;
  }
-search_size = encode_search(tb, (void *)gen_code_buf + gen_code_size);
+search_size = encode_search(tb, (void *)tb->tc_ptr + gen_code_size);
  if (unlikely(search_size < 0)) {
  goto buffer_overflow;
  }
@@ -1145,7 +1143,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
  #endif

  tcg_ctx.code_gen_ptr = (void *)
-ROUND_UP((uintptr_t)gen_code_buf + gen_code_size + search_size,
+ROUND_UP((uintptr_t)tb->tc_ptr + gen_code_size + search_size,
   CODE_GEN_ALIGN);

  /* check next page if needed */