[Qemu-devel] [PATCH v2 0/3] target/riscv: use tcg_lookup_and_goto_ptr

[Qemu-devel] [PATCH v2 0/3] target/riscv: use tcg_lookup_and_goto_ptr

[Emilio G. Cota]

Changes wrt v1: changed patch 3 as suggested by Richard. Also
added his R-b's.

You can fetch this series from:
  https://github.com/cota/qemu/tree/riscv-lookup_ptr-v2

Thanks,

		Emilio

[Qemu-devel] [PATCH v2 1/3] target/riscv: optimize cross-page direct jumps in softmmu

[Emilio G. Cota]

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/riscv/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0b6be74f2d..ec2988b4f6 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -135,7 +135,7 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
         if (ctx->base.singlestep_enabled) {
             gen_exception_debug();
         } else {
-            tcg_gen_exit_tb(NULL, 0);
+            tcg_gen_lookup_and_goto_ptr();
         }
     }
 }
-- 
2.17.1

[Qemu-devel] [PATCH v2 2/3] target/riscv: optimize indirect branches

[Emilio G. Cota]

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/riscv/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index ec2988b4f6..66a80ca772 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -548,7 +548,7 @@ static void gen_jalr(CPURISCVState *env, DisasContext *ctx, uint32_t opc,
         if (rd != 0) {
             tcg_gen_movi_tl(cpu_gpr[rd], ctx->pc_succ_insn);
         }
-        tcg_gen_exit_tb(NULL, 0);
+        tcg_gen_lookup_and_goto_ptr();
 
         if (misaligned) {
             gen_set_label(misaligned);
-- 
2.17.1

[Qemu-devel] [PATCH v2 3/3] target/riscv: call gen_goto_tb on DISAS_TOO_MANY

[Emilio G. Cota]

Performance impact of this and the previous commits, measured with
the very-easy-to-cross-compile rv8-bench:
  https://github.com/rv8-io/rv8-bench

Host: Intel(R) Core(TM) i7-4790K CPU @ 4.00GHz

- Key:
  before: master
  after1,2,3: the 3 commits in this series (i.e. 3 is this commit)

- User-mode:

 bench      before  after1  after2  after3  final speedup
---------------------------------------------------------
 aes        1.12s   1.12s   1.10s   1.00s   1.12
 bigint     0.78s   0.78s   0.78s   0.78s   1
 dhrystone  0.96s   0.97s   0.49s   0.49s   1.9591837
 miniz      1.94s   1.94s   1.88s   1.86s   1.0430108
 norx       0.51s   0.51s   0.49s   0.48s   1.0625
 primes     0.85s   0.85s   0.84s   0.84s   1.0119048
 qsort      4.87s   4.88s   1.86s   1.86s   2.6182796
 sha512     0.76s   0.77s   0.64s   0.64s   1.1875

(after1 only applies to softmmu, so no surprises here)

- Full-system (fedora):

 bench      before  after1  after2  after3  final speedup
---------------------------------------------------------
 aes        2.68s   2.54s   2.60s   2.34s   1.1452991
 bigint     1.61s   1.56s   1.55s   1.64s   0.98170732
 dhrystone  1.78s   1.67s   1.25s   1.24s   1.4354839
 miniz      3.53s   3.35s   3.28s   3.35s   1.0537313
 norx       1.13s   1.09s   1.07s   1.06s   1.0660377
 primes     15.37s  15.41s  15.20s  15.37s  1
 qsort      7.20s   6.71s   3.85s   3.96s   1.8181818
 sha512     1.07s   1.04s   0.90s   0.90s   1.1888889

SoftMMU slows things down, so the numbers are less sensitive.
Cross-page jumps improve things a little bit, though.

Note that I'm not showing here averages, just results from a
single run, so with primes there isn't much to worry about.

Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/riscv/translate.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 66a80ca772..18d7b6d147 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1868,12 +1868,7 @@ static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
 
     switch (ctx->base.is_jmp) {
     case DISAS_TOO_MANY:
-        tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
-        if (ctx->base.singlestep_enabled) {
-            gen_exception_debug();
-        } else {
-            tcg_gen_exit_tb(NULL, 0);
-        }
+        gen_goto_tb(ctx, 0, ctx->base.pc_next);
         break;
     case DISAS_NORETURN:
         break;
-- 
2.17.1

Re: [Qemu-devel] [PATCH v2 3/3] target/riscv: call gen_goto_tb on DISAS_TOO_MANY

[Richard Henderson]

On 08/10/2018 10:39 AM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  target/riscv/translate.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 66a80ca772..18d7b6d147 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1868,12 +1868,7 @@ static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
>  
>      switch (ctx->base.is_jmp) {
>      case DISAS_TOO_MANY:
> -        tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
> -        if (ctx->base.singlestep_enabled) {
> -            gen_exception_debug();
> -        } else {
> -            tcg_gen_exit_tb(NULL, 0);
> -        }
> +        gen_goto_tb(ctx, 0, ctx->base.pc_next);
>          break;

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v2 0/3] target/riscv: use tcg_lookup_and_goto_ptr

[Emilio G. Cota]

On Fri, Aug 10, 2018 at 13:39:38 -0400, Emilio G. Cota wrote:
> Changes wrt v1: changed patch 3 as suggested by Richard. Also
> added his R-b's.
> 
> You can fetch this series from:
>   https://github.com/cota/qemu/tree/riscv-lookup_ptr-v2

RISC-V maintainers: any plans of picking this up for 3.1?

(Cc'ing Alistair since he recently submitted some riscv-related
pull requests; BTW Alistair, should you be listed as a RISC-V
maintainer?)

Thanks,

		Emilio

Re: [Qemu-devel] [PATCH v2 0/3] target/riscv: use tcg_lookup_and_goto_ptr

[Alistair Francis]

On Fri, Aug 31, 2018 at 3:22 PM, Emilio G. Cota <cota@braap.org> wrote:
> On Fri, Aug 10, 2018 at 13:39:38 -0400, Emilio G. Cota wrote:
>> Changes wrt v1: changed patch 3 as suggested by Richard. Also
>> added his R-b's.
>>
>> You can fetch this series from:
>>   https://github.com/cota/qemu/tree/riscv-lookup_ptr-v2
>
> RISC-V maintainers: any plans of picking this up for 3.1?

Thanks for CCing me.

I think Michael must be really busy, I haven't seen any activity for
awhile. I'm going to send a PR with everything that has been reviewed
and applies cleanly.

Alistair

>
> (Cc'ing Alistair since he recently submitted some riscv-related
> pull requests; BTW Alistair, should you be listed as a RISC-V
> maintainer?)
>
> Thanks,
>
>                 Emilio
>

Re: [Qemu-devel] [PATCH v2 0/3] target/riscv: use tcg_lookup_and_goto_ptr

[Emilio G. Cota]

On Tue, Sep 04, 2018 at 13:18:28 -0700, Alistair Francis wrote:
> On Fri, Aug 31, 2018 at 3:22 PM, Emilio G. Cota <cota@braap.org> wrote:
> > On Fri, Aug 10, 2018 at 13:39:38 -0400, Emilio G. Cota wrote:
> >> Changes wrt v1: changed patch 3 as suggested by Richard. Also
> >> added his R-b's.
> >>
> >> You can fetch this series from:
> >>   https://github.com/cota/qemu/tree/riscv-lookup_ptr-v2
> >
> > RISC-V maintainers: any plans of picking this up for 3.1?
> 
> Thanks for CCing me.
> 
> I think Michael must be really busy, I haven't seen any activity for
> awhile. I'm going to send a PR with everything that has been reviewed
> and applies cleanly.

Thanks!

Did patch 1 not apply cleanly? I see it's not part of the pullreq
you just sent.

I'd like the three patches to go in in the original order, since
the perf numbers in patch 3's commit log are laid out assuming
that is the case.

Cheers,
	
		Emilio

Re: [Qemu-devel] [PATCH v2 0/3] target/riscv: use tcg_lookup_and_goto_ptr

[Alistair Francis]

On Tue, Sep 4, 2018 at 4:39 PM, Emilio G. Cota <cota@braap.org> wrote:
> On Tue, Sep 04, 2018 at 13:18:28 -0700, Alistair Francis wrote:
>> On Fri, Aug 31, 2018 at 3:22 PM, Emilio G. Cota <cota@braap.org> wrote:
>> > On Fri, Aug 10, 2018 at 13:39:38 -0400, Emilio G. Cota wrote:
>> >> Changes wrt v1: changed patch 3 as suggested by Richard. Also
>> >> added his R-b's.
>> >>
>> >> You can fetch this series from:
>> >>   https://github.com/cota/qemu/tree/riscv-lookup_ptr-v2
>> >
>> > RISC-V maintainers: any plans of picking this up for 3.1?
>>
>> Thanks for CCing me.
>>
>> I think Michael must be really busy, I haven't seen any activity for
>> awhile. I'm going to send a PR with everything that has been reviewed
>> and applies cleanly.
>
> Thanks!
>
> Did patch 1 not apply cleanly? I see it's not part of the pullreq
> you just sent.
>
> I'd like the three patches to go in in the original order, since
> the perf numbers in patch 3's commit log are laid out assuming
> that is the case.

The content of patch 1 was applied in this commit:

commit 07ea28b41830f946de3841b0ac61a3413679feb9
Author: Richard Henderson <richard.henderson@linaro.org>
Date:   Wed May 30 18:06:23 2018 -0700

    tcg: Pass tb and index to tcg_gen_exit_tb separately

    Do the cast to uintptr_t within the helper, so that the compiler
    can type check the pointer argument.  We can also do some more
    sanity checking of the index argument.

    Reviewed-by: Laurent Vivier <laurent@vivier.eu>
    Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

https://github.com/qemu/qemu/commit/07ea28b41830f946de3841b0ac61a3413679feb9

so unfortunately the patch no longer applies.

Alistair

>
> Cheers,
>
>                 Emilio

Re: [Qemu-devel] [PATCH v2 0/3] target/riscv: use tcg_lookup_and_goto_ptr

[Emilio G. Cota]

On Tue, Sep 04, 2018 at 16:42:32 -0700, Alistair Francis wrote:
> On Tue, Sep 4, 2018 at 4:39 PM, Emilio G. Cota <cota@braap.org> wrote:
> > On Tue, Sep 04, 2018 at 13:18:28 -0700, Alistair Francis wrote:
> >> On Fri, Aug 31, 2018 at 3:22 PM, Emilio G. Cota <cota@braap.org> wrote:
> >> > On Fri, Aug 10, 2018 at 13:39:38 -0400, Emilio G. Cota wrote:
> >> >> Changes wrt v1: changed patch 3 as suggested by Richard. Also
> >> >> added his R-b's.
> >> >>
> >> >> You can fetch this series from:
> >> >>   https://github.com/cota/qemu/tree/riscv-lookup_ptr-v2
> >> >
> >> > RISC-V maintainers: any plans of picking this up for 3.1?
> >>
> >> Thanks for CCing me.
> >>
> >> I think Michael must be really busy, I haven't seen any activity for
> >> awhile. I'm going to send a PR with everything that has been reviewed
> >> and applies cleanly.
> >
> > Thanks!
> >
> > Did patch 1 not apply cleanly? I see it's not part of the pullreq
> > you just sent.
> >
> > I'd like the three patches to go in in the original order, since
> > the perf numbers in patch 3's commit log are laid out assuming
> > that is the case.
> 
> The content of patch 1 was applied in this commit:
> 
> commit 07ea28b41830f946de3841b0ac61a3413679feb9
> Author: Richard Henderson <richard.henderson@linaro.org>
> Date:   Wed May 30 18:06:23 2018 -0700
> 
>     tcg: Pass tb and index to tcg_gen_exit_tb separately
> 
>     Do the cast to uintptr_t within the helper, so that the compiler
>     can type check the pointer argument.  We can also do some more
>     sanity checking of the index argument.
> 
>     Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>     Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> 
> https://github.com/qemu/qemu/commit/07ea28b41830f946de3841b0ac61a3413679feb9
> 
> so unfortunately the patch no longer applies.

If I rebase the 3 patches on top of the current master (19b599f7),
the 3 patches apply cleanly, including patch 1:

--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -135,7 +135,7 @@ static void gen_goto_tb(DisasContext *ctx, int n, targ
         if (ctx->base.singlestep_enabled) {
             gen_exception_debug();
         } else {
-            tcg_gen_exit_tb(NULL, 0);
+            tcg_gen_lookup_and_goto_ptr();
         }
     }
 }

Did you apply the patches manually or pull from a branch?
The latest branch I have on this is:
   https://github.com/cota/qemu/tree/riscv-lookup_ptr-v2
(although note that that branch doesn't have Richard's
R-b tag for patch 3)

Thanks,

		Emilio

Re: [Qemu-devel] [PATCH v2 0/3] target/riscv: use tcg_lookup_and_goto_ptr

[Alistair Francis]

On Tue, Sep 4, 2018 at 6:07 PM, Emilio G. Cota <cota@braap.org> wrote:
> On Tue, Sep 04, 2018 at 16:42:32 -0700, Alistair Francis wrote:
>> On Tue, Sep 4, 2018 at 4:39 PM, Emilio G. Cota <cota@braap.org> wrote:
>> > On Tue, Sep 04, 2018 at 13:18:28 -0700, Alistair Francis wrote:
>> >> On Fri, Aug 31, 2018 at 3:22 PM, Emilio G. Cota <cota@braap.org> wrote:
>> >> > On Fri, Aug 10, 2018 at 13:39:38 -0400, Emilio G. Cota wrote:
>> >> >> Changes wrt v1: changed patch 3 as suggested by Richard. Also
>> >> >> added his R-b's.
>> >> >>
>> >> >> You can fetch this series from:
>> >> >>   https://github.com/cota/qemu/tree/riscv-lookup_ptr-v2
>> >> >
>> >> > RISC-V maintainers: any plans of picking this up for 3.1?
>> >>
>> >> Thanks for CCing me.
>> >>
>> >> I think Michael must be really busy, I haven't seen any activity for
>> >> awhile. I'm going to send a PR with everything that has been reviewed
>> >> and applies cleanly.
>> >
>> > Thanks!
>> >
>> > Did patch 1 not apply cleanly? I see it's not part of the pullreq
>> > you just sent.
>> >
>> > I'd like the three patches to go in in the original order, since
>> > the perf numbers in patch 3's commit log are laid out assuming
>> > that is the case.
>>
>> The content of patch 1 was applied in this commit:
>>
>> commit 07ea28b41830f946de3841b0ac61a3413679feb9
>> Author: Richard Henderson <richard.henderson@linaro.org>
>> Date:   Wed May 30 18:06:23 2018 -0700
>>
>>     tcg: Pass tb and index to tcg_gen_exit_tb separately
>>
>>     Do the cast to uintptr_t within the helper, so that the compiler
>>     can type check the pointer argument.  We can also do some more
>>     sanity checking of the index argument.
>>
>>     Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>     Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>
>> https://github.com/qemu/qemu/commit/07ea28b41830f946de3841b0ac61a3413679feb9
>>
>> so unfortunately the patch no longer applies.
>
> If I rebase the 3 patches on top of the current master (19b599f7),
> the 3 patches apply cleanly, including patch 1:
>
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -135,7 +135,7 @@ static void gen_goto_tb(DisasContext *ctx, int n, targ
>          if (ctx->base.singlestep_enabled) {
>              gen_exception_debug();
>          } else {
> -            tcg_gen_exit_tb(NULL, 0);
> +            tcg_gen_lookup_and_goto_ptr();
>          }
>      }
>  }
>
> Did you apply the patches manually or pull from a branch?
> The latest branch I have on this is:
>    https://github.com/cota/qemu/tree/riscv-lookup_ptr-v2
> (although note that that branch doesn't have Richard's
> R-b tag for patch 3)

I thought I cherry picked them directly from your branch and I saw
conflicts. It seems to be working now, maybe I cherry picked the wrong
patch. I'll update the PR.

Thanks for double checking for me.

Alistair

>
> Thanks,
>
>                 Emilio

Re: [Qemu-devel] [PATCH v2 0/3] target/riscv: use tcg_lookup_and_goto_ptr

[Palmer Dabbelt]

On Fri, 31 Aug 2018 15:22:49 PDT (-0700), cota@braap.org wrote:
> On Fri, Aug 10, 2018 at 13:39:38 -0400, Emilio G. Cota wrote:
>> Changes wrt v1: changed patch 3 as suggested by Richard. Also
>> added his R-b's.
>>
>> You can fetch this series from:
>>   https://github.com/cota/qemu/tree/riscv-lookup_ptr-v2
>
> RISC-V maintainers: any plans of picking this up for 3.1?

Michael has a better handle on the QEMU patch queue than I do, but I'm starting 
to free up some time to actually work on this stuff.  He'll be in town next 
week so we'll try to figure out our story for QEMU 3.1.

Sorry for being a bit out of it, we've been super busy with some internal 
projects.

> (Cc'ing Alistair since he recently submitted some riscv-related
> pull requests; BTW Alistair, should you be listed as a RISC-V
> maintainer?)

I'm not opposed to Alistair being a maintainer, I'll talk it over with the 
other guys.  It'd be great to have someone active and not at SiFive on QEMU, as 
when we get busy we tend to all get busy :).