[PATCH RESEND] perf/util: make TYPE_STATE_MAX_REGS architecture-dependent

[PATCH RESEND] perf/util: make TYPE_STATE_MAX_REGS architecture-dependent

[Suchit Karunakaran]

Replace the fixed definition of TYPE_STATE_MAX_REGS with architecture-
specific values for better accuracy across multiple CPU architectures
including PowerPC, ARM, x86, RISC-V, MIPS, and others. This change ensures
the type state registers array size matches the actual register count of
the target platform.

Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
---
 tools/perf/util/annotate-data.h | 45 ++++++++++++++++++++++++++++++---
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index 541fee1a5f0a..0dfb12a8f1cc 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -189,11 +189,48 @@ struct type_state_stack {
 	u8 kind;
 };
 
-/* FIXME: This should be arch-dependent */
-#ifdef __powerpc__
-#define TYPE_STATE_MAX_REGS  32
+#if defined(__powerpc__) || defined(__powerpc64__)
+#define TYPE_STATE_MAX_REGS 32
+#elif defined(__aarch64__)
+#define TYPE_STATE_MAX_REGS 31
+#elif defined(__arm__)
+#define TYPE_STATE_MAX_REGS 16
+#elif defined(__x86_64__)
+#define TYPE_STATE_MAX_REGS 16
+#elif defined(__i386__)
+#define TYPE_STATE_MAX_REGS 8
+#elif defined(__riscv)
+#define TYPE_STATE_MAX_REGS 32
+#elif defined(__mips__)
+#define TYPE_STATE_MAX_REGS 32
+#elif defined(__sparc__) || defined(__sparc64__)
+#define TYPE_STATE_MAX_REGS 32
+#elif defined(__alpha__)
+#define TYPE_STATE_MAX_REGS 32
+#elif defined(__s390__) || defined(__s390x__)
+#define TYPE_STATE_MAX_REGS 16
+#elif defined(__sh__)
+#define TYPE_STATE_MAX_REGS 16
+#elif defined(__nios2__)
+#define TYPE_STATE_MAX_REGS 32
+#elif defined(__hexagon__)
+#define TYPE_STATE_MAX_REGS 32
+#elif defined(__openrisc__)
+#define TYPE_STATE_MAX_REGS 32
+#elif defined(__csky__)
+#define TYPE_STATE_MAX_REGS 32
+#elif defined(__loongarch__)
+#define TYPE_STATE_MAX_REGS 32
+#elif defined(__arc__)
+#define TYPE_STATE_MAX_REGS 32
+#elif defined(__microblaze__)
+#define TYPE_STATE_MAX_REGS 32
+#elif defined(__xtensa__)
+#define TYPE_STATE_MAX_REGS 16
+#elif defined(__m68k__)
+#define TYPE_STATE_MAX_REGS 16
 #else
