[Qemu-devel] [PATCH qom-cpu 0/5] TCG code generation performance fix

[Qemu-devel] [PATCH qom-cpu 0/5] TCG code generation performance fix

[Andreas Färber]

Hello,

As Richard explained, the purpose of having separate gen_intermediate_code()
and gen_intermediate_code_pc() functions per target is to compile-optimize
gen_intermediate_code_internal() for the non-_pc case.

Multiple targets were using static rather than static inline though, fix this.

I've split these off from my refactorings so that we can backport them to stable,
and I'm rebasing my argument refactoring patches on top.

No actual performance changes have been benchmarked, these changes serve more
to align our targets as clear examples for new targets such as rl78 and bfin.

Regards,
Andreas

Cc: Richard Henderson <rth@twiddle.net>
Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> (cris, mblaze)
Cc: Michael Walle <michael@walle.cc> (lm32)
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com> (mblaze)
Cc: Anthony Green <green@moxielogic.com> (moxie)
Cc: Max Filippov <jcmvbkbc@gmail.com> (xtensa)
Cc: qemu-stable@nongnu.org

Cc: Mike Frysinger <vapier@gentoo.org> (bfin)

Andreas Färber (5):
  target-cris: gen_intermediate_code_internal() should be inlined
  target-lm32: gen_intermediate_code_internal() should be inline
  target-microblaze: gen_intermediate_code_internal() should be inline
  target-moxie: gen_intermediate_code_internal() should be inline
  target-xtensa: gen_intermediate_code_internal() should be inline

 target-cris/translate.c       | 2 +-
 target-lm32/translate.c       | 5 +++--
 target-microblaze/translate.c | 2 +-
 target-moxie/translate.c      | 2 +-
 target-xtensa/translate.c     | 5 +++--
 5 files changed, 9 insertions(+), 7 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH qom-cpu 1/5] target-cris: gen_intermediate_code_internal() should be inlined

[Andreas Färber]

Reported-by: Richard Henderson <rth@twiddle.net>
Cc: qemu-stable@nongnu.org
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-cris/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-cris/translate.c b/target-cris/translate.c
index 09d0d2b..ee9ae22 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3161,7 +3161,7 @@ static void check_breakpoint(CPUCRISState *env, DisasContext *dc)
  */
 
 /* generate intermediate code for basic block 'tb'.  */
-static void
+static inline void
 gen_intermediate_code_internal(CPUCRISState *env, TranslationBlock *tb,
                                int search_pc)
 {
-- 
1.8.1.4

[Qemu-devel] [PATCH qom-cpu 2/5] target-lm32: gen_intermediate_code_internal() should be inline

[Andreas Färber]

Reported-by: Richard Henderson <rth@twiddle.net>
Cc: qemu-stable@nongnu.org
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-lm32/translate.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index 227a801..7d82dc7 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1011,8 +1011,9 @@ static void check_breakpoint(CPULM32State *env, DisasContext *dc)
 }
 
 /* generate intermediate code for basic block 'tb'.  */
-static void gen_intermediate_code_internal(CPULM32State *env,
-        TranslationBlock *tb, int search_pc)
+static inline
+void gen_intermediate_code_internal(CPULM32State *env,
+                                    TranslationBlock *tb, int search_pc)
 {
     struct DisasContext ctx, *dc = &ctx;
     uint16_t *gen_opc_end;
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH qom-cpu 2/5] target-lm32: gen_intermediate_code_internal() should be inline

[Michael Walle]

Am Dienstag, 2. Juli 2013, 21:31:33 schrieb Andreas Färber:
> Reported-by: Richard Henderson <rth@twiddle.net>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  target-lm32/translate.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/target-lm32/translate.c b/target-lm32/translate.c
> index 227a801..7d82dc7 100644
> --- a/target-lm32/translate.c
> +++ b/target-lm32/translate.c
> @@ -1011,8 +1011,9 @@ static void check_breakpoint(CPULM32State *env,
> DisasContext *dc) }
> 
>  /* generate intermediate code for basic block 'tb'.  */
> -static void gen_intermediate_code_internal(CPULM32State *env,
> -        TranslationBlock *tb, int search_pc)
> +static inline
> +void gen_intermediate_code_internal(CPULM32State *env,
> +                                    TranslationBlock *tb, int search_pc)
>  {
>      struct DisasContext ctx, *dc = &ctx;
>      uint16_t *gen_opc_end;

Acked-by: Michael Walle <michael@walle.cc>

-- 
michael

[Qemu-devel] [PATCH qom-cpu 3/5] target-microblaze: gen_intermediate_code_internal() should be inline

[Andreas Färber]

Reported-by: Richard Henderson <rth@twiddle.net>
Cc: qemu-stable@nongnu.org
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-microblaze/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index 54f439f..27da4bf 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -1737,7 +1737,7 @@ static void check_breakpoint(CPUMBState *env, DisasContext *dc)
 }
 
 /* generate intermediate code for basic block 'tb'.  */
