2024-02-14 11:40:47

by Leo Yan

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

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

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



2024-02-14 11:40:53

by Leo Yan

[permalink] [raw]
Subject: [PATCH v1 2/4] perf parse-regs: Always build perf register functions

Currently, the macro HAVE_PERF_REGS_SUPPORT is used as a switch to turn
on or turn off the code of perf registers. If any architecture cannot
support perf register, it disables the perf register parsing, for both
the native parsing and cross parsing for other architectures.

To support both the native parsing and cross parsing, the tool should
always build the perf regs functions. Thus, this patch removes
HAVE_PERF_REGS_SUPPORT from the perf regs files.

Signed-off-by: Leo Yan <[email protected]>
---
.../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 | 4 ---
tools/perf/util/perf_regs.h | 31 -------------------
11 files changed, 71 deletions(-)

diff --git a/tools/perf/util/perf-regs-arch/perf_regs_aarch64.c b/tools/perf/util/perf-regs-arch/perf_regs_aarch64.c
index 696566c54768..9dcda80d310f 100644
--- a/tools/perf/util/perf-regs-arch/perf_regs_aarch64.c
+++ b/tools/perf/util/perf-regs-arch/perf_regs_aarch64.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0

-#ifdef HAVE_PERF_REGS_SUPPORT
-
#include "../perf_regs.h"
#include "../../../arch/arm64/include/uapi/asm/perf_regs.h"

@@ -92,5 +90,3 @@ uint64_t __perf_reg_sp_arm64(void)
{
return PERF_REG_ARM64_SP;
}
-
-#endif
diff --git a/tools/perf/util/perf-regs-arch/perf_regs_arm.c b/tools/perf/util/perf-regs-arch/perf_regs_arm.c
index 700fd07cd2aa..e29d130a587a 100644
--- a/tools/perf/util/perf-regs-arch/perf_regs_arm.c
+++ b/tools/perf/util/perf-regs-arch/perf_regs_arm.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0

-#ifdef HAVE_PERF_REGS_SUPPORT
-
#include "../perf_regs.h"
#include "../../../arch/arm/include/uapi/asm/perf_regs.h"

@@ -56,5 +54,3 @@ uint64_t __perf_reg_sp_arm(void)
{
return PERF_REG_ARM_SP;
}
-
-#endif
diff --git a/tools/perf/util/perf-regs-arch/perf_regs_csky.c b/tools/perf/util/perf-regs-arch/perf_regs_csky.c
index a2841094e096..75b461ef2eba 100644
--- a/tools/perf/util/perf-regs-arch/perf_regs_csky.c
+++ b/tools/perf/util/perf-regs-arch/perf_regs_csky.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0

-#ifdef HAVE_PERF_REGS_SUPPORT
-
#include "../perf_regs.h"
#include "../../arch/csky/include/uapi/asm/perf_regs.h"

@@ -96,5 +94,3 @@ uint64_t __perf_reg_sp_csky(void)
{
return PERF_REG_CSKY_SP;
}
-
-#endif
diff --git a/tools/perf/util/perf-regs-arch/perf_regs_loongarch.c b/tools/perf/util/perf-regs-arch/perf_regs_loongarch.c
index a9ba0f934123..043f97f4e3ac 100644
--- a/tools/perf/util/perf-regs-arch/perf_regs_loongarch.c
+++ b/tools/perf/util/perf-regs-arch/perf_regs_loongarch.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0

-#ifdef HAVE_PERF_REGS_SUPPORT
-
#include "../perf_regs.h"
#include "../../../arch/loongarch/include/uapi/asm/perf_regs.h"

@@ -87,5 +85,3 @@ uint64_t __perf_reg_sp_loongarch(void)
{
return PERF_REG_LOONGARCH_R3;
}
-
-#endif
diff --git a/tools/perf/util/perf-regs-arch/perf_regs_mips.c b/tools/perf/util/perf-regs-arch/perf_regs_mips.c
index 5a45830cfbf5..793178fc3c78 100644
--- a/tools/perf/util/perf-regs-arch/perf_regs_mips.c
+++ b/tools/perf/util/perf-regs-arch/perf_regs_mips.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0

-#ifdef HAVE_PERF_REGS_SUPPORT
-
#include "../perf_regs.h"
#include "../../../arch/mips/include/uapi/asm/perf_regs.h"

