| Message ID | 200908081310.27671.kdudka@redhat.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
On Sat, Aug 8, 2009 at 4:10 AM, Kamil Dudka<kdudka@redhat.com> wrote: > On Friday 07 of August 2009 22:27:08 Kamil Dudka wrote: >> attached is another fix for SPARSE headers improving their sanity. I am >> also attaching a simple test for the dependency tracking of headers. It's >> not enough generic and portable and therefore not really useful. Maybe >> someone skilled in writing makefiles might want to include something like >> that to the SPARSE Makefile as part of the 'check' target. What is the significant of making every header file self compilable? Unlike the kernel header files exported to user space, which usually have self contained meaning. Most of these header file have tight interaction with each other. I don't think it make sense for other sparse application to just use one of the header file. Enforcing each header file to be self compilable will result in a lot of unnecessary include. Gcc needs to include "symbol.h" many times just to skip over it. Take a look at pre-process.c. It is not exactly trivial. It needs to scan the token to find out the end of the if scope. In this case, it might be better just let other header file depend on "symbol.h". I want to heard what other people think about it too. I am a little bit reluctant on this one. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 11 of August 2009 11:40:45 Christopher Li wrote: > What is the significant of making every header file self compilable? > Unlike the kernel header files exported to user space, which usually > have self contained meaning. Most of these header file have tight > interaction with each other. I don't think it make sense for other sparse > application to just use one of the header file. > > Enforcing each header file to be self compilable will result in a lot > of unnecessary include. Gcc needs to include "symbol.h" many times > just to skip over it. Take a look at pre-process.c. It is not exactly > trivial. It needs to scan the token to find out the end of the if > scope. In this case, it might be better just let other header > file depend on "symbol.h". Then an ordinary SPARSE user needs to try various permutation of the includes to pass his hello-world through compiler. It could be really annoying. I'd like to have all includes in alphabetical order because it's then easier to maintenance. If you really want to have external dependencies of SPARSE headers (no matter what reason for), the dependencies should be exactly documented somewhere - maybe in the headers themselves? What about replacing my '#include "allocate.h"' with comment '// ATTENTION: you have to include allocate.h before storage.h'? > I want to heard what other people think about it too. I am a little bit > reluctant on this one. No problem with me as it has easy workaround. Kamil -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 11 of August 2009 11:55:19 Kamil Dudka wrote: > On Tuesday 11 of August 2009 11:40:45 Christopher Li wrote: > > What is the significant of making every header file self compilable? > > Unlike the kernel header files exported to user space, which usually > > have self contained meaning. Most of these header file have tight > > interaction with each other. I don't think it make sense for other sparse > > application to just use one of the header file. > > > > Enforcing each header file to be self compilable will result in a lot > > of unnecessary include. Gcc needs to include "symbol.h" many times > > just to skip over it. Take a look at pre-process.c. It is not exactly > > trivial. It needs to scan the token to find out the end of the if > > scope. In this case, it might be better just let other header > > file depend on "symbol.h". I agree processing of multiple-includes could be painful for the SPARSE preprocessor. But you don't solve it by avoiding multiple-includes in the SPARSE code itself. If you want to pass the SPARSE code through SPARSE, it's small enough to not bother us with performance. But you have absolutely no control of the foreign code processed by SPARSE and there the performance impact could be significant. I think this is the way to go: http://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html Kamil -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 11, 2009 at 11:40, Christopher Li<sparse@chrisli.org> wrote: > On Sat, Aug 8, 2009 at 4:10 AM, Kamil Dudka<kdudka@redhat.com> wrote: >> On Friday 07 of August 2009 22:27:08 Kamil Dudka wrote: >>> attached is another fix for SPARSE headers improving their sanity. I am >>> also attaching a simple test for the dependency tracking of headers. It's >>> not enough generic and portable and therefore not really useful. Maybe >>> someone skilled in writing makefiles might want to include something like >>> that to the SPARSE Makefile as part of the 'check' target. > > What is the significant of making every header file self compilable? > Unlike the kernel header files exported to user space, which usually > have self contained meaning. Most of these header file have tight interaction > with each other. I don't think it make sense for other sparse application > to just use one of the header file. > > Enforcing each header file to be self compilable will result in a lot > of unnecessary include. Gcc needs to include "symbol.h" many times > just to skip over it. Take a look at pre-process.c. It is not exactly > trivial. It needs to scan the token to find out the end of the if > scope. In this case, it might be better just let other header > file depend on "symbol.h". > > I want to heard what other people think about it too. I am a little bit > reluctant on this one. What about just using some forward decls instead of including the header files? e.g. for compile.h it is enough to have a "struct symbol;" there instead of the "#include <symbol.h>" to make it self-compilable. Cheers, -Hannes -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 11, 2009 at 3:40 AM, Kamil Dudka<kdudka@redhat.com> wrote: > I agree processing of multiple-includes could be painful for the SPARSE > preprocessor. But you don't solve it by avoiding multiple-includes in the > SPARSE code itself. If you want to pass the SPARSE code through SPARSE, it's > small enough to not bother us with performance. I am worry about adding too much duplicated include slow down the gcc compile of sparse itself. I know I am paranoid :-) > > But you have absolutely no control of the foreign code processed by SPARSE > and there the performance impact could be significant. I think this is the > way to go: > > http://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html BTW, sparse does similar things already. That is what the stream->protect was for. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 11 of August 2009 23:08:24 Christopher Li wrote: > On Tue, Aug 11, 2009 at 3:40 AM, Kamil Dudka<kdudka@redhat.com> wrote: > > I agree processing of multiple-includes could be painful for the SPARSE > > preprocessor. But you don't solve it by avoiding multiple-includes in the > > SPARSE code itself. If you want to pass the SPARSE code through SPARSE, > > it's small enough to not bother us with performance. > > I am worry about adding too much duplicated include slow down the > gcc compile of sparse itself. I know I am paranoid :-) Tested just now (ccache turned off)... With the original (not amended) patch: $ make clean && time make -j9 real 0m0.966s user 0m4.654s sys 0m1.471s With the amended patch: $ make clean && time make -j9 real 0m0.989s user 0m4.580s sys 0m1.504s Without any of my patches applied: $ make clean && time make -j9 real 0m0.980s user 0m4.596s sys 0m1.495s Note that the results fluctuated approx 10% during runs. My conclusion: absolutely no impact on gcc compile time. Kamil -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From 619088669e2333a8c5921d99befa8d72b322004d Mon Sep 17 00:00:00 2001 From: Kamil Dudka <kdudka@redhat.com> Date: Sat, 8 Aug 2009 12:58:41 +0200 Subject: [PATCH] make sparse headers self-compilable... ... and thus possible to include them in arbitrary order and without any external dependencies. Signed-off-by: Kamil Dudka <kdudka@redhat.com> --- compile.h | 2 ++ flow.h | 2 ++ ptrlist.h | 2 ++ scope.h | 2 ++ storage.h | 3 +++ 5 files changed, 11 insertions(+), 0 deletions(-) diff --git a/compile.h b/compile.h index 0db8fdb..6c5260d 100644 --- a/compile.h +++ b/compile.h @@ -1,6 +1,8 @@ #ifndef COMPILE_H #define COMPILE_H +#include "symbol.h" + extern void emit_one_symbol(struct symbol *); extern void emit_unit_begin(const char *); extern void emit_unit_end(void); diff --git a/flow.h b/flow.h index 9f2e165..241f081 100644 --- a/flow.h +++ b/flow.h @@ -1,6 +1,8 @@ #ifndef FLOW_H #define FLOW_H +#include "symbol.h" + extern unsigned long bb_generation; #define REPEAT_CSE 1 diff --git a/ptrlist.h b/ptrlist.h index dae0906..85b23fc 100644 --- a/ptrlist.h +++ b/ptrlist.h @@ -7,6 +7,8 @@ * (C) Copyright Linus Torvalds 2003-2005 */ +#include <stdlib.h> + #define container(ptr, type, member) \ (type *)((void *)(ptr) - offsetof(type, member)) diff --git a/scope.h b/scope.h index 5f1f232..aff7b6b 100644 --- a/scope.h +++ b/scope.h @@ -9,6 +9,8 @@ * Licensed under the Open Software License version 1.1 */ +#include "symbol.h" + struct scope { struct token *token; /* Scope start information */ struct symbol_list *symbols; /* List of symbols in this scope */ diff --git a/storage.h b/storage.h index 610cbfd..e049e23 100644 --- a/storage.h +++ b/storage.h @@ -1,6 +1,9 @@ #ifndef STORAGE_H #define STORAGE_H +#include "allocate.h" +#include "lib.h" + /* * The "storage" that underlies an incoming/outgoing pseudo. It's * basically the backing store for a pseudo, and may be a real hardware -- 1.6.4