| Message ID | 20211223201812.846495-3-danielhb413@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Re-write PPC64 PMU instruction count using TCG Ops | expand |
On 12/23/21 12:18 PM, Daniel Henrique Barboza wrote: > The code that increments a PMC is repetitive: check if a given register > has a bit/mask set or cleared and increment the counter. > > inc_spr_if_cond() will help deal with this repetition. This patch also > gives a sample of how the function works by incrementing PMC5, which is > supposed to be incremented only if MMCR0_FC56 is not set. > > We've also removing the call from the helper since that would cause > PMC5 to be counted twice. > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > target/ppc/power8-pmu-insn-cnt.c.inc | 43 ++++++++++++++++++++++------ > 1 file changed, 34 insertions(+), 9 deletions(-) > > diff --git a/target/ppc/power8-pmu-insn-cnt.c.inc b/target/ppc/power8-pmu-insn-cnt.c.inc > index 6cdf2d2d88..3cfb801c69 100644 > --- a/target/ppc/power8-pmu-insn-cnt.c.inc > +++ b/target/ppc/power8-pmu-insn-cnt.c.inc > @@ -10,6 +10,38 @@ > * See the COPYING file in the top-level directory. > */ > > +#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) > +/* > + * This function increments a SPR 'spr' by 'inc_val' if a given > + * register 'reg' has a bitmask 'mask' set (cond = TCG_COND_EQ) or > + * not set (TCG_COND_NE). > + */ > +static void inc_spr_if_cond(int reg, uint64_t mask, TCGCond cond, > + int spr, int inc_val) > +{ > + TCGCond exit_cond = tcg_invert_cond(cond); > + TCGLabel *l_exit; > + TCGv t0, t1; > + > + l_exit = gen_new_label(); > + > + t0 = tcg_temp_new(); > + gen_load_spr(t0, reg); > + tcg_gen_andi_tl(t0, t0, mask); > + tcg_gen_brcondi_tl(exit_cond, t0, mask, l_exit); When testing a single bit, compare against 0, not the bit. > + t1 = tcg_temp_new(); > + gen_load_spr(t1, spr); > + tcg_gen_addi_tl(t1, t1, inc_val); > + gen_store_spr(spr, t1); It will probably perform better to make this a true conditional add. I.e. gen_load_spr(t0, spr); tcg_gen_addi_tl(t1, t0, inc); tcg_gen_movcond_tl(cond, t0, reg, zero, t1, t0); gen_store_spr(spr, t0); > - /* > - * The PMU insns_inc() helper stops the internal PMU timer if a > - * counter overflows happens. In that case, if the guest is > - * running with icount and we do not handle it beforehand, > - * the helper can trigger a 'bad icount read'. > - */ > - gen_icount_io_start(ctx); Removing this is incorrect. > - gen_helper_insns_inc(cpu_env, tcg_constant_i32(ctx->base.num_insns)); > + inc_spr_if_cond(SPR_POWER_MMCR0, MMCR0_FC56, TCG_COND_NE, > + SPR_POWER_PMC5, ctx->base.num_insns); This is non-bisectable. You're removing support for all registers and only adding back PMC5. You add them all back before the end of the series, but the middle patches do not behave correctly. r~
diff --git a/target/ppc/power8-pmu-insn-cnt.c.inc b/target/ppc/power8-pmu-insn-cnt.c.inc index 6cdf2d2d88..3cfb801c69 100644 --- a/target/ppc/power8-pmu-insn-cnt.c.inc +++ b/target/ppc/power8-pmu-insn-cnt.c.inc @@ -10,6 +10,38 @@ * See the COPYING file in the top-level directory. */ +#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) +/* + * This function increments a SPR 'spr' by 'inc_val' if a given + * register 'reg' has a bitmask 'mask' set (cond = TCG_COND_EQ) or + * not set (TCG_COND_NE). + */ +static void inc_spr_if_cond(int reg, uint64_t mask, TCGCond cond, + int spr, int inc_val) +{ + TCGCond exit_cond = tcg_invert_cond(cond); + TCGLabel *l_exit; + TCGv t0, t1; + + l_exit = gen_new_label(); + + t0 = tcg_temp_new(); + gen_load_spr(t0, reg); + tcg_gen_andi_tl(t0, t0, mask); + tcg_gen_brcondi_tl(exit_cond, t0, mask, l_exit); + + t1 = tcg_temp_new(); + gen_load_spr(t1, spr); + tcg_gen_addi_tl(t1, t1, inc_val); + gen_store_spr(spr, t1); + + gen_set_label(l_exit); + + tcg_temp_free(t0); + tcg_temp_free(t1); +} +#endif /* #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */ + #if defined(TARGET_PPC64) static void pmu_count_insns(DisasContext *ctx) { @@ -22,15 +54,8 @@ static void pmu_count_insns(DisasContext *ctx) } #if !defined(CONFIG_USER_ONLY) - /* - * The PMU insns_inc() helper stops the internal PMU timer if a - * counter overflows happens. In that case, if the guest is - * running with icount and we do not handle it beforehand, - * the helper can trigger a 'bad icount read'. - */ - gen_icount_io_start(ctx); - - gen_helper_insns_inc(cpu_env, tcg_constant_i32(ctx->base.num_insns)); + inc_spr_if_cond(SPR_POWER_MMCR0, MMCR0_FC56, TCG_COND_NE, + SPR_POWER_PMC5, ctx->base.num_insns); #else /* * User mode can read (but not write) PMC5 and start/stop
The code that increments a PMC is repetitive: check if a given register has a bit/mask set or cleared and increment the counter. inc_spr_if_cond() will help deal with this repetition. This patch also gives a sample of how the function works by incrementing PMC5, which is supposed to be incremented only if MMCR0_FC56 is not set. We've also removing the call from the helper since that would cause PMC5 to be counted twice. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/power8-pmu-insn-cnt.c.inc | 43 ++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 9 deletions(-)