@@ -83,5 +81,3 @@ uint64_t __perf_reg_sp_mips(void)
{
return PERF_REG_MIPS_R29;
}
-
-#endif
diff --git a/tools/perf/util/perf-regs-arch/perf_regs_powerpc.c b/tools/perf/util/perf-regs-arch/perf_regs_powerpc.c
index 1f0d682db74a..08636bb09a3a 100644
--- a/tools/perf/util/perf-regs-arch/perf_regs_powerpc.c
+++ b/tools/perf/util/perf-regs-arch/perf_regs_powerpc.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0

-#ifdef HAVE_PERF_REGS_SUPPORT
-
#include "../perf_regs.h"
#include "../../../arch/powerpc/include/uapi/asm/perf_regs.h"

@@ -141,5 +139,3 @@ uint64_t __perf_reg_sp_powerpc(void)
{
return PERF_REG_POWERPC_R1;
}
-
-#endif
diff --git a/tools/perf/util/perf-regs-arch/perf_regs_riscv.c b/tools/perf/util/perf-regs-arch/perf_regs_riscv.c
index e432630be4c5..337b687c655d 100644
--- a/tools/perf/util/perf-regs-arch/perf_regs_riscv.c
+++ b/tools/perf/util/perf-regs-arch/perf_regs_riscv.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0

-#ifdef HAVE_PERF_REGS_SUPPORT
-
#include "../perf_regs.h"
#include "../../../arch/riscv/include/uapi/asm/perf_regs.h"

@@ -88,5 +86,3 @@ uint64_t __perf_reg_sp_riscv(void)
{
return PERF_REG_RISCV_SP;
}
-
-#endif
diff --git a/tools/perf/util/perf-regs-arch/perf_regs_s390.c b/tools/perf/util/perf-regs-arch/perf_regs_s390.c
index 1c7a46db778c..d69bba881080 100644
--- a/tools/perf/util/perf-regs-arch/perf_regs_s390.c
+++ b/tools/perf/util/perf-regs-arch/perf_regs_s390.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0

-#ifdef HAVE_PERF_REGS_SUPPORT
-
#include "../perf_regs.h"
#include "../../../arch/s390/include/uapi/asm/perf_regs.h"

@@ -92,5 +90,3 @@ uint64_t __perf_reg_sp_s390(void)
{
return PERF_REG_S390_R15;
}
-
-#endif
diff --git a/tools/perf/util/perf-regs-arch/perf_regs_x86.c b/tools/perf/util/perf-regs-arch/perf_regs_x86.c
index 873c620f0634..708954a9d35d 100644
--- a/tools/perf/util/perf-regs-arch/perf_regs_x86.c
+++ b/tools/perf/util/perf-regs-arch/perf_regs_x86.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0

-#ifdef HAVE_PERF_REGS_SUPPORT
-
#include "../perf_regs.h"
#include "../../../arch/x86/include/uapi/asm/perf_regs.h"

@@ -94,5 +92,3 @@ uint64_t __perf_reg_sp_x86(void)
{
return PERF_REG_X86_SP;
}
-
-#endif
diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
index e2275856b570..64724eb58dd5 100644
--- a/tools/perf/util/perf_regs.c
+++ b/tools/perf/util/perf_regs.c
@@ -21,8 +21,6 @@ uint64_t __weak arch__user_reg_mask(void)
return 0;
}

-#ifdef HAVE_PERF_REGS_SUPPORT
-
const char *perf_reg_name(int id, const char *arch)
{
const char *reg_name = NULL;
@@ -125,5 +123,3 @@ uint64_t perf_arch_reg_sp(const char *arch)
pr_err("Fail to find SP register for arch %s, returns 0\n", arch);
return 0;
}
-
-#endif
diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
index ecd2a5362042..7fd0c7b4cec1 100644
--- a/tools/perf/util/perf_regs.h
+++ b/tools/perf/util/perf_regs.h
@@ -27,7 +27,6 @@ int arch_sdt_arg_parse_op(char *old_op, char **new_op);
uint64_t arch__intr_reg_mask(void);
uint64_t arch__user_reg_mask(void);

-#ifdef HAVE_PERF_REGS_SUPPORT
extern const struct sample_reg sample_reg_masks[];

const char *perf_reg_name(int id, const char *arch);
@@ -67,34 +66,4 @@ static inline uint64_t DWARF_MINIMAL_REGS(const char *arch)
return (1ULL << perf_arch_reg_ip(arch)) | (1ULL << perf_arch_reg_sp(arch));
}

