diff mbox

[v5,1/9] tcg: pass down TranslationBlock to tcg_code_gen

Message ID 1454597781-18115-2-git-send-email-alex.bennee@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Bennée Feb. 4, 2016, 2:56 p.m. UTC
My later debugging patches need access to the origin PC which is held in
the TranslationBlock structure. Pass down the whole structure as it also
holds the information about the code start point.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v1
 - checkpatch fixes
v5
 - much simplified due to changes since last posting
---
 tcg/tcg.c       |  6 +++---
 tcg/tcg.h       |  2 +-
 translate-all.c | 10 ++++------
 3 files changed, 8 insertions(+), 10 deletions(-)

Comments

Richard Henderson Feb. 4, 2016, 9:24 p.m. UTC | #1
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(&tcg_ctx, gen_code_buf);
> +    gen_code_size = tcg_gen_code(&tcg_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 Feb. 10, 2016, 3:23 p.m. UTC | #2
Richard Henderson <rth@twiddle.net> 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(&tcg_ctx, gen_code_buf);
>> +    gen_code_size = tcg_gen_code(&tcg_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
diff mbox

Patch

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 3ce02dc..0101cc1 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2316,7 +2316,7 @@  void tcg_dump_op_count(FILE *f, fprintf_function cpu_fprintf)
 #endif
 
 
-int tcg_gen_code(TCGContext *s, tcg_insn_unit *gen_code_buf)
+int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
 {
     int i, oi, oi_next, num_insns;
 
@@ -2375,8 +2375,8 @@  int tcg_gen_code(TCGContext *s, tcg_insn_unit *gen_code_buf)
 
     tcg_reg_alloc_start(s);
 
-    s->code_buf = gen_code_buf;
-    s->code_ptr = gen_code_buf;
+    s->code_buf = tb->tc_ptr;
+    s->code_ptr = tb->tc_ptr;
 
     tcg_out_tb_init(s);
 
diff --git a/tcg/tcg.h b/tcg/tcg.h
index a696922..9a18ee4 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -626,7 +626,7 @@  void tcg_context_init(TCGContext *s);
 void tcg_prologue_init(TCGContext *s);
 void tcg_func_start(TCGContext *s);
 
-int tcg_gen_code(TCGContext *s, tcg_insn_unit *gen_code_buf);
+int tcg_gen_code(TCGContext *s, TranslationBlock *tb);
 
 void tcg_set_frame(TCGContext *s, int reg, intptr_t start, intptr_t size);
 
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;
     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(&tcg_ctx, gen_code_buf);
+    gen_code_size = tcg_gen_code(&tcg_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 */