Re: [PATCH v2] target/riscv: Use a direct cast for better performance

[LIU Zhiwei]

On 2023/10/9 20:53, Richard W.M. Jones wrote:

On Mon, Oct 09, 2023 at 08:36:28PM +0800, LIU Zhiwei wrote:

On 2023/10/9 5:50, Richard W.M. Jones wrote:

RISCV_CPU(cs) uses a checked cast.  When QOM cast debugging is enabled
this adds about 5% total overhead when emulating RV64 on x86-64 host.

Using a RISC-V guest with 16 vCPUs, 16 GB of guest RAM, virtio-blk
disk.  The guest has a copy of the qemu source tree.  The test
involves compiling the qemu source tree with 'make clean; time make -j16'.

Before making this change the compile step took 449 & 447 seconds over
two consecutive runs.

After making this change, 428 & 422 seconds.

The saving is about 5%.

Thanks: Paolo Bonzini
Signed-off-by: Richard W.M. Jones <rjo...@redhat.com>
Reviewed-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
---
  target/riscv/cpu_helper.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 3a02079290..479d9863ae 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -66,7 +66,11 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
                            uint64_t *cs_base, uint32_t *pflags)
  {
      CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
+    /*
+     * Using the checked cast RISCV_CPU(cs) imposes ~ 5% overhead when
+     * QOM cast debugging is enabled, so use a direct cast instead.
+     */
+    RISCVCPU *cpu = (RISCVCPU *)cs;

This function is very hot. Maybe we should cache the tbflags instead
of calculate it here. Otherwise,

This function is indeed very hot, taking over 20% of total host time
in my guest stress test.

How would we cache the flags?  AIUI they simply depend on machine
state and we must recalculate them either when the machine state
changes (sprinkle "update_tbflags" everywhere)

Yes, we should do in this way.

  or here.  If you have
any suggestions I can try things.

I think it exceeds this patch.

Reviewed-by: LIU Zhiwei <zhiwei_...@linux.alibaba.com>

I posted a v3 based on Philippe's feedback.

OK.

Thanks
Zhiwei

Rich.

Zhiwei

      RISCVExtStatus fs, vs;
      uint32_t flags = 0;