-#else
-
-static inline uint64_t DWARF_MINIMAL_REGS(const char *arch __maybe_unused)
-{
- return 0;
-}
-
-static inline const char *perf_reg_name(int id __maybe_unused, const char *arch __maybe_unused)
-{
- return "unknown";
-}
-
-static inline int perf_reg_value(u64 *valp __maybe_unused,
- struct regs_dump *regs __maybe_unused,
- int id __maybe_unused)
-{
- return 0;
-}
-
-static inline uint64_t perf_arch_reg_ip(const char *arch __maybe_unused)
-{
- return 0;
-}
-
-static inline uint64_t perf_arch_reg_sp(const char *arch __maybe_unused)
-{
- return 0;
-}
-
-#endif /* HAVE_PERF_REGS_SUPPORT */
#endif /* __PERF_REGS_H */
--
2.34.1


2024-02-14 11:41:09

by Leo Yan

[permalink] [raw]
Subject: [PATCH v1 3/4] perf parse-regs: Introduce a weak function arch__sample_reg_masks()

Every architecture can provide a register list for sampling. If an
architecture doesn't support register sampling, it won't define the data
structure 'sample_reg_masks'. Consequently, any code using this
structure must be protected by the macro 'HAVE_PERF_REGS_SUPPORT'.

This patch defines a weak function, arch__sample_reg_masks(), which will
be replaced by an architecture-defined function for returning the
architecture's register list. With this refactoring, the function always
exists, the condition checking for 'HAVE_PERF_REGS_SUPPORT' is not
needed anymore, so remove it.

Signed-off-by: Leo Yan <[email protected]>
---
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 ++------
tools/perf/util/perf_regs.c | 9 +++++++++
tools/perf/util/perf_regs.h | 3 +--
13 files changed, 68 insertions(+), 17 deletions(-)

diff --git a/tools/perf/arch/arm/util/perf_regs.c b/tools/perf/arch/arm/util/perf_regs.c
index 2c56e8b56ddf..f94a0210c7b7 100644
--- a/tools/perf/arch/arm/util/perf_regs.c
+++ b/tools/perf/arch/arm/util/perf_regs.c
@@ -2,7 +2,7 @@
#include "perf_regs.h"
#include "../../../util/perf_regs.h"

