Re: [PATCH v1 0/4] perf parse-regs: Cleanup config and building

[Ian Rogers]

On Wed, Feb 14, 2024 at 3:40 AM Leo Yan <leo.yan@xxxxxxxxx> wrote:
>
> Currently, the perf building enables register parsing based on the
> target architecture has supported register feature.
>
> Furthermore, the perf building system needs to maintain a variable
> 'NO_PERF_REGS' and defines macro 'HAVE_PERF_REGS_SUPPORT' for statically
> compiling the tool.
>
> As a result, the perf has no flexibilty for parsing register if an
> architecture doesn't support it. And the source files use the macro
> 'HAVE_PERF_REGS_SUPPORT' to switch on and off the register parsing
> related code, which is not a good practice.
>
> This series is to remove the static building for register parsing. In
> theory, we should can dynamically detect if an arch has support this
> feature and functions can return errors when the feature is not
> supported.
>
> The first patch is to remove unused build configuration
> CONFIG_PERF_REGS.
>
> The second patch is to build perf register functions, without using the
> macro 'HAVE_PERF_REGS_SUPPORT' to statically turn on or off code.
>
> The third patch is to introduce a weak function arch__sample_reg_masks(),
> this function can allow the target arch to return its sample register
> list.  With this change, we can totally remove the macro
> 'HAVE_PERF_REGS_SUPPORT' in the source file.
>
> The forth patch is to clean up the Makefile for removing relevant
> configuration and macro definition, as they are not useful anymore.
>
> I tested this patch set on Arm64 and x86 for building and did a cross
> register parsing ('perf record' on Arm64 and 'perf report' on x86).
>
>
> Leo Yan (4):
>   perf build: Remove unused CONFIG_PERF_REGS
>   perf parse-regs: Always build perf register functions
>   perf parse-regs: Introduce a weak function arch__sample_reg_masks()
>   perf build: Cleanup perf register configuration

Thanks Leo, this is great cleanup! Series:
Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Ian

>  tools/perf/Makefile.config                    | 25 --------------
>  tools/perf/arch/arm/util/perf_regs.c          |  7 +++-
>  tools/perf/arch/arm64/util/machine.c          |  2 ++
>  tools/perf/arch/arm64/util/perf_regs.c        |  7 +++-
>  tools/perf/arch/csky/util/perf_regs.c         |  7 +++-
>  tools/perf/arch/loongarch/util/perf_regs.c    |  7 +++-
>  tools/perf/arch/mips/util/perf_regs.c         |  7 +++-
>  tools/perf/arch/powerpc/util/perf_regs.c      |  7 +++-
>  tools/perf/arch/riscv/util/perf_regs.c        |  7 +++-
>  tools/perf/arch/s390/util/perf_regs.c         |  7 +++-
>  tools/perf/arch/x86/util/perf_regs.c          |  7 +++-
>  tools/perf/util/parse-regs-options.c          |  8 ++---
>  .../util/perf-regs-arch/perf_regs_aarch64.c   |  4 ---
>  .../perf/util/perf-regs-arch/perf_regs_arm.c  |  4 ---
>  .../perf/util/perf-regs-arch/perf_regs_csky.c |  4 ---
>  .../util/perf-regs-arch/perf_regs_loongarch.c |  4 ---
>  .../perf/util/perf-regs-arch/perf_regs_mips.c |  4 ---
>  .../util/perf-regs-arch/perf_regs_powerpc.c   |  4 ---
>  .../util/perf-regs-arch/perf_regs_riscv.c     |  4 ---
>  .../perf/util/perf-regs-arch/perf_regs_s390.c |  4 ---
>  .../perf/util/perf-regs-arch/perf_regs_x86.c  |  4 ---
>  tools/perf/util/perf_regs.c                   | 11 ++++--
>  tools/perf/util/perf_regs.h                   | 34 +------------------
>  23 files changed, 67 insertions(+), 112 deletions(-)
>
> --
> 2.34.1
>