-#define TYPE_STATE_MAX_REGS  16
+#define TYPE_STATE_MAX_REGS 16
 #endif
 
 /*
-- 
2.50.1

Re: [PATCH RESEND] perf/util: make TYPE_STATE_MAX_REGS architecture-dependent

[Namhyung Kim]

Hello,

On Fri, Aug 15, 2025 at 11:57:42PM +0530, Suchit Karunakaran wrote:
> Replace the fixed definition of TYPE_STATE_MAX_REGS with architecture-
> specific values for better accuracy across multiple CPU architectures
> including PowerPC, ARM, x86, RISC-V, MIPS, and others. This change ensures
> the type state registers array size matches the actual register count of
> the target platform.

Thanks for the patch.  Unfortunately we support x86 and powerpc only but
this looks like good information for later work. :)

Thanks,
Namhyung

> 
> Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> ---
>  tools/perf/util/annotate-data.h | 45 ++++++++++++++++++++++++++++++---
>  1 file changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
> index 541fee1a5f0a..0dfb12a8f1cc 100644
> --- a/tools/perf/util/annotate-data.h
> +++ b/tools/perf/util/annotate-data.h
> @@ -189,11 +189,48 @@ struct type_state_stack {
>  	u8 kind;
>  };
>  
> -/* FIXME: This should be arch-dependent */
> -#ifdef __powerpc__
> -#define TYPE_STATE_MAX_REGS  32
> +#if defined(__powerpc__) || defined(__powerpc64__)
> +#define TYPE_STATE_MAX_REGS 32
> +#elif defined(__aarch64__)
> +#define TYPE_STATE_MAX_REGS 31
> +#elif defined(__arm__)
> +#define TYPE_STATE_MAX_REGS 16
> +#elif defined(__x86_64__)
> +#define TYPE_STATE_MAX_REGS 16
> +#elif defined(__i386__)
> +#define TYPE_STATE_MAX_REGS 8
> +#elif defined(__riscv)
> +#define TYPE_STATE_MAX_REGS 32
> +#elif defined(__mips__)
> +#define TYPE_STATE_MAX_REGS 32
> +#elif defined(__sparc__) || defined(__sparc64__)
> +#define TYPE_STATE_MAX_REGS 32
> +#elif defined(__alpha__)
> +#define TYPE_STATE_MAX_REGS 32
> +#elif defined(__s390__) || defined(__s390x__)
> +#define TYPE_STATE_MAX_REGS 16
> +#elif defined(__sh__)
> +#define TYPE_STATE_MAX_REGS 16
> +#elif defined(__nios2__)
> +#define TYPE_STATE_MAX_REGS 32
> +#elif defined(__hexagon__)
> +#define TYPE_STATE_MAX_REGS 32
> +#elif defined(__openrisc__)
> +#define TYPE_STATE_MAX_REGS 32
> +#elif defined(__csky__)
> +#define TYPE_STATE_MAX_REGS 32
> +#elif defined(__loongarch__)
> +#define TYPE_STATE_MAX_REGS 32
> +#elif defined(__arc__)
> +#define TYPE_STATE_MAX_REGS 32
> +#elif defined(__microblaze__)
> +#define TYPE_STATE_MAX_REGS 32
> +#elif defined(__xtensa__)
> +#define TYPE_STATE_MAX_REGS 16
> +#elif defined(__m68k__)
> +#define TYPE_STATE_MAX_REGS 16
>  #else
> -#define TYPE_STATE_MAX_REGS  16
> +#define TYPE_STATE_MAX_REGS 16
>  #endif
>  
>  /*
> -- 
> 2.50.1
> 

Re: [PATCH RESEND] perf/util: make TYPE_STATE_MAX_REGS architecture-dependent

[Suchit Karunakaran]

Hi Namhyung,
Thanks for reviewing the patch. I'd like to ask if there's anything
else I should do regarding this patch, given that it's supported only
for x86 and powerpc?

Thanks,
Suchit

Re: [PATCH RESEND] perf/util: make TYPE_STATE_MAX_REGS architecture-dependent

[Suchit Karunakaran]

On Sat, 16 Aug 2025 at 12:16, Suchit Karunakaran
<suchitkarunakaran@gmail.com> wrote:
>
> Hi Namhyung,
> Thanks for reviewing the patch. I'd like to ask if there's anything
> else I should do regarding this patch, given that it's supported only
> for x86 and powerpc?
>
> Thanks,
> Suchit


Hi Namhyung,
Following up on this patch. I would appreciate any guidance on further
actions needed, considering its current support is only for x86 and
PowerPC.

Thanks,
Suchit

Re: [PATCH RESEND] perf/util: make TYPE_STATE_MAX_REGS architecture-dependent

[Ian Rogers]

On Mon, Aug 25, 2025 at 7:17 AM Suchit Karunakaran
<suchitkarunakaran@gmail.com> wrote:
>
> On Sat, 16 Aug 2025 at 12:16, Suchit Karunakaran
> <suchitkarunakaran@gmail.com> wrote:
> >
> > Hi Namhyung,
> > Thanks for reviewing the patch. I'd like to ask if there's anything
> > else I should do regarding this patch, given that it's supported only
> > for x86 and powerpc?
> >
> > Thanks,
> > Suchit
>
>
> Hi Namhyung,
> Following up on this patch. I would appreciate any guidance on further
> actions needed, considering its current support is only for x86 and
> PowerPC.

Can we just make  TYPE_STATE_MAX_REGS 32? There's no reason to assume
the architecture the perf binary is built on matches the perf.data
file being processed by perf annotate. Given 32 is bigger than 16 then
this can just be a maximum value (a comment to this effect in the code
would be grand). In general, the use of the arch directory and things
like "#ifdef __powerpc__" are code smells that are going to break with
cross-architecture profiling.

Thanks,
Ian

Re: [PATCH RESEND] perf/util: make TYPE_STATE_MAX_REGS architecture-dependent

[Suchit Karunakaran]

On Mon, 25 Aug 2025 at 22:54, Ian Rogers <irogers@google.com> wrote:
>
> On Mon, Aug 25, 2025 at 7:17 AM Suchit Karunakaran
> <suchitkarunakaran@gmail.com> wrote:
> >
> > On Sat, 16 Aug 2025 at 12:16, Suchit Karunakaran
> > <suchitkarunakaran@gmail.com> wrote:
> > >
> > > Hi Namhyung,
> > > Thanks for reviewing the patch. I'd like to ask if there's anything
> > > else I should do regarding this patch, given that it's supported only
> > > for x86 and powerpc?
> > >
> > > Thanks,
> > > Suchit
> >
> >
> > Hi Namhyung,
> > Following up on this patch. I would appreciate any guidance on further
> > actions needed, considering its current support is only for x86 and
> > PowerPC.
>
> Can we just make  TYPE_STATE_MAX_REGS 32? There's no reason to assume
> the architecture the perf binary is built on matches the perf.data
> file being processed by perf annotate. Given 32 is bigger than 16 then
> this can just be a maximum value (a comment to this effect in the code
> would be grand). In general, the use of the arch directory and things
> like "#ifdef __powerpc__" are code smells that are going to break with
> cross-architecture profiling.
>
> Thanks,
> Ian

Yeah I agree that setting TYPE_STATE_MAX_REGS to 32 is a more
reasonable approach. I will send a v2 patch incorporating this change.
Thanks,
Suchit