We're going to add a significant amount of TCG ops code for
instruction counting, eventually getting rid of the 'helper_insn_inc'
helper entirely.

Create a new file to avoid putting even more stuff on the already
crowded target/ppc/translate.c.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 target/ppc/power8-pmu-insn-cnt.c.inc | 54 ++++++++++++++++++++++++++++
 target/ppc/translate.c               | 44 ++---------------------
 2 files changed, 56 insertions(+), 42 deletions(-)
 create mode 100644 target/ppc/power8-pmu-insn-cnt.c.inc

diff --git a/target/ppc/power8-pmu-insn-cnt.c.inc b/target/ppc/power8-pmu-insn-cnt.c.inc
new file mode 100644
index 0000000000..2febbcc27e
--- /dev/null
+++ b/target/ppc/power8-pmu-insn-cnt.c.inc
@@ -0,0 +1,54 @@
+/*
+ * PMU instruction counting for TCG IBM POWER chips
+ *
+ * Copyright IBM Corp. 2021
+ *
+ * Authors:
+ *  Daniel Henrique Barboza      <danielhb413@gmail.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#if defined(TARGET_PPC64)
+static void pmu_count_insns(DisasContext *ctx)
+{
+    /*
+     * Do not bother calling the helper if the PMU isn't counting
+     * instructions.
+     */
+    if (!ctx->pmu_insn_cnt) {
+        return;
+    }
+
+ #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));
+#else
+    /*
+     * User mode can read (but not write) PMC5 and start/stop
+     * the PMU via MMCR0_FC. In this case just increment
+     * PMC5 with base.num_insns.
+     */
+    TCGv t0 = tcg_temp_new();
+
+    gen_load_spr(t0, SPR_POWER_PMC5);
+    tcg_gen_addi_tl(t0, t0, ctx->base.num_insns);
+    gen_store_spr(SPR_POWER_PMC5, t0);
+
+    tcg_temp_free(t0);
+#endif /* #if !defined(CONFIG_USER_ONLY) */
+}
+#else
+static void pmu_count_insns(DisasContext *ctx)
+{
+    return;
+}
+#endif /* #if defined(TARGET_PPC64) */
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 114456148c..44773bc6cd 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4183,48 +4183,8 @@ static inline void gen_update_cfar(DisasContext *ctx, target_ulong nip)
 #endif
 }
 
