Following patch is fix for wrong DEBUG configuration.
Signed-off-by: Martin Liska <[email protected]>
---
tools/perf/arch/common.c | 2 +-
tools/perf/config/Makefile | 2 +-
tools/perf/util/symbol.c | 2 +-
tools/perf/util/trace-event-parse.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index 49776f1..b7bb42c 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
static bool lookup_path(char *name)
{
bool found = false;
- char *path, *tmp;
+ char *path, *tmp = NULL;
char buf[PATH_MAX];
char *env = getenv("PATH");
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 59a98c6..b708ae0 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -129,7 +129,7 @@ ifndef DEBUG
endif
ifeq ($(DEBUG),0)
- CFLAGS += -O6
+ CFLAGS += -Og
endif
ifdef PARSER_DEBUG
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 201f6c4c..29792d2 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -397,7 +397,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
const char *name)
{
struct rb_node *n;
- struct symbol_name_rb_node *s;
+ struct symbol_name_rb_node *s = NULL;
if (symbols == NULL)
return NULL;
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 25d6c73..d495741 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
char *line;
char *next = NULL;
char *addr_str;
- char *fmt;
+ char *fmt = NULL;
line = strtok_r(file, "\n", &next);
while (line) {
--
2.1.4
Em Tue, May 19, 2015 at 03:41:03PM +0200, Martin Liška escreveu:
> Following patch is fix for wrong DEBUG configuration.
Can you please state, in the changelog, what you think is wrong and how
your proposed change fixes it, so that after reading your intent I can
go and look at the code to see if it matches that?
- Arnaldo
> Signed-off-by: Martin Liska <[email protected]>
> ---
> tools/perf/arch/common.c | 2 +-
> tools/perf/config/Makefile | 2 +-
> tools/perf/util/symbol.c | 2 +-
> tools/perf/util/trace-event-parse.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> index 49776f1..b7bb42c 100644
> --- a/tools/perf/arch/common.c
> +++ b/tools/perf/arch/common.c
> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
> static bool lookup_path(char *name)
> {
> bool found = false;
> - char *path, *tmp;
> + char *path, *tmp = NULL;
> char buf[PATH_MAX];
> char *env = getenv("PATH");
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index 59a98c6..b708ae0 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -129,7 +129,7 @@ ifndef DEBUG
> endif
> ifeq ($(DEBUG),0)
> - CFLAGS += -O6
> + CFLAGS += -Og
> endif
> ifdef PARSER_DEBUG
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 201f6c4c..29792d2 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -397,7 +397,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
> const char *name)
> {
> struct rb_node *n;
> - struct symbol_name_rb_node *s;
> + struct symbol_name_rb_node *s = NULL;
> if (symbols == NULL)
> return NULL;
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index 25d6c73..d495741 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
> char *line;
> char *next = NULL;
> char *addr_str;
> - char *fmt;
> + char *fmt = NULL;
> line = strtok_r(file, "\n", &next);
> while (line) {
> --
> 2.1.4
On 05/19/2015 04:04 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, May 19, 2015 at 03:41:03PM +0200, Martin Liška escreveu:
>> Following patch is fix for wrong DEBUG configuration.
>
> Can you please state, in the changelog, what you think is wrong and how
> your proposed change fixes it, so that after reading your intent I can
> go and look at the code to see if it matches that?
>
> - Arnaldo
>
>> Signed-off-by: Martin Liska <[email protected]>
>> ---
>> tools/perf/arch/common.c | 2 +-
>> tools/perf/config/Makefile | 2 +-
>> tools/perf/util/symbol.c | 2 +-
>> tools/perf/util/trace-event-parse.c | 2 +-
>> 4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
>> index 49776f1..b7bb42c 100644
>> --- a/tools/perf/arch/common.c
>> +++ b/tools/perf/arch/common.c
>> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
>> static bool lookup_path(char *name)
>> {
>> bool found = false;
>> - char *path, *tmp;
>> + char *path, *tmp = NULL;
>> char buf[PATH_MAX];
>> char *env = getenv("PATH");
>> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
>> index 59a98c6..b708ae0 100644
>> --- a/tools/perf/config/Makefile
>> +++ b/tools/perf/config/Makefile
>> @@ -129,7 +129,7 @@ ifndef DEBUG
>> endif
>> ifeq ($(DEBUG),0)
>> - CFLAGS += -O6
>> + CFLAGS += -Og
>> endif
>> ifdef PARSER_DEBUG
>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>> index 201f6c4c..29792d2 100644
>> --- a/tools/perf/util/symbol.c
>> +++ b/tools/perf/util/symbol.c
>> @@ -397,7 +397,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
>> const char *name)
>> {
>> struct rb_node *n;
>> - struct symbol_name_rb_node *s;
>> + struct symbol_name_rb_node *s = NULL;
>> if (symbols == NULL)
>> return NULL;
>> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
>> index 25d6c73..d495741 100644
>> --- a/tools/perf/util/trace-event-parse.c
>> +++ b/tools/perf/util/trace-event-parse.c
>> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
>> char *line;
>> char *next = NULL;
>> char *addr_str;
>> - char *fmt;
>> + char *fmt = NULL;
>> line = strtok_r(file, "\n", &next);
>> while (line) {
>> --
>> 2.1.4
Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
experience is given by passing -Og to compiler.
Assign default value for pointers that are identified by compiler as
non-initialized.
Signed-off-by: Martin Liska <[email protected]>
Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
> On 05/19/2015 04:04 PM, Arnaldo Carvalho de Melo wrote:
> >Em Tue, May 19, 2015 at 03:41:03PM +0200, Martin Liška escreveu:
> >>Following patch is fix for wrong DEBUG configuration.
> >
> >Can you please state, in the changelog, what you think is wrong and how
> >your proposed change fixes it, so that after reading your intent I can
> >go and look at the code to see if it matches that?
Ok, that improved things, thanks! A question:
>
> Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
> experience is given by passing -Og to compiler.
Is this is this -Og available in old gcc versions? When was it
introduced? Do you know?
- Arnaldo
> Assign default value for pointers that are identified by compiler as
> non-initialized.
>
> Signed-off-by: Martin Liska <[email protected]>
> >From b5f2b42b279ab497e92d41e1f4021dd5cb6ae3ce Mon Sep 17 00:00:00 2001
> From: mliska <[email protected]>
> Date: Thu, 2 Apr 2015 15:24:49 +0200
> Subject: [PATCH] perf: fix wrong DEBUG configuration
>
> Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
> experience is given by passing -Og to compiler.
> Assign default value for pointers that are identified by compiler as
> non-initialized.
>
> Signed-off-by: Martin Liska <[email protected]>
> ---
> tools/perf/arch/common.c | 2 +-
> tools/perf/config/Makefile | 2 +-
> tools/perf/util/symbol.c | 2 +-
> tools/perf/util/trace-event-parse.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> index 49776f1..b7bb42c 100644
> --- a/tools/perf/arch/common.c
> +++ b/tools/perf/arch/common.c
> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
> static bool lookup_path(char *name)
> {
> bool found = false;
> - char *path, *tmp;
> + char *path, *tmp = NULL;
> char buf[PATH_MAX];
> char *env = getenv("PATH");
>
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index e3b3724..ca8d3e3 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -129,7 +129,7 @@ ifndef DEBUG
> endif
>
> ifeq ($(DEBUG),0)
> - CFLAGS += -O6
> + CFLAGS += -Og
> endif
>
> ifdef PARSER_DEBUG
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 82a31fd..a19fbd4 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
> const char *name)
> {
> struct rb_node *n;
> - struct symbol_name_rb_node *s;
> + struct symbol_name_rb_node *s = NULL;
>
> if (symbols == NULL)
> return NULL;
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index 25d6c73..d495741 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
> char *line;
> char *next = NULL;
> char *addr_str;
> - char *fmt;
> + char *fmt = NULL;
>
> line = strtok_r(file, "\n", &next);
> while (line) {
> --
> 2.1.4
>
On 05/20/2015 03:17 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
>> On 05/19/2015 04:04 PM, Arnaldo Carvalho de Melo wrote:
>>> Em Tue, May 19, 2015 at 03:41:03PM +0200, Martin Liška escreveu:
>>>> Following patch is fix for wrong DEBUG configuration.
>>>
>>> Can you please state, in the changelog, what you think is wrong and how
>>> your proposed change fixes it, so that after reading your intent I can
>>> go and look at the code to see if it matches that?
>
> Ok, that improved things, thanks! A question:
>
>>
>> Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
>> experience is given by passing -Og to compiler.
>
> Is this is this -Og available in old gcc versions? When was it
> introduced? Do you know?
>
> - Arnaldo
Hi
GCC 4.8.0 is the first version capable of the option: https://gcc.gnu.org/gcc-4.8/changes.html.
That can be problematic, which GCC version do you support in linux/perf?
Martin
>
>> Assign default value for pointers that are identified by compiler as
>> non-initialized.
>>
>> Signed-off-by: Martin Liska <[email protected]>
>
>> >From b5f2b42b279ab497e92d41e1f4021dd5cb6ae3ce Mon Sep 17 00:00:00 2001
>> From: mliska <[email protected]>
>> Date: Thu, 2 Apr 2015 15:24:49 +0200
>> Subject: [PATCH] perf: fix wrong DEBUG configuration
>>
>> Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
>> experience is given by passing -Og to compiler.
>> Assign default value for pointers that are identified by compiler as
>> non-initialized.
>>
>> Signed-off-by: Martin Liska <[email protected]>
>> ---
>> tools/perf/arch/common.c | 2 +-
>> tools/perf/config/Makefile | 2 +-
>> tools/perf/util/symbol.c | 2 +-
>> tools/perf/util/trace-event-parse.c | 2 +-
>> 4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
>> index 49776f1..b7bb42c 100644
>> --- a/tools/perf/arch/common.c
>> +++ b/tools/perf/arch/common.c
>> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
>> static bool lookup_path(char *name)
>> {
>> bool found = false;
>> - char *path, *tmp;
>> + char *path, *tmp = NULL;
>> char buf[PATH_MAX];
>> char *env = getenv("PATH");
>>
>> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
>> index e3b3724..ca8d3e3 100644
>> --- a/tools/perf/config/Makefile
>> +++ b/tools/perf/config/Makefile
>> @@ -129,7 +129,7 @@ ifndef DEBUG
>> endif
>>
>> ifeq ($(DEBUG),0)
>> - CFLAGS += -O6
>> + CFLAGS += -Og
>> endif
>>
>> ifdef PARSER_DEBUG
>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>> index 82a31fd..a19fbd4 100644
>> --- a/tools/perf/util/symbol.c
>> +++ b/tools/perf/util/symbol.c
>> @@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
>> const char *name)
>> {
>> struct rb_node *n;
>> - struct symbol_name_rb_node *s;
>> + struct symbol_name_rb_node *s = NULL;
>>
>> if (symbols == NULL)
>> return NULL;
>> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
>> index 25d6c73..d495741 100644
>> --- a/tools/perf/util/trace-event-parse.c
>> +++ b/tools/perf/util/trace-event-parse.c
>> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
>> char *line;
>> char *next = NULL;
>> char *addr_str;
>> - char *fmt;
>> + char *fmt = NULL;
>>
>> line = strtok_r(file, "\n", &next);
>> while (line) {
>> --
>> 2.1.4
>>
>
Em Wed, May 20, 2015 at 03:29:37PM +0200, Martin Liška escreveu:
> On 05/20/2015 03:17 PM, Arnaldo Carvalho de Melo wrote:
> >Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
> >>Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
> >>experience is given by passing -Og to compiler.
> >Is this is this -Og available in old gcc versions? When was it
> >introduced? Do you know?
> GCC 4.8.0 is the first version capable of the option: https://gcc.gnu.org/gcc-4.8/changes.html.
> That can be problematic, which GCC version do you support in linux/perf?
So the rule has been: What are the kernel requirements for the
toolchain? tools/perf/ should build with that.
Looking at the kernel's README, that is...
-------------
COMPILING the kernel:
- Make sure you have at least gcc 3.2 available.
For more information, refer to Documentation/Changes.
--------------
The oldest toolchain I personally test from time to time is:
[acme@rhel5 ~]$ rpm -q gcc make binutils
gcc-4.1.2-55.el5
make-3.81-3.el5
binutils-2.17.50.0.6-26.el5
[acme@rhel5 ~]$
- Arnaldo
* Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Wed, May 20, 2015 at 03:29:37PM +0200, Martin Liška escreveu:
> > On 05/20/2015 03:17 PM, Arnaldo Carvalho de Melo wrote:
> > >Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
> > >>Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
> > >>experience is given by passing -Og to compiler.
>
> > >Is this is this -Og available in old gcc versions? When was it
> > >introduced? Do you know?
>
> > GCC 4.8.0 is the first version capable of the option: https://gcc.gnu.org/gcc-4.8/changes.html.
> > That can be problematic, which GCC version do you support in linux/perf?
>
> So the rule has been: What are the kernel requirements for the
> toolchain? tools/perf/ should build with that.
So we could use -Og if it works, like Kbuild does it:
KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
the 'cc-option' Make function does some magic of silently calling GCC
with that option and observing the result.
See:
scripts/Kbuild.include:cc-option = $(call try-run,\
et al.
Thanks,
Ingo
On 05/20/2015 04:55 PM, Ingo Molnar wrote:
>
> * Arnaldo Carvalho de Melo <[email protected]> wrote:
>
>> Em Wed, May 20, 2015 at 03:29:37PM +0200, Martin Liška escreveu:
>>> On 05/20/2015 03:17 PM, Arnaldo Carvalho de Melo wrote:
>>>> Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
>>>>> Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
>>>>> experience is given by passing -Og to compiler.
>>
>>>> Is this is this -Og available in old gcc versions? When was it
>>>> introduced? Do you know?
>>
>>> GCC 4.8.0 is the first version capable of the option: https://gcc.gnu.org/gcc-4.8/changes.html.
>>> That can be problematic, which GCC version do you support in linux/perf?
>>
>> So the rule has been: What are the kernel requirements for the
>> toolchain? tools/perf/ should build with that.
>
> So we could use -Og if it works, like Kbuild does it:
>
> KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
>
> the 'cc-option' Make function does some magic of silently calling GCC
> with that option and observing the result.
>
> See:
>
> scripts/Kbuild.include:cc-option = $(call try-run,\
>
> et al.
>
> Thanks,
>
> Ingo
>
Hi.
I am sorry, I did mistake in understanding of DEBUG variable.
Following patch should be fixed, except missing auto-detection
of -Og option.
Unfortunately, following hunk does not work, no -Ox is added to CFLAGS?
-- CFLAGS += -Og
++ CFLAGS += $(call cc-option,-Og,-O0)
Thanks,
Martin
Em Wed, May 20, 2015 at 06:16:51PM +0200, Martin Liška escreveu:
> On 05/20/2015 04:55 PM, Ingo Molnar wrote:
> >* Arnaldo Carvalho de Melo <[email protected]> wrote:
> >>So the rule has been: What are the kernel requirements for the
> >>toolchain? tools/perf/ should build with that.
> >
> >So we could use -Og if it works, like Kbuild does it:
> >
> > KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
> >
> >the 'cc-option' Make function does some magic of silently calling GCC
> >with that option and observing the result.
> >See:
> > scripts/Kbuild.include:cc-option = $(call try-run,\
> I am sorry, I did mistake in understanding of DEBUG variable.
> Following patch should be fixed, except missing auto-detection
> of -Og option.
>
> Unfortunately, following hunk does not work, no -Ox is added to CFLAGS?
>
> -- CFLAGS += -Og
> ++ CFLAGS += $(call cc-option,-Og,-O0)
I don't know if we have this cc-option, perhaps Ingo is suggesting we
get it in tools/build/? Or include scripts/Kbuild.include and then use
it?
I.e. we have checks to see if we can use, for instance
-fstack-protector-all, see tools/build/feature/Makefile, usong this
cc-option thing, importing it from Kbuild would solve the issue at
hand in a definitive way and in line with what we have been pursuing:
to make the tools/ build system be based on Kbuild.
- Arnaldo
* Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Wed, May 20, 2015 at 06:16:51PM +0200, Martin Liška escreveu:
> > On 05/20/2015 04:55 PM, Ingo Molnar wrote:
> > >* Arnaldo Carvalho de Melo <[email protected]> wrote:
> > >>So the rule has been: What are the kernel requirements for the
> > >>toolchain? tools/perf/ should build with that.
> > >
> > >So we could use -Og if it works, like Kbuild does it:
> > >
> > > KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
> > >
> > >the 'cc-option' Make function does some magic of silently calling GCC
> > >with that option and observing the result.
>
> > >See:
>
> > > scripts/Kbuild.include:cc-option = $(call try-run,\
>
> > I am sorry, I did mistake in understanding of DEBUG variable.
> > Following patch should be fixed, except missing auto-detection
> > of -Og option.
> >
> > Unfortunately, following hunk does not work, no -Ox is added to CFLAGS?
> >
> > -- CFLAGS += -Og
> > ++ CFLAGS += $(call cc-option,-Og,-O0)
>
> I don't know if we have this cc-option, perhaps Ingo is suggesting
> we get it in tools/build/? Or include scripts/Kbuild.include and
> then use it?
>
> I.e. we have checks to see if we can use, for instance
> -fstack-protector-all, see tools/build/feature/Makefile, using this
> cc-option thing, importing it from Kbuild would solve the issue at
> hand in a definitive way and in line with what we have been
> pursuing: to make the tools/ build system be based on Kbuild.
So I'd suggest copying any necessary functions instead of outright
including all of Kbuild in the tooling build system which creates
non-trivial dependencies that is not necessarily tested as thoroughly
on the kbuild side as on the tooling side.
Thanks,
Ingo
On 05/22/2015 09:02 AM, Ingo Molnar wrote:
>
> * Arnaldo Carvalho de Melo <[email protected]> wrote:
>
>> Em Wed, May 20, 2015 at 06:16:51PM +0200, Martin Liška escreveu:
>>> On 05/20/2015 04:55 PM, Ingo Molnar wrote:
>>>> * Arnaldo Carvalho de Melo <[email protected]> wrote:
>>>>> So the rule has been: What are the kernel requirements for the
>>>>> toolchain? tools/perf/ should build with that.
>>>>
>>>> So we could use -Og if it works, like Kbuild does it:
>>>>
>>>> KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
>>>>
>>>> the 'cc-option' Make function does some magic of silently calling GCC
>>>> with that option and observing the result.
>>
>>>> See:
>>
>>>> scripts/Kbuild.include:cc-option = $(call try-run,\
>>
>>> I am sorry, I did mistake in understanding of DEBUG variable.
>>> Following patch should be fixed, except missing auto-detection
>>> of -Og option.
>>>
>>> Unfortunately, following hunk does not work, no -Ox is added to CFLAGS?
>>>
>>> -- CFLAGS += -Og
>>> ++ CFLAGS += $(call cc-option,-Og,-O0)
>>
>> I don't know if we have this cc-option, perhaps Ingo is suggesting
>> we get it in tools/build/? Or include scripts/Kbuild.include and
>> then use it?
>>
>> I.e. we have checks to see if we can use, for instance
>> -fstack-protector-all, see tools/build/feature/Makefile, using this
>> cc-option thing, importing it from Kbuild would solve the issue at
>> hand in a definitive way and in line with what we have been
>> pursuing: to make the tools/ build system be based on Kbuild.
>
> So I'd suggest copying any necessary functions instead of outright
> including all of Kbuild in the tooling build system which creates
> non-trivial dependencies that is not necessarily tested as thoroughly
> on the kbuild side as on the tooling side.
>
> Thanks,
>
> Ingo
>
May I ask you Ingo for helping with that? It would be easy to test if you
grab my patch and add necessary bash functions.
Thank you,
Martin
On 05/22/2015 09:02 AM, Ingo Molnar wrote:
>
> * Arnaldo Carvalho de Melo <[email protected]> wrote:
>
>> Em Wed, May 20, 2015 at 06:16:51PM +0200, Martin Liška escreveu:
>>> On 05/20/2015 04:55 PM, Ingo Molnar wrote:
>>>> * Arnaldo Carvalho de Melo <[email protected]> wrote:
>>>>> So the rule has been: What are the kernel requirements for the
>>>>> toolchain? tools/perf/ should build with that.
>>>>
>>>> So we could use -Og if it works, like Kbuild does it:
>>>>
>>>> KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
>>>>
>>>> the 'cc-option' Make function does some magic of silently calling GCC
>>>> with that option and observing the result.
>>
>>>> See:
>>
>>>> scripts/Kbuild.include:cc-option = $(call try-run,\
>>
>>> I am sorry, I did mistake in understanding of DEBUG variable.
>>> Following patch should be fixed, except missing auto-detection
>>> of -Og option.
>>>
>>> Unfortunately, following hunk does not work, no -Ox is added to CFLAGS?
>>>
>>> -- CFLAGS += -Og
>>> ++ CFLAGS += $(call cc-option,-Og,-O0)
>>
>> I don't know if we have this cc-option, perhaps Ingo is suggesting
>> we get it in tools/build/? Or include scripts/Kbuild.include and
>> then use it?
>>
>> I.e. we have checks to see if we can use, for instance
>> -fstack-protector-all, see tools/build/feature/Makefile, using this
>> cc-option thing, importing it from Kbuild would solve the issue at
>> hand in a definitive way and in line with what we have been
>> pursuing: to make the tools/ build system be based on Kbuild.
>
> So I'd suggest copying any necessary functions instead of outright
> including all of Kbuild in the tooling build system which creates
> non-trivial dependencies that is not necessarily tested as thoroughly
> on the kbuild side as on the tooling side.
>
> Thanks,
>
> Ingo
>
Hi.
Thanks for advice, I copied necessary functions to include/utilities.mak.
Following patch works for, tested both with a compiler capable of -Og and
a not capable one.
Thanks,
Martin
* Martin Liška <[email protected]> wrote:
> Currently, GCC optimizes -O6 same as -O3 level, thus change the value
> to -O6.
s/to -O6
to -O3
> Right optimize debugging experience is given by passing -Og to
> compiler. Assign default value for pointers that are identified by
> compiler as non-initialized.
s/Right optimize debugging experience is given/
Correct debugging experience is given/
s/identified by compiler
identified by the compiler
> ifeq ($(DEBUG),0)
> - CFLAGS += -O6
> + CFLAGS += -O3
> +else
> + CFLAGS += $(call cc-option,-Og,-O0)
> endif
> +# try-run
> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
> +# Exit code chooses option. "$$TMP" is can be used as temporary file and
> +# is automatically cleaned up.
> +try-run = $(shell set -e; \
> + TMP="$(TMPOUT).$$$$.tmp"; \
> + TMPO="$(TMPOUT).$$$$.o"; \
> + if ($(1)) >/dev/null 2>&1; \
> + then echo "$(2)"; \
> + else echo "$(3)"; \
> + fi; \
> + rm -f "$$TMP" "$$TMPO")
> +
> +# cc-option
> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
> +
> +cc-option = $(call try-run,\
> + $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
Looks good to me!
Acked-by: Ingo Molnar <[email protected]>
Thanks,
Ingo
On 05/25/2015 12:47 PM, Ingo Molnar wrote:
>
> * Martin Liška <[email protected]> wrote:
>
>> Currently, GCC optimizes -O6 same as -O3 level, thus change the value
>> to -O6.
>
> s/to -O6
> to -O3
>
>> Right optimize debugging experience is given by passing -Og to
>> compiler. Assign default value for pointers that are identified by
>> compiler as non-initialized.
>
> s/Right optimize debugging experience is given/
> Correct debugging experience is given/
>
> s/identified by compiler
> identified by the compiler
>
>> ifeq ($(DEBUG),0)
>> - CFLAGS += -O6
>> + CFLAGS += -O3
>> +else
>> + CFLAGS += $(call cc-option,-Og,-O0)
>> endif
>
>> +# try-run
>> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
>> +# Exit code chooses option. "$$TMP" is can be used as temporary file and
>> +# is automatically cleaned up.
>> +try-run = $(shell set -e; \
>> + TMP="$(TMPOUT).$$$$.tmp"; \
>> + TMPO="$(TMPOUT).$$$$.o"; \
>> + if ($(1)) >/dev/null 2>&1; \
>> + then echo "$(2)"; \
>> + else echo "$(3)"; \
>> + fi; \
>> + rm -f "$$TMP" "$$TMPO")
>> +
>> +# cc-option
>> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
>> +
>> +cc-option = $(call try-run,\
>> + $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
>
> Looks good to me!
>
> Acked-by: Ingo Molnar <[email protected]>
>
> Thanks,
>
> Ingo
>
Thank you for review.
This is final version of the patch, where I appended your acknowledgment.
Martin
* Martin Liška <[email protected]> wrote:
> >>Right optimize debugging experience is given by passing -Og to
> >>compiler. Assign default value for pointers that are identified by
> >>compiler as non-initialized.
> >
> >s/Right optimize debugging experience is given/
> > Correct debugging experience is given/
> Right debugging experience is given by passing -Og to compiler.
So I fixed the spelling here once already :-/ If you want to use
'right' in this context then use it like this:
The right debugging experience is given by ...
Or you can use what I suggested first:
Correct debugging experience is given by ...
Thanks,
Ingo
On 05/25/2015 01:09 PM, Ingo Molnar wrote:
>
> * Martin Liška <[email protected]> wrote:
>
>>>> Right optimize debugging experience is given by passing -Og to
>>>> compiler. Assign default value for pointers that are identified by
>>>> compiler as non-initialized.
>>>
>>> s/Right optimize debugging experience is given/
>>> Correct debugging experience is given/
>
>> Right debugging experience is given by passing -Og to compiler.
>
> So I fixed the spelling here once already :-/ If you want to use
> 'right' in this context then use it like this:
>
> The right debugging experience is given by ...
>
> Or you can use what I suggested first:
>
> Correct debugging experience is given by ...
>
> Thanks,
>
> Ingo
>
Sorry Ingo for that, I overlooked that correction :)
Attached version should reflect that.
Thanks,
Martin
Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
> On 05/25/2015 01:09 PM, Ingo Molnar wrote:
> >
> >* Martin Liška <[email protected]> wrote:
> >
> >>>>Right optimize debugging experience is given by passing -Og to
> >>>>compiler. Assign default value for pointers that are identified by
> >>>>compiler as non-initialized.
> >>>
> >>>s/Right optimize debugging experience is given/
> >>> Correct debugging experience is given/
> >
> >>Right debugging experience is given by passing -Og to compiler.
> >
> >So I fixed the spelling here once already :-/ If you want to use
> >'right' in this context then use it like this:
> >
> > The right debugging experience is given by ...
> >
> >Or you can use what I suggested first:
> >
> > Correct debugging experience is given by ...
> >
> >Thanks,
> >
> > Ingo
> >
>
> Sorry Ingo for that, I overlooked that correction :)
Are you ok with this Ingo?
I can apply, but there seems to be two patches folded here, one that
sets the possibly unitiliazed variables to NULL, and could be the first
in the series, and the other, that deals with multiple versions of gcc
and how should we ask something from them.
- Arnaldo
> Attached version should reflect that.
>
> Thanks,
> Martin
> Currently, GCC optimizes -O6 same as -O3 level, thus change the value
> to -O3.
> Correct debugging experience is given by passing -Og to compiler.
> Assign default value for pointers that are identified by the compiler as
> non-initialized.
>
> Signed-off-by: Martin Liska <[email protected]>
> Acked-by: Ingo Molnar <[email protected]>
> ---
> tools/perf/arch/common.c | 2 +-
> tools/perf/config/Makefile | 4 +++-
> tools/perf/config/utilities.mak | 19 +++++++++++++++++++
> tools/perf/util/symbol.c | 2 +-
> tools/perf/util/trace-event-parse.c | 2 +-
> 5 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> index 49776f1..b7bb42c 100644
> --- a/tools/perf/arch/common.c
> +++ b/tools/perf/arch/common.c
> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
> static bool lookup_path(char *name)
> {
> bool found = false;
> - char *path, *tmp;
> + char *path, *tmp = NULL;
> char buf[PATH_MAX];
> char *env = getenv("PATH");
>
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index e3b3724..47e4ae1 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -129,7 +129,9 @@ ifndef DEBUG
> endif
>
> ifeq ($(DEBUG),0)
> - CFLAGS += -O6
> + CFLAGS += -O3
> +else
> + CFLAGS += $(call cc-option,-Og,-O0)
> endif
>
> ifdef PARSER_DEBUG
> diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
> index c16ce83..0ebef09 100644
> --- a/tools/perf/config/utilities.mak
> +++ b/tools/perf/config/utilities.mak
> @@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
> endef
> _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
> _gea_err = $(if $(1),$(error Please set '$(1)' appropriately))
> +
> +# try-run
> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
> +# Exit code chooses option. "$$TMP" is can be used as temporary file and
> +# is automatically cleaned up.
> +try-run = $(shell set -e; \
> + TMP="$(TMPOUT).$$$$.tmp"; \
> + TMPO="$(TMPOUT).$$$$.o"; \
> + if ($(1)) >/dev/null 2>&1; \
> + then echo "$(2)"; \
> + else echo "$(3)"; \
> + fi; \
> + rm -f "$$TMP" "$$TMPO")
> +
> +# cc-option
> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
> +
> +cc-option = $(call try-run,\
> + $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 82a31fd..a19fbd4 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
> const char *name)
> {
> struct rb_node *n;
> - struct symbol_name_rb_node *s;
> + struct symbol_name_rb_node *s = NULL;
>
> if (symbols == NULL)
> return NULL;
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index 25d6c73..d495741 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
> char *line;
> char *next = NULL;
> char *addr_str;
> - char *fmt;
> + char *fmt = NULL;
>
> line = strtok_r(file, "\n", &next);
> while (line) {
> --
> 2.1.4
>
* Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
> > On 05/25/2015 01:09 PM, Ingo Molnar wrote:
> > >
> > >* Martin Liška <[email protected]> wrote:
> > >
> > >>>>Right optimize debugging experience is given by passing -Og to
> > >>>>compiler. Assign default value for pointers that are identified by
> > >>>>compiler as non-initialized.
> > >>>
> > >>>s/Right optimize debugging experience is given/
> > >>> Correct debugging experience is given/
> > >
> > >>Right debugging experience is given by passing -Og to compiler.
> > >
> > >So I fixed the spelling here once already :-/ If you want to use
> > >'right' in this context then use it like this:
> > >
> > > The right debugging experience is given by ...
> > >
> > >Or you can use what I suggested first:
> > >
> > > Correct debugging experience is given by ...
> > >
> > >Thanks,
> > >
> > > Ingo
> > >
> >
> > Sorry Ingo for that, I overlooked that correction :)
>
> Are you ok with this Ingo?
Yeah, certainly!
> I can apply, but there seems to be two patches folded here, one that
> sets the possibly unitiliazed variables to NULL, and could be the
> first in the series, and the other, that deals with multiple
> versions of gcc and how should we ask something from them.
Yeah.
Thanks,
Ingo
On 05/25/2015 08:32 PM, Ingo Molnar wrote:
>
> * Arnaldo Carvalho de Melo <[email protected]> wrote:
>
>> Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
>>> On 05/25/2015 01:09 PM, Ingo Molnar wrote:
>>>>
>>>> * Martin Liška <[email protected]> wrote:
>>>>
>>>>>>> Right optimize debugging experience is given by passing -Og to
>>>>>>> compiler. Assign default value for pointers that are identified by
>>>>>>> compiler as non-initialized.
>>>>>>
>>>>>> s/Right optimize debugging experience is given/
>>>>>> Correct debugging experience is given/
>>>>
>>>>> Right debugging experience is given by passing -Og to compiler.
>>>>
>>>> So I fixed the spelling here once already :-/ If you want to use
>>>> 'right' in this context then use it like this:
>>>>
>>>> The right debugging experience is given by ...
>>>>
>>>> Or you can use what I suggested first:
>>>>
>>>> Correct debugging experience is given by ...
>>>>
>>>> Thanks,
>>>>
>>>> Ingo
>>>>
>>>
>>> Sorry Ingo for that, I overlooked that correction :)
>>
>> Are you ok with this Ingo?
>
> Yeah, certainly!
>
>> I can apply, but there seems to be two patches folded here, one that
>> sets the possibly unitiliazed variables to NULL, and could be the
>> first in the series, and the other, that deals with multiple
>> versions of gcc and how should we ask something from them.
>
> Yeah.
>
> Thanks,
>
> Ingo
>
Hello.
As Arnaldo pointed, I split the patch to following 2 smaller patches.
Thanks,
Martin
Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
> On 05/25/2015 08:32 PM, Ingo Molnar wrote:
> >
> >* Arnaldo Carvalho de Melo <[email protected]> wrote:
> >
> >>Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
> >>>On 05/25/2015 01:09 PM, Ingo Molnar wrote:
> >>>>
> >>>>* Martin Liška <[email protected]> wrote:
> >>>>
> >>>>>>>Right optimize debugging experience is given by passing -Og to
> >>>>>>>compiler. Assign default value for pointers that are identified by
> >>>>>>>compiler as non-initialized.
> >>>>>>
> >>>>>>s/Right optimize debugging experience is given/
> >>>>>> Correct debugging experience is given/
> >>>>
> >>>>>Right debugging experience is given by passing -Og to compiler.
> >>>>
> >>>>So I fixed the spelling here once already :-/ If you want to use
> >>>>'right' in this context then use it like this:
> >>>>
> >>>> The right debugging experience is given by ...
> >>>>
> >>>>Or you can use what I suggested first:
> >>>>
> >>>> Correct debugging experience is given by ...
> >>>>
> >>>>Thanks,
> >>>>
> >>>> Ingo
> >>>>
> >>>
> >>>Sorry Ingo for that, I overlooked that correction :)
> >>
> >>Are you ok with this Ingo?
> >
> >Yeah, certainly!
> >
> >>I can apply, but there seems to be two patches folded here, one that
> >>sets the possibly unitiliazed variables to NULL, and could be the
> >>first in the series, and the other, that deals with multiple
> >>versions of gcc and how should we ask something from them.
> >
> >Yeah.
> >
> >Thanks,
> >
> > Ingo
> >
>
> Hello.
>
> As Arnaldo pointed, I split the patch to following 2 smaller patches.
Thanks, but how did you generated the two attached patches?
[acme@ssdandy linux]$ git am /wb/0001-perf-Fix-compiler-warnings.patch
Patch format detection failed.
So I'll fix it up manually, next time please try:
git format-patch -n HEAD^^..
Or equivalent,
- Arnaldo
> Thanks,
> Martin
> Assign default value for pointers that are identified by the compiler as
> non-initialized.
>
> Signed-off-by: Martin Liska <[email protected]>
> Acked-by: Ingo Molnar <[email protected]>
> ---
> tools/perf/arch/common.c | 2 +-
> tools/perf/util/symbol.c | 2 +-
> tools/perf/util/trace-event-parse.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> index 49776f1..b7bb42c 100644
> --- a/tools/perf/arch/common.c
> +++ b/tools/perf/arch/common.c
> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
> static bool lookup_path(char *name)
> {
> bool found = false;
> - char *path, *tmp;
> + char *path, *tmp = NULL;
> char buf[PATH_MAX];
> char *env = getenv("PATH");
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 82a31fd..a19fbd4 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
> const char *name)
> {
> struct rb_node *n;
> - struct symbol_name_rb_node *s;
> + struct symbol_name_rb_node *s = NULL;
>
> if (symbols == NULL)
> return NULL;
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index 25d6c73..d495741 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
> char *line;
> char *next = NULL;
> char *addr_str;
> - char *fmt;
> + char *fmt = NULL;
>
> line = strtok_r(file, "\n", &next);
> while (line) {
> --
> 2.1.4
>
> Currently, GCC optimizes -O6 same as -O3 level, thus change the value
> to -O3.
> Correct debugging experience is given by passing -Og to compiler.
>
> Signed-off-by: Martin Liska <[email protected]>
> Acked-by: Ingo Molnar <[email protected]>
> ---
> tools/perf/config/Makefile | 4 +++-
> tools/perf/config/utilities.mak | 19 +++++++++++++++++++
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index e3b3724..47e4ae1 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -129,7 +129,9 @@ ifndef DEBUG
> endif
>
> ifeq ($(DEBUG),0)
> - CFLAGS += -O6
> + CFLAGS += -O3
> +else
> + CFLAGS += $(call cc-option,-Og,-O0)
> endif
>
> ifdef PARSER_DEBUG
> diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
> index c16ce83..0ebef09 100644
> --- a/tools/perf/config/utilities.mak
> +++ b/tools/perf/config/utilities.mak
> @@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
> endef
> _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
> _gea_err = $(if $(1),$(error Please set '$(1)' appropriately))
> +
> +# try-run
> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
> +# Exit code chooses option. "$$TMP" is can be used as temporary file and
> +# is automatically cleaned up.
> +try-run = $(shell set -e; \
> + TMP="$(TMPOUT).$$$$.tmp"; \
> + TMPO="$(TMPOUT).$$$$.o"; \
> + if ($(1)) >/dev/null 2>&1; \
> + then echo "$(2)"; \
> + else echo "$(3)"; \
> + fi; \
> + rm -f "$$TMP" "$$TMPO")
> +
> +# cc-option
> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
> +
> +cc-option = $(call try-run,\
> + $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
> --
> 2.1.4
>
Em Tue, May 26, 2015 at 11:36:26AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
> > As Arnaldo pointed, I split the patch to following 2 smaller patches.
>
> Thanks, but how did you generated the two attached patches?
>
> [acme@ssdandy linux]$ git am /wb/0001-perf-Fix-compiler-warnings.patch
> Patch format detection failed.
>
> So I'll fix it up manually, next time please try:
>
> git format-patch -n HEAD^^..
>
> Or equivalent,
I am also splitting the second patch into two, as it does two unrelated
things:
1. Conditionally use -Og instead of -O0
2. Use -O3 instead of -O6, as -O6 is the same as -O3 currently
And I am not applying the second, as it doesn't fixes anything nor
improves the current situation, right?
- Arnaldo
On 05/26/2015 05:22 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, May 26, 2015 at 11:36:26AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
>>> As Arnaldo pointed, I split the patch to following 2 smaller patches.
>>
>> Thanks, but how did you generated the two attached patches?
>>
>> [acme@ssdandy linux]$ git am /wb/0001-perf-Fix-compiler-warnings.patch
>> Patch format detection failed.
>>
>> So I'll fix it up manually, next time please try:
>>
>> git format-patch -n HEAD^^..
>>
>> Or equivalent,
>
> I am also splitting the second patch into two, as it does two unrelated
> things:
>
> 1. Conditionally use -Og instead of -O0
>
> 2. Use -O3 instead of -O6, as -O6 is the same as -O3 currently
>
> And I am not applying the second, as it doesn't fixes anything nor
> improves the current situation, right?
Hi.
Works for me, the first one is more important.
Martin
>
> - Arnaldo
>
On 05/26/2015 04:36 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
>> On 05/25/2015 08:32 PM, Ingo Molnar wrote:
>>>
>>> * Arnaldo Carvalho de Melo <[email protected]> wrote:
>>>
>>>> Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
>>>>> On 05/25/2015 01:09 PM, Ingo Molnar wrote:
>>>>>>
>>>>>> * Martin Liška <[email protected]> wrote:
>>>>>>
>>>>>>>>> Right optimize debugging experience is given by passing -Og to
>>>>>>>>> compiler. Assign default value for pointers that are identified by
>>>>>>>>> compiler as non-initialized.
>>>>>>>>
>>>>>>>> s/Right optimize debugging experience is given/
>>>>>>>> Correct debugging experience is given/
>>>>>>
>>>>>>> Right debugging experience is given by passing -Og to compiler.
>>>>>>
>>>>>> So I fixed the spelling here once already :-/ If you want to use
>>>>>> 'right' in this context then use it like this:
>>>>>>
>>>>>> The right debugging experience is given by ...
>>>>>>
>>>>>> Or you can use what I suggested first:
>>>>>>
>>>>>> Correct debugging experience is given by ...
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Ingo
>>>>>>
>>>>>
>>>>> Sorry Ingo for that, I overlooked that correction :)
>>>>
>>>> Are you ok with this Ingo?
>>>
>>> Yeah, certainly!
>>>
>>>> I can apply, but there seems to be two patches folded here, one that
>>>> sets the possibly unitiliazed variables to NULL, and could be the
>>>> first in the series, and the other, that deals with multiple
>>>> versions of gcc and how should we ask something from them.
>>>
>>> Yeah.
>>>
>>> Thanks,
>>>
>>> Ingo
>>>
>>
>> Hello.
>>
>> As Arnaldo pointed, I split the patch to following 2 smaller patches.
>
> Thanks, but how did you generated the two attached patches?
>
> [acme@ssdandy linux]$ git am /wb/0001-perf-Fix-compiler-warnings.patch
> Patch format detection failed.
>
> So I'll fix it up manually, next time please try:
>
> git format-patch -n HEAD^^..
>
> Or equivalent,
>
> - Arnaldo
Hi.
That's quite strange, I've just rebuilt my branch to perf/perf/core and
I was given equal patches for:
'git format-patch -2'
Maybe that's related to erased patch header(From, Date, Subject).
Thanks for re-application,
Martin
>
>
>
>> Thanks,
>> Martin
>
>> Assign default value for pointers that are identified by the compiler as
>> non-initialized.
>>
>> Signed-off-by: Martin Liska <[email protected]>
>> Acked-by: Ingo Molnar <[email protected]>
>> ---
>> tools/perf/arch/common.c | 2 +-
>> tools/perf/util/symbol.c | 2 +-
>> tools/perf/util/trace-event-parse.c | 2 +-
>> 3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
>> index 49776f1..b7bb42c 100644
>> --- a/tools/perf/arch/common.c
>> +++ b/tools/perf/arch/common.c
>> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
>> static bool lookup_path(char *name)
>> {
>> bool found = false;
>> - char *path, *tmp;
>> + char *path, *tmp = NULL;
>> char buf[PATH_MAX];
>> char *env = getenv("PATH");
>>
>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>> index 82a31fd..a19fbd4 100644
>> --- a/tools/perf/util/symbol.c
>> +++ b/tools/perf/util/symbol.c
>> @@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
>> const char *name)
>> {
>> struct rb_node *n;
>> - struct symbol_name_rb_node *s;
>> + struct symbol_name_rb_node *s = NULL;
>>
>> if (symbols == NULL)
>> return NULL;
>> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
>> index 25d6c73..d495741 100644
>> --- a/tools/perf/util/trace-event-parse.c
>> +++ b/tools/perf/util/trace-event-parse.c
>> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
>> char *line;
>> char *next = NULL;
>> char *addr_str;
>> - char *fmt;
>> + char *fmt = NULL;
>>
>> line = strtok_r(file, "\n", &next);
>> while (line) {
>> --
>> 2.1.4
>>
>
>> Currently, GCC optimizes -O6 same as -O3 level, thus change the value
>> to -O3.
>> Correct debugging experience is given by passing -Og to compiler.
>>
>> Signed-off-by: Martin Liska <[email protected]>
>> Acked-by: Ingo Molnar <[email protected]>
>> ---
>> tools/perf/config/Makefile | 4 +++-
>> tools/perf/config/utilities.mak | 19 +++++++++++++++++++
>> 2 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
>> index e3b3724..47e4ae1 100644
>> --- a/tools/perf/config/Makefile
>> +++ b/tools/perf/config/Makefile
>> @@ -129,7 +129,9 @@ ifndef DEBUG
>> endif
>>
>> ifeq ($(DEBUG),0)
>> - CFLAGS += -O6
>> + CFLAGS += -O3
>> +else
>> + CFLAGS += $(call cc-option,-Og,-O0)
>> endif
>>
>> ifdef PARSER_DEBUG
>> diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
>> index c16ce83..0ebef09 100644
>> --- a/tools/perf/config/utilities.mak
>> +++ b/tools/perf/config/utilities.mak
>> @@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
>> endef
>> _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
>> _gea_err = $(if $(1),$(error Please set '$(1)' appropriately))
>> +
>> +# try-run
>> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
>> +# Exit code chooses option. "$$TMP" is can be used as temporary file and
>> +# is automatically cleaned up.
>> +try-run = $(shell set -e; \
>> + TMP="$(TMPOUT).$$$$.tmp"; \
>> + TMPO="$(TMPOUT).$$$$.o"; \
>> + if ($(1)) >/dev/null 2>&1; \
>> + then echo "$(2)"; \
>> + else echo "$(3)"; \
>> + fi; \
>> + rm -f "$$TMP" "$$TMPO")
>> +
>> +# cc-option
>> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
>> +
>> +cc-option = $(call try-run,\
>> + $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
>> --
>> 2.1.4
>>
>
Em Tue, May 26, 2015 at 05:48:27PM +0200, Martin Liška escreveu:
> On 05/26/2015 04:36 PM, Arnaldo Carvalho de Melo wrote:
> >Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
> >>>* Arnaldo Carvalho de Melo <[email protected]> wrote:
> >>>>Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
> >>As Arnaldo pointed, I split the patch to following 2 smaller patches.
> >Thanks, but how did you generated the two attached patches?
> > [acme@ssdandy linux]$ git am /wb/0001-perf-Fix-compiler-warnings.patch
> > Patch format detection failed.
> >So I'll fix it up manually, next time please try:
> > git format-patch -n HEAD^^..
> >Or equivalent,
> That's quite strange, I've just rebuilt my branch to perf/perf/core and
> I was given equal patches for:
> 'git format-patch -2'
> Maybe that's related to erased patch header(From, Date, Subject).
Yes, I had to add at least From: and Subject:, but my scripts will pick
as well the Message-ID to create the Link: line, Cc: info to add the CC
and some other stuff that are present when fed with one message per
patch.
> Thanks for re-application,
You're welcome, keep on contributing!
- Arnaldo