Add to know RISC-V binutils path.
Secondarily, edit the code block with alphabetical order.
Signed-off-by: Paran Lee <[email protected]>
---
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-",
"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;
--
2.34.1
On Sat, Mar 11, 2023 at 3:22 AM paranlee <[email protected]> wrote:
>
> Add to know RISC-V binutils path.
> Secondarily, edit the code block with alphabetical order.
>
> Signed-off-by: Paran Lee <[email protected]>
> ---
> 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 <[email protected]> wrote:
>>
>> Add to know RISC-V binutils path.
>> Secondarily, edit the code block with alphabetical order.
>>
>> Signed-off-by: Paran Lee <[email protected]>
>> ---
>> 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 <[email protected]> wrote:
>
>
>
> 23. 3. 12. 15:27에 Ian Rogers 이(가) 쓴 글:
> > On Sat, Mar 11, 2023 at 3:22 AM paranlee <[email protected]> wrote:
> >>
> >> Add to know RISC-V binutils path.
> >> Secondarily, edit the code block with alphabetical order.
> >>
> >> Signed-off-by: Paran Lee <[email protected]>
> >> ---
> >> 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...