| Message ID | 20230311112122.28894-1-p4ranlee@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | perf tools riscv: Add support for riscv lookup_binutils_path | expand |
On Sat, Mar 11, 2023 at 3:22 AM paranlee <p4ranlee@gmail.com> wrote: > > Add to know RISC-V binutils path. > Secondarily, edit the code block with alphabetical order. > > Signed-off-by: Paran Lee <p4ranlee@gmail.com> > --- > tools/perf/arch/common.c | 51 +++++++++++++++++++++++++++------------- > 1 file changed, 35 insertions(+), 16 deletions(-) > > diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c > index 59dd875fd5e4..058527ededdd 100644 > --- a/tools/perf/arch/common.c > +++ b/tools/perf/arch/common.c > @@ -29,11 +29,23 @@ const char *const arm_triplets[] = { > }; > > const char *const arm64_triplets[] = { > + "aarch64-unknown-linux-", Modifying ARM64 behavior should be a separate change. > "aarch64-linux-android-", > "aarch64-linux-gnu-", > NULL > }; > > +const char *const mips_triplets[] = { > + "mips-unknown-linux-gnu-", > + "mipsel-linux-android-", > + "mips-linux-gnu-", > + "mips64-linux-gnu-", > + "mips64el-linux-gnuabi64-", > + "mips64-linux-gnuabi64-", > + "mipsel-linux-gnu-", > + NULL > +}; > + This will affect the blame history. It should probably be its own change too. > const char *const powerpc_triplets[] = { > "powerpc-unknown-linux-gnu-", > "powerpc-linux-gnu-", > @@ -43,6 +55,20 @@ const char *const powerpc_triplets[] = { > NULL > }; > > +const char *const riscv32_triplets[] = { > + "riscv32-unknown-linux-gnu-", > + "riscv32-linux-android-", > + "riscv32-linux-gnu-", > + NULL > +}; > + > +const char *const riscv64_triplets[] = { > + "riscv64-unknown-linux-gnu-", > + "riscv64-linux-android-", > + "riscv64-linux-gnu-", > + NULL > +}; > + > const char *const s390_triplets[] = { > "s390-ibm-linux-", > "s390x-linux-gnu-", > @@ -78,17 +104,6 @@ const char *const x86_triplets[] = { > NULL > }; > > -const char *const mips_triplets[] = { > - "mips-unknown-linux-gnu-", > - "mipsel-linux-android-", > - "mips-linux-gnu-", > - "mips64-linux-gnu-", > - "mips64el-linux-gnuabi64-", > - "mips64-linux-gnuabi64-", > - "mipsel-linux-gnu-", > - NULL > -}; > - > static bool lookup_path(char *name) > { > bool found = false; > @@ -164,18 +179,22 @@ static int perf_env__lookup_binutils_path(struct perf_env *env, > path_list = arm_triplets; > else if (!strcmp(arch, "arm64")) > path_list = arm64_triplets; > + else if (!strcmp(arch, "mips")) > + path_list = mips_triplets; > else if (!strcmp(arch, "powerpc")) > path_list = powerpc_triplets; > - else if (!strcmp(arch, "sh")) > - path_list = sh_triplets; > + else if (!strcmp(arch, "riscv32")) > + path_list = riscv32_triplets; > + else if (!strcmp(arch, "riscv64")) > + path_list = riscv64_triplets; > else if (!strcmp(arch, "s390")) > - path_list = s390_triplets; > + path_list = s390_triplets; whitespace issue? > + else if (!strcmp(arch, "sh")) > + path_list = sh_triplets; > else if (!strcmp(arch, "sparc")) > path_list = sparc_triplets; > else if (!strcmp(arch, "x86")) > path_list = x86_triplets; > - else if (!strcmp(arch, "mips")) > - path_list = mips_triplets; > else { > ui__error("binutils for %s not supported.\n", arch); > goto out_error; I think in general we need to revamp this code. Two things that I see that are missing are (1) support for perf config and (2) addr2line should be configurable, you may want llvm-addr2line. Adding RISC-V is of course important too :-) Thanks, Ian > -- > 2.34.1 >
23. 3. 12. 15:27에 Ian Rogers 이(가) 쓴 글: > On Sat, Mar 11, 2023 at 3:22 AM paranlee <p4ranlee@gmail.com> wrote: >> >> Add to know RISC-V binutils path. >> Secondarily, edit the code block with alphabetical order. >> >> Signed-off-by: Paran Lee <p4ranlee@gmail.com> >> --- >> tools/perf/arch/common.c | 51 +++++++++++++++++++++++++++------------- >> 1 file changed, 35 insertions(+), 16 deletions(-) >> >> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c >> index 59dd875fd5e4..058527ededdd 100644 >> --- a/tools/perf/arch/common.c >> +++ b/tools/perf/arch/common.c >> @@ -29,11 +29,23 @@ const char *const arm_triplets[] = { >> }; >> >> const char *const arm64_triplets[] = { >> + "aarch64-unknown-linux-", > > Modifying ARM64 behavior should be a separate change. > >> "aarch64-linux-android-", >> "aarch64-linux-gnu-", >> NULL >> }; >> >> +const char *const mips_triplets[] = { >> + "mips-unknown-linux-gnu-", >> + "mipsel-linux-android-", >> + "mips-linux-gnu-", >> + "mips64-linux-gnu-", >> + "mips64el-linux-gnuabi64-", >> + "mips64-linux-gnuabi64-", >> + "mipsel-linux-gnu-", >> + NULL >> +}; >> + > > This will affect the blame history. It should probably be its own change too. Thank you for review! I agree. So I would split the patch. >> - >> static bool lookup_path(char *name) >> { >> bool found = false; >> @@ -164,18 +179,22 @@ static int perf_env__lookup_binutils_path(struct perf_env *env, >> path_list = arm_triplets; >> else if (!strcmp(arch, "arm64")) >> path_list = arm64_triplets; >> + else if (!strcmp(arch, "mips")) >> + path_list = mips_triplets; >> else if (!strcmp(arch, "powerpc")) >> path_list = powerpc_triplets; >> - else if (!strcmp(arch, "sh")) >> - path_list = sh_triplets; >> + else if (!strcmp(arch, "riscv32")) >> + path_list = riscv32_triplets; >> + else if (!strcmp(arch, "riscv64")) >> + path_list = riscv64_triplets; >> else if (!strcmp(arch, "s390")) >> - path_list = s390_triplets; >> + path_list = s390_triplets; > > whitespace issue? I tried to correct the alphabetical order because it was vaguely sorted. And I'll try to work on blame history on each arch code block as well. >> + else if (!strcmp(arch, "sh")) >> + path_list = sh_triplets; >> else if (!strcmp(arch, "sparc")) >> path_list = sparc_triplets; >> else if (!strcmp(arch, "x86")) >> path_list = x86_triplets; >> - else if (!strcmp(arch, "mips")) >> - path_list = mips_triplets; >> else { >> ui__error("binutils for %s not supported.\n", arch); >> goto out_error; > > I think in general we need to revamp this code. Two things that I see > that are missing are (1) support for perf config and (2) addr2line > should be configurable, you may want llvm-addr2line. Adding RISC-V is > of course important too :-) > > Thanks, > Ian May I ask documentation or hint that I can help work with? P.S. I'm interested in the Google Summer Of code perf category this year, especially the part about risc-v architecture, I recently purchased a development board and would like to be able to test perf on a Sifive U74 CPU based environment. But I've only used perf with command tool and don't know much about the internals, so if there is a roadmap for perf development or contribution, I have interest in perf internals both kernel and user side. May I ask information to apply? I am developing Linux Security Driver drivers for a security company. BR Paran Lee
On Sun, Mar 12, 2023 at 5:53 AM Paran Lee <p4ranlee@gmail.com> wrote: > > > > 23. 3. 12. 15:27에 Ian Rogers 이(가) 쓴 글: > > On Sat, Mar 11, 2023 at 3:22 AM paranlee <p4ranlee@gmail.com> wrote: > >> > >> Add to know RISC-V binutils path. > >> Secondarily, edit the code block with alphabetical order. > >> > >> Signed-off-by: Paran Lee <p4ranlee@gmail.com> > >> --- > >> tools/perf/arch/common.c | 51 +++++++++++++++++++++++++++------------- > >> 1 file changed, 35 insertions(+), 16 deletions(-) > >> > >> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c > >> index 59dd875fd5e4..058527ededdd 100644 > >> --- a/tools/perf/arch/common.c > >> +++ b/tools/perf/arch/common.c > >> @@ -29,11 +29,23 @@ const char *const arm_triplets[] = { > >> }; > >> > >> const char *const arm64_triplets[] = { > >> + "aarch64-unknown-linux-", > > > > Modifying ARM64 behavior should be a separate change. > > > >> "aarch64-linux-android-", > >> "aarch64-linux-gnu-", > >> NULL > >> }; > >> > >> +const char *const mips_triplets[] = { > >> + "mips-unknown-linux-gnu-", > >> + "mipsel-linux-android-", > >> + "mips-linux-gnu-", > >> + "mips64-linux-gnu-", > >> + "mips64el-linux-gnuabi64-", > >> + "mips64-linux-gnuabi64-", > >> + "mipsel-linux-gnu-", > >> + NULL > >> +}; > >> + > > > > This will affect the blame history. It should probably be its own change too. > > Thank you for review! I agree. So I would split the patch. > > >> - > >> static bool lookup_path(char *name) > >> { > >> bool found = false; > >> @@ -164,18 +179,22 @@ static int perf_env__lookup_binutils_path(struct perf_env *env, > >> path_list = arm_triplets; > >> else if (!strcmp(arch, "arm64")) > >> path_list = arm64_triplets; > >> + else if (!strcmp(arch, "mips")) > >> + path_list = mips_triplets; > >> else if (!strcmp(arch, "powerpc")) > >> path_list = powerpc_triplets; > >> - else if (!strcmp(arch, "sh")) > >> - path_list = sh_triplets; > >> + else if (!strcmp(arch, "riscv32")) > >> + path_list = riscv32_triplets; > >> + else if (!strcmp(arch, "riscv64")) > >> + path_list = riscv64_triplets; > >> else if (!strcmp(arch, "s390")) > >> - path_list = s390_triplets; > >> + path_list = s390_triplets; > > > > whitespace issue? > > I tried to correct the alphabetical order because it was vaguely sorted. > And I'll try to work on blame history on each arch code block as well. > > >> + else if (!strcmp(arch, "sh")) > >> + path_list = sh_triplets; > >> else if (!strcmp(arch, "sparc")) > >> path_list = sparc_triplets; > >> else if (!strcmp(arch, "x86")) > >> path_list = x86_triplets; > >> - else if (!strcmp(arch, "mips")) > >> - path_list = mips_triplets; > >> else { > >> ui__error("binutils for %s not supported.\n", arch); > >> goto out_error; > > > > I think in general we need to revamp this code. Two things that I see > > that are missing are (1) support for perf config and (2) addr2line > > should be configurable, you may want llvm-addr2line. Adding RISC-V is > > of course important too :-) > > > > Thanks, > > Ian > > May I ask documentation or hint that I can help work with? > > P.S. > > I'm interested in the Google Summer Of code perf category this year, > especially the part about risc-v architecture, I recently purchased a > development board and would like to be able to test perf on a Sifive U74 > CPU based environment. > But I've only used perf with command tool and don't know much about the > internals, so if there is a roadmap for perf development or > contribution, I have interest in perf internals both kernel and user side. > May I ask information to apply? > I am developing Linux Security Driver drivers for a security company. > > BR > Paran Lee Hi Paran, Thanks for being interested in GSoC with Linux perf. Here is what I posted on the mailing list: https://lore.kernel.org/linux-perf-users/CAP-5=fWxF6in4vQyGuh=0kpAYEXAYZN_KobXCY=TX2oxssZ+HQ@mail.gmail.com/ Applications are ultimately sent to: https://summerofcode.withgoogle.com/ and the entry requirements are there. I believe they are less strict than previously. Wrt the patch, could you reply to Conor's response. Thanks, Ian
On Mon, Mar 13, 2023 at 09:27:35AM -0700, Ian Rogers wrote:
> Wrt the patch, could you reply to Conor's response.
Apologies if I asked something silly, I've been ill and kinda out of it,
just noticed it as I tried to keep up with mail and was curious...
diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c index 59dd875fd5e4..058527ededdd 100644 --- a/tools/perf/arch/common.c +++ b/tools/perf/arch/common.c @@ -29,11 +29,23 @@ const char *const arm_triplets[] = { }; const char *const arm64_triplets[] = { + "aarch64-unknown-linux-", "aarch64-linux-android-", "aarch64-linux-gnu-", NULL }; +const char *const mips_triplets[] = { + "mips-unknown-linux-gnu-", + "mipsel-linux-android-", + "mips-linux-gnu-", + "mips64-linux-gnu-", + "mips64el-linux-gnuabi64-", + "mips64-linux-gnuabi64-", + "mipsel-linux-gnu-", + NULL +}; + const char *const powerpc_triplets[] = { "powerpc-unknown-linux-gnu-", "powerpc-linux-gnu-", @@ -43,6 +55,20 @@ const char *const powerpc_triplets[] = { NULL }; +const char *const riscv32_triplets[] = { + "riscv32-unknown-linux-gnu-", + "riscv32-linux-android-", + "riscv32-linux-gnu-", + NULL +}; + +const char *const riscv64_triplets[] = { + "riscv64-unknown-linux-gnu-", + "riscv64-linux-android-", + "riscv64-linux-gnu-", + NULL +}; + const char *const s390_triplets[] = { "s390-ibm-linux-", "s390x-linux-gnu-", @@ -78,17 +104,6 @@ const char *const x86_triplets[] = { NULL }; -const char *const mips_triplets[] = { - "mips-unknown-linux-gnu-", - "mipsel-linux-android-", - "mips-linux-gnu-", - "mips64-linux-gnu-", - "mips64el-linux-gnuabi64-", - "mips64-linux-gnuabi64-", - "mipsel-linux-gnu-", - NULL -}; - static bool lookup_path(char *name) { bool found = false; @@ -164,18 +179,22 @@ static int perf_env__lookup_binutils_path(struct perf_env *env, path_list = arm_triplets; else if (!strcmp(arch, "arm64")) path_list = arm64_triplets; + else if (!strcmp(arch, "mips")) + path_list = mips_triplets; else if (!strcmp(arch, "powerpc")) path_list = powerpc_triplets; - else if (!strcmp(arch, "sh")) - path_list = sh_triplets; + else if (!strcmp(arch, "riscv32")) + path_list = riscv32_triplets; + else if (!strcmp(arch, "riscv64")) + path_list = riscv64_triplets; else if (!strcmp(arch, "s390")) - path_list = s390_triplets; + path_list = s390_triplets; + else if (!strcmp(arch, "sh")) + path_list = sh_triplets; else if (!strcmp(arch, "sparc")) path_list = sparc_triplets; else if (!strcmp(arch, "x86")) path_list = x86_triplets; - else if (!strcmp(arch, "mips")) - path_list = mips_triplets; else { ui__error("binutils for %s not supported.\n", arch); goto out_error;
Add to know RISC-V binutils path. Secondarily, edit the code block with alphabetical order. Signed-off-by: Paran Lee <p4ranlee@gmail.com> --- tools/perf/arch/common.c | 51 +++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 16 deletions(-)