tools/perf/util/annotate-data.h | 45 ++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 4 deletions(-)
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
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 >
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
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
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
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
© 2016 - 2025 Red Hat, Inc.