-static void
+static inline void
 gen_intermediate_code_internal(CPUMBState *env, TranslationBlock *tb,
                                int search_pc)
 {
-- 
1.8.1.4

[Qemu-devel] [PATCH qom-cpu 4/5] target-moxie: gen_intermediate_code_internal() should be inline

[Andreas Färber]

Reported-by: Richard Henderson <rth@twiddle.net>
Cc: qemu-stable@nongnu.org
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-moxie/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-moxie/translate.c b/target-moxie/translate.c
index b0ae38a..664d359 100644
--- a/target-moxie/translate.c
+++ b/target-moxie/translate.c
@@ -820,7 +820,7 @@ static int decode_opc(MoxieCPU *cpu, DisasContext *ctx)
 }
 
 /* generate intermediate code for basic block 'tb'.  */
-static void
+static inline void
 gen_intermediate_code_internal(MoxieCPU *cpu, TranslationBlock *tb,
                                bool search_pc)
 {
-- 
1.8.1.4

[Qemu-devel] [PATCH qom-cpu 5/5] target-xtensa: gen_intermediate_code_internal() should be inline

[Andreas Färber]

Reported-by: Richard Henderson <rth@twiddle.net>
Cc: qemu-stable@nongnu.org
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-xtensa/translate.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index dcb90a5..e384812 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -2875,8 +2875,9 @@ static void gen_ibreak_check(CPUXtensaState *env, DisasContext *dc)
     }
 }
 
-static void gen_intermediate_code_internal(
-        CPUXtensaState *env, TranslationBlock *tb, int search_pc)
+static inline
+void gen_intermediate_code_internal(CPUXtensaState *env,
+                                    TranslationBlock *tb, int search_pc)
 {
     DisasContext dc;
     int insn_count = 0;
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH qom-cpu 0/5] TCG code generation performance fix

[Richard Henderson]

On 07/02/2013 12:31 PM, Andreas Färber wrote:
> Hello,
> 
> As Richard explained, the purpose of having separate gen_intermediate_code()
> and gen_intermediate_code_pc() functions per target is to compile-optimize
> gen_intermediate_code_internal() for the non-_pc case.
> 
> Multiple targets were using static rather than static inline though, fix this.
> 
> I've split these off from my refactorings so that we can backport them to stable,
> and I'm rebasing my argument refactoring patches on top.
> 
> No actual performance changes have been benchmarked, these changes serve more
> to align our targets as clear examples for new targets such as rl78 and bfin.

All:
Reviewed-by: Richard Henderson <rth@twiddle.net>

Although we should probably do some benchmarking at some point to see if
the duplicated code paths really do improve things over, say, unlikely().
But failing that we should at least have conformity of implementation.


r~

Re: [Qemu-devel] [PATCH qom-cpu 0/5] TCG code generation performance fix

[Andreas Färber]

Am 02.07.2013 21:59, schrieb Richard Henderson:
> On 07/02/2013 12:31 PM, Andreas Färber wrote:
>> Hello,
>>
>> As Richard explained, the purpose of having separate gen_intermediate_code()
>> and gen_intermediate_code_pc() functions per target is to compile-optimize
>> gen_intermediate_code_internal() for the non-_pc case.
>>
>> Multiple targets were using static rather than static inline though, fix this.
>>
>> I've split these off from my refactorings so that we can backport them to stable,
>> and I'm rebasing my argument refactoring patches on top.
>>
>> No actual performance changes have been benchmarked, these changes serve more
>> to align our targets as clear examples for new targets such as rl78 and bfin.
> 
> All:
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> 
> Although we should probably do some benchmarking at some point to see if
> the duplicated code paths really do improve things over, say, unlikely().

I did wonder about unlikely() last night, but I thought that's just a
branch optimization whereas inline might avoid some branches in the
first place.

> But failing that we should at least have conformity of implementation.

Thanks, applied to qom-cpu (moving log_cpu_state to qom-cpu-next):
https://github.com/afaerber/qemu-cpu/commits/qom-cpu
(If maintainers want to ack/nack, please do.)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg