perf: fix wrong DEBUG configuration
PATCH 1/1 perf: fix wrong DEBUG configuration
Following patch is fix for wrong DEBUG configuration.
--- 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(-)
tools/perf/arch/common.c +1 -1
@@ -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" );
tools/perf/config/Makefile +1 -1
@@ -129,7 +129,7 @@ ifndef DEBUG endif ifeq ($(DEBUG),0) - CFLAGS += -O6 + CFLAGS += -Og endif ifdef PARSER_DEBUG
tools/perf/util/symbol.c +1 -1
@@ -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 ;
tools/perf/util/trace-event-parse.c +1 -1
@@ -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) {
Signature · 1 line
2.1.4
Arnaldo Carvalho de Melo
maintainerEm Tue, May 19, 2015 at 03:41:03PM +0200, Martin Liška escreveu:
Quoted 1 line
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
Quoted 57 lines
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(-)tools/perf/arch/common.c +1
-1@@ -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" );tools/perf/config/Makefile +1
-1@@ -129,7 +129,7 @@ ifndef DEBUG endif ifeq ($(DEBUG),0) - CFLAGS += -O6 + CFLAGS += -Og endif ifdef PARSER_DEBUGtools/perf/util/symbol.c +1
-1@@ -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 ;tools/perf/util/trace-event-parse.c +1
-1@@ -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) {Signature · 1 line
2.1.4
Quoted 58 lines · Arnaldo Carvalho de Melo
Em Tue, May 19, 2015 at 03:41:03PM +0200, Martin Liška escreveu:
Quoted 1 line
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
Quoted 49 lines
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(-)tools/perf/arch/common.c +1
-1@@ -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" );tools/perf/config/Makefile +1
-1@@ -129,7 +129,7 @@ ifndef DEBUG endif ifeq ($(DEBUG),0) - CFLAGS += -O6 + CFLAGS += -Og endif ifdef PARSER_DEBUGtools/perf/util/symbol.c +1
-1@@ -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 ;tools/perf/util/trace-event-parse.c +0
-0@@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,char *line;
Quoted 8 lines
Quoted 8 lines
tools/perf/util/trace-event-parse.c +1
-2char *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.
Quoted 1 line
From
b5f2b42b279ab497e92d41e1f4021dd5cb6ae3ceMon 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.
--- 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(-)
tools/perf/arch/common.c +1 -1
@@ -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" );
tools/perf/config/Makefile +1 -1
@@ -129,7 +129,7 @@ ifndef DEBUG endif ifeq ($(DEBUG),0) - CFLAGS += -O6 + CFLAGS += -Og endif ifdef PARSER_DEBUG
tools/perf/util/symbol.c +1 -1
@@ -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 ;
tools/perf/util/trace-event-parse.c +1 -1
@@ -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) {
Signature · 1 line
2.1.4
Arnaldo Carvalho de Melo
maintainerEm Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
Quoted 7 lines · Arnaldo Carvalho de Melo
Quoted 6 lines
Em Tue, May 19, 2015 at 03:41:03PM +0200, Martin Liška escreveu:
Quoted 1 line
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:
Quoted 3 lines
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
Quoted 78 lines
Assign default value for pointers that are identified by compiler as non-initialized.
Signed-off-by Martin Liska <[email protected]>Quoted 1 line
From
b5f2b42b279ab497e92d41e1f4021dd5cb6ae3ceMon Sep 17 00:00:00 2001From: mliska <[email protected]>
Date: Thu, 2 Apr 2015 15:24:49 +0200
Subject: [PATCH] perf: fix wrong DEBUG configurationCurrently, 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(-)tools/perf/arch/common.c +1
-1@@ -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" );tools/perf/config/Makefile +1
-1@@ -129,7 +129,7 @@ ifndef DEBUG endif ifeq ($(DEBUG),0) - CFLAGS += -O6 + CFLAGS += -Og endif ifdef PARSER_DEBUGtools/perf/util/symbol.c +1
-1@@ -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 ;tools/perf/util/trace-event-parse.c +1
-1@@ -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) {Signature · 1 line
2.1.4
Quoted 12 lines · Arnaldo Carvalho de Melo
Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
Quoted 7 lines · Arnaldo Carvalho de Melo
Quoted 6 lines
Em Tue, May 19, 2015 at 03:41:03PM +0200, Martin Liška escreveu:
Quoted 1 line
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:
Quoted 1 line
Quoted 7 lines
Quoted 2 lines
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
Quoted 80 lines
Quoted 79 lines
Assign default value for pointers that are identified by compiler as non-initialized.
Signed-off-by Martin Liska <[email protected]>Quoted 1 line
From
b5f2b42b279ab497e92d41e1f4021dd5cb6ae3ceMon Sep 17 00:00:00 2001From: mliska <[email protected]>
Date: Thu, 2 Apr 2015 15:24:49 +0200
Subject: [PATCH] perf: fix wrong DEBUG configurationCurrently, 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(-)tools/perf/arch/common.c +1
-1@@ -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" );tools/perf/config/Makefile +1
-1@@ -129,7 +129,7 @@ ifndef DEBUG endif ifeq ($(DEBUG),0) - CFLAGS += -O6 + CFLAGS += -Og endif ifdef PARSER_DEBUGtools/perf/util/symbol.c +1
-1@@ -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 ;tools/perf/util/trace-event-parse.c +1
-2@@ -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
maintainerEm Wed, May 20, 2015 at 03:29:37PM +0200, Martin Liška escreveu:
Quoted 10 lines · Arnaldo Carvalho de Melo
Quoted 6 lines
Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
Quoted 2 lines
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
Quoted 6 lines · Arnaldo Carvalho de Melo
Em Wed, May 20, 2015 at 03:29:37PM +0200, Martin Liška escreveu:
Quoted 4 lines · Arnaldo Carvalho de Melo
Quoted 3 lines
Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
Quoted 2 lines
Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging experience is given by passing -Og to compiler.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
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
Quoted 27 lines · Ingo Molnar
Quoted 6 lines · Arnaldo Carvalho de Melo
Em Wed, May 20, 2015 at 03:29:37PM +0200, Martin Liška escreveu:
Quoted 4 lines · Arnaldo Carvalho de Melo
Quoted 3 lines
Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
Quoted 2 lines
Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging experience is given by passing -Og to compiler.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
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
Kbuilddoes 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:
Quoted 8 lines
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
Quoted 1 line
From
98ef10fcb27afc3ae7aaab3ebd157ab90b3b1afeMon 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, thus change the value to -O6.
Right optimize debugging experience is given by passing -Og to compiler. Assign default value for pointers that are identified by compiler as non-initialized.
--- tools/perf/arch/common.c | 2 +- tools/perf/config/Makefile | 4 +++- tools/perf/util/symbol.c | 2 +- tools/perf/util/trace-event-parse.c | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-)
tools/perf/arch/common.c +1 -1
@@ -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" );
tools/perf/config/Makefile +3 -1
@@ -129,7 +129,9 @@ ifndef DEBUG endif ifeq ($(DEBUG),0) - CFLAGS += -O6 + CFLAGS += -O3 +else + CFLAGS += -Og endif ifdef PARSER_DEBUG
tools/perf/util/symbol.c +1 -1
@@ -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 ;
tools/perf/util/trace-event-parse.c +1 -1
@@ -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) {
Signature · 1 line
2.1.4
Arnaldo Carvalho de Melo
maintainerEm Wed, May 20, 2015 at 06:16:51PM +0200, Martin Liška escreveu:
Quoted 16 lines · Ingo Molnar
Quoted 6 lines · Arnaldo Carvalho de Melo
Quoted 2 lines
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
Kbuilddoes it:Quoted 8 lines
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,\
Quoted 8 lines
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
Quoted 30 lines · Arnaldo Carvalho de Melo
Em Wed, May 20, 2015 at 06:16:51PM +0200, Martin Liška escreveu:
Quoted 16 lines · Ingo Molnar
Quoted 6 lines · Arnaldo Carvalho de Melo
Quoted 2 lines
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
Kbuilddoes it:Quoted 8 lines
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,\Quoted 8 lines
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 includescripts/Kbuild.includeand then use it?
Quoted 5 lines
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 fromKbuildwould solve the issue at hand in a definitive way and in line with what we have been
pursuing: to make thetools/build system be based onKbuild.
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
Quoted 39 lines · Ingo Molnar
Quoted 30 lines · Arnaldo Carvalho de Melo
Em Wed, May 20, 2015 at 06:16:51PM +0200, Martin Liška escreveu:
Quoted 16 lines · Ingo Molnar
Quoted 6 lines · Arnaldo Carvalho de Melo
Quoted 2 lines
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
Kbuilddoes it:Quoted 8 lines
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,\Quoted 8 lines
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 includescripts/Kbuild.includeand then use it?Quoted 5 lines
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 fromKbuildwould solve the issue at hand in a definitive way and in line with what we have been
pursuing: to make thetools/build system be based onKbuild.
Quoted 9 lines
So I'd suggest copying any necessary functions instead of outright including all of
Kbuildin 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
Quoted 39 lines · Ingo Molnar
Quoted 30 lines · Arnaldo Carvalho de Melo
Em Wed, May 20, 2015 at 06:16:51PM +0200, Martin Liška escreveu:
Quoted 16 lines · Ingo Molnar
Quoted 6 lines · Arnaldo Carvalho de Melo
Quoted 2 lines
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
Kbuilddoes it:Quoted 8 lines
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,\Quoted 8 lines
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 includescripts/Kbuild.includeand then use it?Quoted 5 lines
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 fromKbuildwould solve the issue at hand in a definitive way and in line with what we have been
pursuing: to make thetools/build system be based onKbuild.
Quoted 9 lines
So I'd suggest copying any necessary functions instead of outright including all of
Kbuildin 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
Currently, GCC optimizes -O6 same as -O3 level, thus change the value to -O6.
Right optimize debugging experience is given by passing -Og to compiler. Assign default value for pointers that are identified by compiler as non-initialized.
--- 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(-)
tools/perf/arch/common.c +1 -1
@@ -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" );
tools/perf/config/Makefile +3 -1
@@ -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
tools/perf/config/utilities.mak +19 -0
@@ -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))
tools/perf/util/symbol.c +1 -1
@@ -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 ;
tools/perf/util/trace-event-parse.c +1 -1
@@ -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) {
Signature · 1 line
2.1.4
Ingo Molnar
Acked-byQuoted 2 lines · Martin Liška
Currently, GCC optimizes -O6 same as -O3 level, thus change the value to -O6.
s/to -O6
to -O3
Quoted 3 lines
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
Quoted 15 lines
ifeq ($(DEBUG),0)
(quoted diff) +11
-1- 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; \
Review
Quoted 10 lines
(quoted diff) +10
-0+ 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!
Thanks,
Ingo
Martin Liška
Acked-by Ingo MolnarQuoted 47 lines · Ingo Molnar
Quoted 2 lines · Martin Liška
Currently, GCC optimizes -O6 same as -O3 level, thus change the value to -O6.
s/to -O6
to -O3Quoted 3 lines
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 compilerQuoted 15 lines
ifeq ($(DEBUG),0)(quoted diff) +11
-1- 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; \Review
Quoted 10 lines
(quoted diff) +10
-0+ 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!
Quoted 6 lines
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
Currently, GCC optimizes -O6 same as -O3 level, thus change the value to -O3.
Right debugging experience is given by passing -Og to compiler. Assign default value for pointers that are identified by the compiler as non-initialized.
--- 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(-)
tools/perf/arch/common.c +1 -1
@@ -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" );
tools/perf/config/Makefile +3 -1
@@ -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
tools/perf/config/utilities.mak +19 -0
@@ -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))
tools/perf/util/symbol.c +1 -1
@@ -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 ;
tools/perf/util/trace-event-parse.c +1 -1
@@ -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) {
Signature · 1 line
2.1.4
Quoted 8 lines · Martin Liška
Quoted 6 lines
Quoted 3 lines
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
Martin Liška
Acked-by Ingo MolnarQuoted 17 lines · Ingo Molnar
Quoted 8 lines · Martin Liška
Quoted 6 lines
Quoted 3 lines
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 ...
Quoted 8 lines
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
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.
--- 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(-)
tools/perf/arch/common.c +1 -1
@@ -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" );
tools/perf/config/Makefile +3 -1
@@ -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
tools/perf/config/utilities.mak +19 -0
@@ -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))
tools/perf/util/symbol.c +1 -1
@@ -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 ;
tools/perf/util/trace-event-parse.c +1 -1
@@ -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) {
Signature · 1 line
2.1.4
Arnaldo Carvalho de Melo
maintainerEm Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
Quoted 20 lines · Ingo Molnar
Quoted 19 lines · Martin Liška
Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
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:
Quoted 8 lines
Quoted 6 lines
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
Quoted 105 lines
Attached version should reflect that.
Thanks,
MartinCurrently, 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(-)tools/perf/arch/common.c +1
-1@@ -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" );tools/perf/config/Makefile +3
-1@@ -129,7 +129,9 @@ ifndef DEBUG endif ifeq ($(DEBUG),0) - CFLAGS += -O6 + CFLAGS += -O3 +else + CFLAGS += $(call cc-option,-Og,-O0) endif ifdef PARSER_DEBUGtools/perf/config/utilities.mak +19
-0@@ -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))tools/perf/util/symbol.c +1
-1@@ -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 ;tools/perf/util/trace-event-parse.c +1
-1@@ -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) {Signature · 1 line
2.1.4
Quoted 23 lines · Arnaldo Carvalho de Melo
Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
Quoted 22 lines · Ingo Molnar
Quoted 21 lines · Martin Liška
Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
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 ...
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Thanks,
Ingo
Sorry Ingo for that, I overlooked that correction :)
Are you ok with this Ingo?
Yeah, certainly!
Quoted 4 lines
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
Quoted 42 lines · Ingo Molnar
Quoted 23 lines · Arnaldo Carvalho de Melo
Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
Quoted 22 lines · Ingo Molnar
Quoted 21 lines · Martin Liška
Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
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 ...
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Thanks,
Ingo
Sorry Ingo for that, I overlooked that correction :)
Are you ok with this Ingo?
Yeah, certainly!
Quoted 4 lines
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.
Quoted 6 lines
Yeah.
Thanks,
Ingo
Hello.
As Arnaldo pointed, I split the patch to following 2 smaller patches.
Thanks,
Martin
Assign default value for pointers that are identified by the compiler as non-initialized.
--- 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(-)
tools/perf/arch/common.c +1 -1
@@ -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" );
tools/perf/util/symbol.c +1 -1
@@ -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 ;
tools/perf/util/trace-event-parse.c +1 -1
@@ -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) {
Signature · 1 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.
--- tools/perf/config/Makefile | 4 +++- tools/perf/config/utilities.mak | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-)
tools/perf/config/Makefile +3 -1
@@ -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
tools/perf/config/utilities.mak +19 -0
@@ -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))
Signature · 1 line
2.1.4
Arnaldo Carvalho de Melo
maintainerEm Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
Quoted 45 lines · Ingo Molnar
Quoted 44 lines · Arnaldo Carvalho de Melo
Quoted 24 lines
Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
Quoted 22 lines · Ingo Molnar
Quoted 21 lines · Martin Liška
Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
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 ...
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Thanks,
Ingo
Sorry Ingo for that, I overlooked that correction :)
Are you ok with this Ingo?
Yeah, certainly!
Quoted 4 lines
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.
Quoted 8 lines
Quoted 4 lines
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
Quoted 113 lines
Thanks,
MartinAssign 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(-)tools/perf/arch/common.c +1
-1@@ -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" );tools/perf/util/symbol.c +1
-1@@ -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 ;tools/perf/util/trace-event-parse.c +1
-1@@ -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) {Signature · 13 lines
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(-)tools/perf/config/Makefile +3
-1@@ -129,7 +129,9 @@ ifndef DEBUG endif ifeq ($(DEBUG),0) - CFLAGS += -O6 + CFLAGS += -O3 +else + CFLAGS += $(call cc-option,-Og,-O0) endif ifdef PARSER_DEBUGtools/perf/config/utilities.mak +19
-0@@ -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))Signature · 1 line
2.1.4
Arnaldo Carvalho de Melo
maintainerEm Tue, May 26, 2015 at 11:36:26AM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 5 lines
Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
Quoted 1 line
As Arnaldo pointed, I split the patch to following 2 smaller patches.
Thanks, but how did you generated the two attached patches?
Quoted 8 lines
[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
Quoted 18 lines · Arnaldo Carvalho de Melo
Em Tue, May 26, 2015 at 11:36:26AM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 5 lines
Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
Quoted 1 line
As Arnaldo pointed, I split the patch to following 2 smaller patches.
Thanks, but how did you generated the two attached patches?
Quoted 8 lines
[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:
Quoted 6 lines
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
Quoted 3 lines
- Arnaldo
Quoted 60 lines · Arnaldo Carvalho de Melo
Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
Quoted 45 lines · Ingo Molnar
Quoted 44 lines · Arnaldo Carvalho de Melo
Quoted 24 lines
Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
Quoted 22 lines · Ingo Molnar
Quoted 21 lines · Martin Liška
Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
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 ...
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Thanks,
Ingo
Sorry Ingo for that, I overlooked that correction :)
Are you ok with this Ingo?
Yeah, certainly!
Quoted 4 lines
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.
Quoted 8 lines
Quoted 4 lines
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.
Quoted 7 lines
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
Quoted 117 lines
Quoted 116 lines
Thanks,
MartinAssign 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(-)tools/perf/arch/common.c +1
-1@@ -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" );tools/perf/util/symbol.c +1
-1@@ -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 ;tools/perf/util/trace-event-parse.c +1
-2@@ -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(-)tools/perf/config/Makefile +3
-1@@ -129,7 +129,9 @@ ifndef DEBUG endif ifeq ($(DEBUG),0) - CFLAGS += -O6 + CFLAGS += -O3 +else + CFLAGS += $(call cc-option,-Og,-O0) endif ifdef PARSER_DEBUGtools/perf/config/utilities.mak +19
-1@@ -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
Arnaldo Carvalho de Melo
maintainerEm Tue, May 26, 2015 at 05:48:27PM +0200, Martin Liška escreveu:
Quoted 15 lines · Arnaldo Carvalho de Melo
Quoted 13 lines
Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
Quoted 3 lines
Quoted 2 lines · Arnaldo Carvalho de Melo
Quoted 1 line
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^^..
Quoted 8 lines
Quoted 1 line
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.
Quoted 1 line
Thanks for re-application,
You're welcome, keep on contributing!
- Arnaldo
tip-bot for Martin Liška
merge bot[tip:perf/core] perf tools: Assign default value for some pointers
In reply to Martin Liškaperf tools: Assign default value for some pointers
Commit-ID: 5bcaaca3e4d15ce39008a0b9c431c0ac4be784bd Gitweb: http://git.kernel.org/tip/5bcaaca3e4d15ce39008a0b9c431c0ac4be784bd Author: Martin Liška <[email protected]> AuthorDate: Tue, 26 May 2015 11:41:37 -0300 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Wed, 27 May 2015 12:21:45 -0300
perf tools: Assign default value for some pointers
Assign default value for pointers that are identified by the compiler as non-initialized.
--- 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(-)
tools/perf/arch/common.c +1 -1
@@ -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" );
tools/perf/util/symbol.c +1 -1
@@ -398,7 +398,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 ;
tools/perf/util/trace-event-parse.c +1 -1
@@ -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) {
tip-bot for Martin Liska
merge bot[tip:perf/core] perf tools: Improve setting of gcc debug option
In reply to Martin Liškaperf tools: Improve setting of gcc debug option
Commit-ID: e8b7ea4356fdd3c4de5478f3418eb84f8dce2b61 Gitweb: http://git.kernel.org/tip/e8b7ea4356fdd3c4de5478f3418eb84f8dce2b61 Author: Martin Liska <[email protected]> AuthorDate: Tue, 26 May 2015 12:23:24 -0300 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Wed, 27 May 2015 12:21:45 -0300
perf tools: Improve setting of gcc debug option
Correct debugging experience is given by passing -Og to compiler.
Do it in a way that supports older compilers
--- tools/perf/config/Makefile | 2 ++ tools/perf/config/utilities.mak | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+)
tools/perf/config/Makefile +2 -0
@@ -130,6 +130,8 @@ endif ifeq ($(DEBUG),0) CFLAGS += -O6 +else + CFLAGS += $(call cc-option,-Og,-O0) endif ifdef PARSER_DEBUG
tools/perf/config/utilities.mak +19 -0
@@ -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))