-#if defined(TARGET_PPC64)
-static void pmu_count_insns(DisasContext *ctx)
-{
-    /*
-     * Do not bother calling the helper if the PMU isn't counting
-     * instructions.
-     */
-    if (!ctx->pmu_insn_cnt) {
-        return;
-    }
-
- #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));
-#else
-    /*
-     * User mode can read (but not write) PMC5 and start/stop
-     * the PMU via MMCR0_FC. In this case just increment
-     * PMC5 with base.num_insns.
-     */
-    TCGv t0 = tcg_temp_new();
-
-    gen_load_spr(t0, SPR_POWER_PMC5);
-    tcg_gen_addi_tl(t0, t0, ctx->base.num_insns);
-    gen_store_spr(SPR_POWER_PMC5, t0);
-
-    tcg_temp_free(t0);
-#endif /* #if !defined(CONFIG_USER_ONLY) */
-}
-#else
-static void pmu_count_insns(DisasContext *ctx)
-{
-    return;
-}
-#endif /* #if defined(TARGET_PPC64) */
+/* For pmu_count_insns */
+#include "target/ppc/power8-pmu-insn-cnt.c.inc"
 
 static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
 {
-- 
2.33.1

On 12/22/21 14:45, Daniel Henrique Barboza wrote:
> We're going to add a significant amount of TCG ops code for
> instruction counting, eventually getting rid of the 'helper_insn_inc'
> helper entirely.
> 
> Create a new file to avoid putting even more stuff on the already
> crowded target/ppc/translate.c.

We already have a power8-pmu-regs.c.inc file. Couldn't we use it or is it
a bad idea ?

Thanks,

C.


> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>   target/ppc/power8-pmu-insn-cnt.c.inc | 54 ++++++++++++++++++++++++++++
>   target/ppc/translate.c               | 44 ++---------------------
>   2 files changed, 56 insertions(+), 42 deletions(-)
>   create mode 100644 target/ppc/power8-pmu-insn-cnt.c.inc
> 
> diff --git a/target/ppc/power8-pmu-insn-cnt.c.inc b/target/ppc/power8-pmu-insn-cnt.c.inc
> new file mode 100644
> index 0000000000..2febbcc27e
> --- /dev/null
> +++ b/target/ppc/power8-pmu-insn-cnt.c.inc
> @@ -0,0 +1,54 @@
> +/*
> + * PMU instruction counting for TCG IBM POWER chips
> + *
> + * Copyright IBM Corp. 2021
> + *
> + * Authors:
> + *  Daniel Henrique Barboza      <danielhb413@gmail.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#if defined(TARGET_PPC64)
> +static void pmu_count_insns(DisasContext *ctx)
> +{
> +    /*
> +     * Do not bother calling the helper if the PMU isn't counting
> +     * instructions.
> +     */
> +    if (!ctx->pmu_insn_cnt) {
> +        return;
> +    }
> +
> + #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));
> +#else
> +    /*
> +     * User mode can read (but not write) PMC5 and start/stop
> +     * the PMU via MMCR0_FC. In this case just increment
> +     * PMC5 with base.num_insns.
> +     */
> +    TCGv t0 = tcg_temp_new();
> +
> +    gen_load_spr(t0, SPR_POWER_PMC5);
> +    tcg_gen_addi_tl(t0, t0, ctx->base.num_insns);
> +    gen_store_spr(SPR_POWER_PMC5, t0);
> +
> +    tcg_temp_free(t0);
> +#endif /* #if !defined(CONFIG_USER_ONLY) */
> +}
> +#else
> +static void pmu_count_insns(DisasContext *ctx)
> +{
> +    return;
> +}
> +#endif /* #if defined(TARGET_PPC64) */
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 114456148c..44773bc6cd 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4183,48 +4183,8 @@ static inline void gen_update_cfar(DisasContext *ctx, target_ulong nip)
>   #endif
>   }
>   
> -#if defined(TARGET_PPC64)
> -static void pmu_count_insns(DisasContext *ctx)
> -{
> -    /*
> -     * Do not bother calling the helper if the PMU isn't counting
> -     * instructions.
> -     */
> -    if (!ctx->pmu_insn_cnt) {
> -        return;
> -    }
> -
> - #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));
> -#else
> -    /*
> -     * User mode can read (but not write) PMC5 and start/stop
> -     * the PMU via MMCR0_FC. In this case just increment
> -     * PMC5 with base.num_insns.
> -     */
> -    TCGv t0 = tcg_temp_new();
> -
> -    gen_load_spr(t0, SPR_POWER_PMC5);
> -    tcg_gen_addi_tl(t0, t0, ctx->base.num_insns);
> -    gen_store_spr(SPR_POWER_PMC5, t0);
> -
> -    tcg_temp_free(t0);
> -#endif /* #if !defined(CONFIG_USER_ONLY) */
> -}
> -#else
> -static void pmu_count_insns(DisasContext *ctx)
> -{
> -    return;
> -}
> -#endif /* #if defined(TARGET_PPC64) */
> +/* For pmu_count_insns */
> +#include "target/ppc/power8-pmu-insn-cnt.c.inc"
>   
>   static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
>   {
> 

On 12/22/21 15:00, Cédric Le Goater wrote:
> On 12/22/21 14:45, Daniel Henrique Barboza wrote:
>> We're going to add a significant amount of TCG ops code for
>> instruction counting, eventually getting rid of the 'helper_insn_inc'
>> helper entirely.
>>
>> Create a new file to avoid putting even more stuff on the already
>> crowded target/ppc/translate.c.
> 
> We already have a power8-pmu-regs.c.inc file. Couldn't we use it or is it
> a bad idea ?

That was my first idea. But then I thought that this code would feel out of place
in power8-pmu-regs.c.inc since, at this moment, the file only has SPR read/write
implementations that are being declared by spr_tcg.h to be used by cpu_init.c.


I guess it's a matter of taste. I'm ok putting this code in power8-pmu-regs.c.inc if
you think it's better.



Daniel

> 
> Thanks,
> 
> C.
> 
> 
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   target/ppc/power8-pmu-insn-cnt.c.inc | 54 ++++++++++++++++++++++++++++
>>   target/ppc/translate.c               | 44 ++---------------------
>>   2 files changed, 56 insertions(+), 42 deletions(-)
>>   create mode 100644 target/ppc/power8-pmu-insn-cnt.c.inc
>>
>> diff --git a/target/ppc/power8-pmu-insn-cnt.c.inc b/target/ppc/power8-pmu-insn-cnt.c.inc
>> new file mode 100644
>> index 0000000000..2febbcc27e
>> --- /dev/null
>> +++ b/target/ppc/power8-pmu-insn-cnt.c.inc
>> @@ -0,0 +1,54 @@
>> +/*
>> + * PMU instruction counting for TCG IBM POWER chips
>> + *
>> + * Copyright IBM Corp. 2021
>> + *
>> + * Authors:
>> + *  Daniel Henrique Barboza      <danielhb413@gmail.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#if defined(TARGET_PPC64)
>> +static void pmu_count_insns(DisasContext *ctx)
>> +{
>> +    /*
>> +     * Do not bother calling the helper if the PMU isn't counting
>> +     * instructions.
>> +     */
>> +    if (!ctx->pmu_insn_cnt) {
>> +        return;
>> +    }
>> +
>> + #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));
>> +#else
>> +    /*
>> +     * User mode can read (but not write) PMC5 and start/stop
>> +     * the PMU via MMCR0_FC. In this case just increment
>> +     * PMC5 with base.num_insns.
>> +     */
>> +    TCGv t0 = tcg_temp_new();
>> +
>> +    gen_load_spr(t0, SPR_POWER_PMC5);
>> +    tcg_gen_addi_tl(t0, t0, ctx->base.num_insns);
>> +    gen_store_spr(SPR_POWER_PMC5, t0);
>> +
>> +    tcg_temp_free(t0);
>> +#endif /* #if !defined(CONFIG_USER_ONLY) */
>> +}
>> +#else
>> +static void pmu_count_insns(DisasContext *ctx)
>> +{
>> +    return;
>> +}
>> +#endif /* #if defined(TARGET_PPC64) */
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index 114456148c..44773bc6cd 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -4183,48 +4183,8 @@ static inline void gen_update_cfar(DisasContext *ctx, target_ulong nip)
>>   #endif
>>   }
>> -#if defined(TARGET_PPC64)
>> -static void pmu_count_insns(DisasContext *ctx)
>> -{
>> -    /*
>> -     * Do not bother calling the helper if the PMU isn't counting
>> -     * instructions.
>> -     */
>> -    if (!ctx->pmu_insn_cnt) {
>> -        return;
>> -    }
>> -
>> - #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));
>> -#else
>> -    /*
>> -     * User mode can read (but not write) PMC5 and start/stop
>> -     * the PMU via MMCR0_FC. In this case just increment
>> -     * PMC5 with base.num_insns.
>> -     */
>> -    TCGv t0 = tcg_temp_new();
>> -
>> -    gen_load_spr(t0, SPR_POWER_PMC5);
>> -    tcg_gen_addi_tl(t0, t0, ctx->base.num_insns);
>> -    gen_store_spr(SPR_POWER_PMC5, t0);
>> -
>> -    tcg_temp_free(t0);
>> -#endif /* #if !defined(CONFIG_USER_ONLY) */
>> -}
>> -#else
>> -static void pmu_count_insns(DisasContext *ctx)
>> -{
>> -    return;
>> -}
>> -#endif /* #if defined(TARGET_PPC64) */
>> +/* For pmu_count_insns */
>> +#include "target/ppc/power8-pmu-insn-cnt.c.inc"
>>   static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
>>   {
>>
>