-const struct sample_reg sample_reg_masks[] = {
+static const struct sample_reg sample_reg_masks[] = {
SMPL_REG_END
};

@@ -15,3 +15,8 @@ uint64_t arch__user_reg_mask(void)
{
return PERF_REGS_MASK;
}
+
+const struct sample_reg *arch__sample_reg_masks(void)
+{
+ return sample_reg_masks;
+}
diff --git a/tools/perf/arch/arm64/util/machine.c b/tools/perf/arch/arm64/util/machine.c
index ba1144366e85..aab1cc2bc283 100644
--- a/tools/perf/arch/arm64/util/machine.c
+++ b/tools/perf/arch/arm64/util/machine.c
@@ -12,5 +12,7 @@

void arch__add_leaf_frame_record_opts(struct record_opts *opts)
{
+ const struct sample_reg *sample_reg_masks = arch__sample_reg_masks();
+
opts->sample_user_regs |= sample_reg_masks[PERF_REG_ARM64_LR].mask;
}
diff --git a/tools/perf/arch/arm64/util/perf_regs.c b/tools/perf/arch/arm64/util/perf_regs.c
index 1b79d8eab22f..09308665e28a 100644
--- a/tools/perf/arch/arm64/util/perf_regs.c
+++ b/tools/perf/arch/arm64/util/perf_regs.c
@@ -16,7 +16,7 @@
#define HWCAP_SVE (1 << 22)
#endif

-const struct sample_reg sample_reg_masks[] = {
+static const struct sample_reg sample_reg_masks[] = {
SMPL_REG(x0, PERF_REG_ARM64_X0),
SMPL_REG(x1, PERF_REG_ARM64_X1),
SMPL_REG(x2, PERF_REG_ARM64_X2),
@@ -175,3 +175,8 @@ uint64_t arch__user_reg_mask(void)
}
return PERF_REGS_MASK;
}
+
+const struct sample_reg *arch__sample_reg_masks(void)
+{
+ return sample_reg_masks;
+}
diff --git a/tools/perf/arch/csky/util/perf_regs.c b/tools/perf/arch/csky/util/perf_regs.c
index c0877c264d49..6b1665f41180 100644
--- a/tools/perf/arch/csky/util/perf_regs.c
+++ b/tools/perf/arch/csky/util/perf_regs.c
@@ -2,7 +2,7 @@
#include "perf_regs.h"
#include "../../util/perf_regs.h"

-const struct sample_reg sample_reg_masks[] = {
+static const struct sample_reg sample_reg_masks[] = {
SMPL_REG_END
};

@@ -15,3 +15,8 @@ uint64_t arch__user_reg_mask(void)
{
return PERF_REGS_MASK;
}
+
+const struct sample_reg *arch__sample_reg_masks(void)
+{
+ return sample_reg_masks;
+}
diff --git a/tools/perf/arch/loongarch/util/perf_regs.c b/tools/perf/arch/loongarch/util/perf_regs.c
index 2c56e8b56ddf..f94a0210c7b7 100644
--- a/tools/perf/arch/loongarch/util/perf_regs.c
+++ b/tools/perf/arch/loongarch/util/perf_regs.c
@@ -2,7 +2,7 @@
#include "perf_regs.h"
#include "../../../util/perf_regs.h"

-const struct sample_reg sample_reg_masks[] = {
+static const struct sample_reg sample_reg_masks[] = {
SMPL_REG_END
};

@@ -15,3 +15,8 @@ uint64_t arch__user_reg_mask(void)
{
return PERF_REGS_MASK;
}
+
+const struct sample_reg *arch__sample_reg_masks(void)
+{
+ return sample_reg_masks;
+}
diff --git a/tools/perf/arch/mips/util/perf_regs.c b/tools/perf/arch/mips/util/perf_regs.c
index c0877c264d49..6b1665f41180 100644
--- a/tools/perf/arch/mips/util/perf_regs.c
+++ b/tools/perf/arch/mips/util/perf_regs.c
@@ -2,7 +2,7 @@
#include "perf_regs.h"
#include "../../util/perf_regs.h"

-const struct sample_reg sample_reg_masks[] = {
+static const struct sample_reg sample_reg_masks[] = {
SMPL_REG_END
};

@@ -15,3 +15,8 @@ uint64_t arch__user_reg_mask(void)
{
return PERF_REGS_MASK;
}
+
+const struct sample_reg *arch__sample_reg_masks(void)
+{
+ return sample_reg_masks;
+}
diff --git a/tools/perf/arch/powerpc/util/perf_regs.c b/tools/perf/arch/powerpc/util/perf_regs.c
index b38aa056eea0..e8e6e6fc6f17 100644
--- a/tools/perf/arch/powerpc/util/perf_regs.c
+++ b/tools/perf/arch/powerpc/util/perf_regs.c
@@ -17,7 +17,7 @@
#define PVR_POWER9 0x004E
#define PVR_POWER10 0x0080

-const struct sample_reg sample_reg_masks[] = {
+static const struct sample_reg sample_reg_masks[] = {
SMPL_REG(r0, PERF_REG_POWERPC_R0),
SMPL_REG(r1, PERF_REG_POWERPC_R1),
SMPL_REG(r2, PERF_REG_POWERPC_R2),
@@ -232,3 +232,8 @@ uint64_t arch__user_reg_mask(void)
{
return PERF_REGS_MASK;
}
+
+const struct sample_reg *arch__sample_reg_masks(void)
+{
+ return sample_reg_masks;
+}
diff --git a/tools/perf/arch/riscv/util/perf_regs.c b/tools/perf/arch/riscv/util/perf_regs.c
index c0877c264d49..6b1665f41180 100644
--- a/tools/perf/arch/riscv/util/perf_regs.c
+++ b/tools/perf/arch/riscv/util/perf_regs.c
@@ -2,7 +2,7 @@
#include "perf_regs.h"
#include "../../util/perf_regs.h"

-const struct sample_reg sample_reg_masks[] = {
+static const struct sample_reg sample_reg_masks[] = {
SMPL_REG_END
};

@@ -15,3 +15,8 @@ uint64_t arch__user_reg_mask(void)
{
return PERF_REGS_MASK;
}
+
+const struct sample_reg *arch__sample_reg_masks(void)
+{
+ return sample_reg_masks;
+}
diff --git a/tools/perf/arch/s390/util/perf_regs.c b/tools/perf/arch/s390/util/perf_regs.c
index c0877c264d49..6b1665f41180 100644
--- a/tools/perf/arch/s390/util/perf_regs.c
+++ b/tools/perf/arch/s390/util/perf_regs.c
@@ -2,7 +2,7 @@
#include "perf_regs.h"
#include "../../util/perf_regs.h"

-const struct sample_reg sample_reg_masks[] = {
+static const struct sample_reg sample_reg_masks[] = {
SMPL_REG_END
};

@@ -15,3 +15,8 @@ uint64_t arch__user_reg_mask(void)
{
return PERF_REGS_MASK;
}
+
+const struct sample_reg *arch__sample_reg_masks(void)
+{
+ return sample_reg_masks;
+}
diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
index b813502a2727..12fd93f04802 100644
--- a/tools/perf/arch/x86/util/perf_regs.c
+++ b/tools/perf/arch/x86/util/perf_regs.c
@@ -13,7 +13,7 @@
#include "../../../util/pmu.h"
#include "../../../util/pmus.h"

-const struct sample_reg sample_reg_masks[] = {
+static const struct sample_reg sample_reg_masks[] = {
SMPL_REG(AX, PERF_REG_X86_AX),
SMPL_REG(BX, PERF_REG_X86_BX),
SMPL_REG(CX, PERF_REG_X86_CX),
@@ -276,6 +276,11 @@ int arch_sdt_arg_parse_op(char *old_op, char **new_op)
return SDT_ARG_VALID;
}

+const struct sample_reg *arch__sample_reg_masks(void)
+{
+ return sample_reg_masks;
+}
+
uint64_t arch__intr_reg_mask(void)
{
struct perf_event_attr attr = {
diff --git a/tools/perf/util/parse-regs-options.c b/tools/perf/util/parse-regs-options.c
index a4a100425b3a..cda1c620968e 100644
--- a/tools/perf/util/parse-regs-options.c
+++ b/tools/perf/util/parse-regs-options.c
@@ -46,22 +46,18 @@ __parse_regs(const struct option *opt, const char *str, int unset, bool intr)

if (!strcmp(s, "?")) {
fprintf(stderr, "available registers: ");
-#ifdef HAVE_PERF_REGS_SUPPORT
- for (r = sample_reg_masks; r->name; r++) {
+ for (r = arch__sample_reg_masks(); r->name; r++) {
if (r->mask & mask)
fprintf(stderr, "%s ", r->name);
}
-#endif
fputc('\n', stderr);
/* just printing available regs */
goto error;
}
-#ifdef HAVE_PERF_REGS_SUPPORT
- for (r = sample_reg_masks; r->name; r++) {
+ for (r = arch__sample_reg_masks(); r->name; r++) {
if ((r->mask & mask) && !strcasecmp(s, r->name))
break;
}
-#endif
if (!r || !r->name) {
ui__warning("Unknown register \"%s\", check man page or run \"perf record %s?\"\n",
s, intr ? "-I" : "--user-regs=");
diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
index 64724eb58dd5..44b90bbf2d07 100644
--- a/tools/perf/util/perf_regs.c
+++ b/tools/perf/util/perf_regs.c
@@ -21,6 +21,15 @@ uint64_t __weak arch__user_reg_mask(void)
return 0;
}

+static const struct sample_reg sample_reg_masks[] = {
+ SMPL_REG_END
+};
+
+const struct sample_reg * __weak arch__sample_reg_masks(void)
+{
+ return sample_reg_masks;
+}
+
const char *perf_reg_name(int id, const char *arch)
{
const char *reg_name = NULL;
diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
index 7fd0c7b4cec1..f2d0736d65cc 100644
--- a/tools/perf/util/perf_regs.h
+++ b/tools/perf/util/perf_regs.h
@@ -26,8 +26,7 @@ enum {
int arch_sdt_arg_parse_op(char *old_op, char **new_op);
uint64_t arch__intr_reg_mask(void);
uint64_t arch__user_reg_mask(void);
-
-extern const struct sample_reg sample_reg_masks[];
+const struct sample_reg *arch__sample_reg_masks(void);

const char *perf_reg_name(int id, const char *arch);
int perf_reg_value(u64 *valp, struct regs_dump *regs, int id);
--
2.34.1


2024-02-14 11:49:48

by Leo Yan

[permalink] [raw]
Subject: [PATCH v1 4/4] perf build: Cleanup perf register configuration

The target is to allow the tool to always enable the perf register
feature for native parsing and cross parsing, and current code doesn't
depend on the macro 'HAVE_PERF_REGS_SUPPORT'.

This patch remove the variable 'NO_PERF_REGS' and the defined macro
'HAVE_PERF_REGS_SUPPORT' from the Makefile.

Signed-off-by: Leo Yan <[email protected]>
---
tools/perf/Makefile.config | 21 ---------------------
1 file changed, 21 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 8b740c668ab7..7de7111c0226 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -28,8 +28,6 @@ include $(srctree)/tools/scripts/Makefile.arch

$(call detected_var,SRCARCH)

-NO_PERF_REGS := 1
-
ifneq ($(NO_SYSCALL_TABLE),1)
NO_SYSCALL_TABLE := 1

@@ -50,7 +48,6 @@ endif

# Additional ARCH settings for ppc
ifeq ($(SRCARCH),powerpc)
- NO_PERF_REGS := 0
CFLAGS += -I$(OUTPUT)arch/powerpc/include/generated
LIBUNWIND_LIBS := -lunwind -lunwind-ppc64
endif
@@ -66,41 +63,27 @@ ifeq ($(SRCARCH),x86)
else
LIBUNWIND_LIBS = -lunwind-x86 -llzma -lunwind
endif
- NO_PERF_REGS := 0
endif

ifeq ($(SRCARCH),arm)
- NO_PERF_REGS := 0
LIBUNWIND_LIBS = -lunwind -lunwind-arm
endif

ifeq ($(SRCARCH),arm64)
- NO_PERF_REGS := 0
CFLAGS += -I$(OUTPUT)arch/arm64/include/generated
LIBUNWIND_LIBS = -lunwind -lunwind-aarch64
endif

ifeq ($(SRCARCH),loongarch)
- NO_PERF_REGS := 0
CFLAGS += -I$(OUTPUT)arch/loongarch/include/generated
LIBUNWIND_LIBS = -lunwind -lunwind-loongarch64
endif

-ifeq ($(SRCARCH),riscv)
- NO_PERF_REGS := 0
-endif
-
-ifeq ($(SRCARCH),csky)
- NO_PERF_REGS := 0
-endif
-
ifeq ($(ARCH),s390)
- NO_PERF_REGS := 0
CFLAGS += -fPIC -I$(OUTPUT)arch/s390/include/generated
endif

ifeq ($(ARCH),mips)
- NO_PERF_REGS := 0
CFLAGS += -I$(OUTPUT)arch/mips/include/generated
LIBUNWIND_LIBS = -lunwind -lunwind-mips
endif
@@ -161,10 +144,6 @@ endif
FEATURE_CHECK_CFLAGS-libopencsd := $(LIBOPENCSD_CFLAGS)
FEATURE_CHECK_LDFLAGS-libopencsd := $(LIBOPENCSD_LDFLAGS) $(OPENCSDLIBS)

-ifeq ($(NO_PERF_REGS),0)
- CFLAGS += -DHAVE_PERF_REGS_SUPPORT
-endif
-
# for linking with debug library, run like:
# make DEBUG=1 LIBDW_DIR=/opt/libdw/
ifdef LIBDW_DIR
--
2.34.1


2024-02-14 11:53:07

by Leo Yan

[permalink] [raw]
Subject: [PATCH v1 1/4] perf build: Remove unused CONFIG_PERF_REGS

CONFIG_PERF_REGS is not used, remove it.

Signed-off-by: Leo Yan <[email protected]>
---
tools/perf/Makefile.config | 4 ----
1 file changed, 4 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index aa55850fbc21..8b740c668ab7 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -105,10 +105,6 @@ ifeq ($(ARCH),mips)
LIBUNWIND_LIBS = -lunwind -lunwind-mips
endif

-ifeq ($(NO_PERF_REGS),0)
- $(call detected,CONFIG_PERF_REGS)
-endif
-
# So far there's only x86 and arm libdw unwind support merged in perf.
# Disable it on all other architectures in case libdw unwind
# support is detected in system. Add supported architectures
--
2.34.1


2024-02-14 22:43:25

by Ian Rogers

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

On Wed, Feb 14, 2024 at 3:40 AM Leo Yan <[email protected]> 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 <[email protected]>

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
>

2024-02-15 06:32:59

by Leo Yan

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

On Wed, Feb 14, 2024 at 02:42:55PM -0800, Ian Rogers wrote:

[...]

> Thanks Leo, this is great cleanup! Series:
> Reviewed-by: Ian Rogers <[email protected]>

Thanks a lot for reviewing, Ian!

Leo

2024-02-16 19:42:28

by Namhyung Kim

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

On Wed, 14 Feb 2024 19:39:43 +0800, Leo Yan 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.
>
> [...]

Applied to perf-tools-next, thanks!

Best regards,
--
Namhyung Kim <[email protected]>