We do _not_ need "sparse" in sparse arguments ;-)
What we do need is __BIG_ENDIAN__; right now unconditional, when m32r starts
using CPU_LITTLE_ENDIAN, we'll need to adjust.
Signed-off-by: Al Viro <[email protected]>
----
diff -urN RC14-rc2-git5-rpc-iomem/arch/m32r/Makefile RC14-rc2-git5-sparse-m32r/arch/m32r/Makefile
--- RC14-rc2-git5-rpc-iomem/arch/m32r/Makefile 2005-06-17 15:48:29.000000000 -0400
+++ RC14-rc2-git5-sparse-m32r/arch/m32r/Makefile 2005-09-25 23:46:36.000000000 -0400
@@ -24,7 +24,7 @@
CFLAGS += $(cflags-y)
AFLAGS += $(aflags-y)
-CHECKFLAGS := $(CHECK) -D__m32r__
+CHECKFLAGS += -D__m32r__ -D__BIG_ENDIAN__=1
head-y := arch/m32r/kernel/head.o arch/m32r/kernel/init_task.o
Please don't specify __BIG_ENDIAN__. ;-)
We have supported both big- and little-endian for the m32r kernel.
I hope it will be kept unconditional.
Now, the endianness is to be determined by a (cross)compiler:
- For the big-endian, a compiler (m32r-linux-gcc or m32r-linux-gnu-gcc)
provides a predefined macro __BIG_ENDIAN__.
- For little-endian, a compiler (m32rle-linux-gcc or m32rle-linux-gnu-gcc)
provides a predefined macro __LITTLE_ENDIAN__.
Here is a modified patch.
Thank you.
From: Al Viro <[email protected]>
Date: Mon, 26 Sep 2005 06:18:04 +0100
> What we do need is __BIG_ENDIAN__; right now unconditional, when m32r starts
> using CPU_LITTLE_ENDIAN, we'll need to adjust.
Signed-off-by: Al Viro <[email protected]>
Signed-off-by: Hirokazu Takata <[email protected]>
----
arch/m32r/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.14-rc2/arch/m32r/Makefile
===================================================================
--- linux-2.6.14-rc2.orig/arch/m32r/Makefile 2005-08-29 08:41:01.000000000 +0900
+++ linux-2.6.14-rc2/arch/m32r/Makefile 2005-09-27 13:15:19.000000000 +0900
@@ -24,7 +24,7 @@ aflags-$(CONFIG_ISA_M32R) += -DNO_FPU -W
CFLAGS += $(cflags-y)
AFLAGS += $(aflags-y)
-CHECKFLAGS := $(CHECK) -D__m32r__
+CHECKFLAGS += -D__m32r__
head-y := arch/m32r/kernel/head.o arch/m32r/kernel/init_task.o
--
Hirokazu Takata <[email protected]>
Linux/M32R Project: http://www.linux-m32r.org/
Please don't specify __BIG_ENDIAN__. ;-)
We have supported both big- and little-endian for the m32r kernel.
I hope it will be kept unconditional.
Now, the endianness is to be determined by a (cross)compiler:
- For the big-endian, a compiler (m32r-linux-gcc or m32r-linux-gnu-gcc)
provides a predefined macro __BIG_ENDIAN__.
- For little-endian, a compiler (m32rle-linux-gcc or m32rle-linux-gnu-gcc)
provides a predefined macro __LITTLE_ENDIAN__.
Here is a modified patch.
Thank you.
From: Al Viro <[email protected]>
Date: Mon, 26 Sep 2005 06:18:04 +0100
> What we do need is __BIG_ENDIAN__; right now unconditional, when m32r starts
> using CPU_LITTLE_ENDIAN, we'll need to adjust.
Signed-off-by: Al Viro <[email protected]>
Signed-off-by: Hirokazu Takata <[email protected]>
----
arch/m32r/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.14-rc2/arch/m32r/Makefile
===================================================================
--- linux-2.6.14-rc2.orig/arch/m32r/Makefile 2005-08-29 08:41:01.000000000 +0900
+++ linux-2.6.14-rc2/arch/m32r/Makefile 2005-09-27 13:15:19.000000000 +0900
@@ -24,7 +24,7 @@ aflags-$(CONFIG_ISA_M32R) += -DNO_FPU -W
CFLAGS += $(cflags-y)
AFLAGS += $(aflags-y)
-CHECKFLAGS := $(CHECK) -D__m32r__
+CHECKFLAGS += -D__m32r__
head-y := arch/m32r/kernel/head.o arch/m32r/kernel/init_task.o
--
Hirokazu Takata <[email protected]>
Linux/M32R Project: http://www.linux-m32r.org/
On Tue, Sep 27, 2005 at 03:13:01PM +0900, Hirokazu Takata wrote:
> Please don't specify __BIG_ENDIAN__. ;-)
> We have supported both big- and little-endian for the m32r kernel.
> I hope it will be kept unconditional.
>
> Now, the endianness is to be determined by a (cross)compiler:
> - For the big-endian, a compiler (m32r-linux-gcc or m32r-linux-gnu-gcc)
> provides a predefined macro __BIG_ENDIAN__.
> - For little-endian, a compiler (m32rle-linux-gcc or m32rle-linux-gnu-gcc)
> provides a predefined macro __LITTLE_ENDIAN__.
>
> Here is a modified patch.
>
> Thank you.
You do realize that this way sparse will get neither? It does not
pick predefined symbols from gcc; thus the -D<your_arch>, etc.
One solution would be to really run cross-gcc on something like
#define HAVE(x) -D##x=x
#ifdef __LITTLE_ENDIAN__
HAVE(__LITTLE_ENDIAN__)
#endif
#ifdef __BIG_ENDIAN__
HAVE(__BIG_ENDIAN__)
#endif
with grep -- -D on the output and have it added to CHECKFLAGS. The only
problem being, for a generic solution we want to make sure that this
sucker is run _after_ we have decided on gcc arguments.
Note that it's potentially more than just m32r - e.g. s390 wants
__s390x__ added only for case of 64bit target. Similar for powerpc
if these get merged, etc.
Sam, any help in that area? Ideally we want to have something equivalent
to
PREDEFINED_WE_MIGHT_WANT = __m32r__ __LITTLE_ENDIAN__ __BIG_ENDIAN__
and CHECKFLAGS done from that - basically, the subset of cross-gcc
predefined symbols reproduced for sparse. Ideally with -m64 added
if we have sizeof(long) == 8 on target, to take care of all that
crap in one go.
Suggestions?
On Tue, 27 Sep 2005, Hirokazu Takata wrote:
>
> Now, the endianness is to be determined by a (cross)compiler:
> - For the big-endian, a compiler (m32r-linux-gcc or m32r-linux-gnu-gcc)
> provides a predefined macro __BIG_ENDIAN__.
> - For little-endian, a compiler (m32rle-linux-gcc or m32rle-linux-gnu-gcc)
> provides a predefined macro __LITTLE_ENDIAN__.
Hmm.. You need to tell sparse _some_ way which one you use, since sparse
won't do it.
Picking one at random is fine, of course. It doesn't even have to match
the one you'll compile with, although that means that sparse will
obviously be testing a different configuration than the one you'd actually
compile.
So I think having -D__BIG_ENDIAN__ in the sparse flags is better than not
having anything at all (since otherwise it won't be able to check
anything). And having something that matches the compiler would be better
still.
Linus
On Sep 27, 2005, at 03:10:25, Al Viro wrote:
> You do realize that this way sparse will get neither? It does not
> pick predefined symbols from gcc; thus the -D<your_arch>, etc.
How about sticking this in some global Makefile somewhere? This will
give sparse the same list of defines that GCC uses:
CHECKFLAGS += $(shell echo | gcc -E - -dM | sed -re 's/^#define +([^ ]
+) +(.*)$/-D\1=\2/g')
Or you could do this:
include/linux/checkerdefines.h:
echo | gcc -E - -dM >$@
And in linux/stddef.h or linux/compiler.h or something do:
#if defined(__CHECKER__) and not defined(_LINUX_CHECKERDEFINES_H)
# define _LINUX_CHECKERDEFINES_H 1
# include <linux/checkerdefines.h>
#endif
Cheers,
Kyle Moffett
--
Q: Why do programmers confuse Halloween and Christmas?
A: Because OCT 31 == DEC 25.
On Tue, Sep 27, 2005 at 11:13:44AM -0400, Kyle Moffett wrote:
> On Sep 27, 2005, at 03:10:25, Al Viro wrote:
> >You do realize that this way sparse will get neither? It does not
> >pick predefined symbols from gcc; thus the -D<your_arch>, etc.
>
> How about sticking this in some global Makefile somewhere? This will
> give sparse the same list of defines that GCC uses:
>
> CHECKFLAGS += $(shell echo | gcc -E - -dM | sed -re 's/^#define +([^ ]
> +) +(.*)$/-D\1=\2/g')
>
> Or you could do this:
>
> include/linux/checkerdefines.h:
> echo | gcc -E - -dM >$@
*Hell*, no.
1) any use of cross-gcc is immediately b0rken
2) anything like building i386 on amd64 (native toolchain, -m32
in arguments) is b0rken
3) way, way, *WAY* too much spew. gcc pre-defines shitloads of
stuff and some of that stuff very definitely should not be there for sparse.
IOW, it's not that simple. You need final CC, CFLAGS and CPPFLAGS for
that. Moreover, your variant won't do at all - change .config and you
might need to rebuild that file on biarch platforms (parisc and s390 at
the very least; ppc/ppc64 are also moving in that direction).
On Sep 27, 2005, at 12:34:55, Al Viro wrote:
> *Hell*, no.
> 1) any use of cross-gcc is immediately b0rken
> 2) anything like building i386 on amd64 (native toolchain, -m32
> in arguments) is b0rken
So s/gcc/$(SOME_MAKEFILE_VARIABLE)/g I was simplifying for the sake
of suggestion/example; adding in support for all of those is
nontrivial and best left to those who thoroughly understand the
kernel Makefile system (IE: Not me :-D).
> 3) way, way, *WAY* too much spew. gcc pre-defines shitloads of
> stuff and some of that stuff very definitely should not be there
> for sparse.
Why not? Some of that stuff may get used in kernel headers, which
sparse should definitely have defined. Besides, sparse is designed
to check C source code, which will be compiled with said GCC using
those preprocessing defines. Why should it use a different set of
defines?
> IOW, it's not that simple. You need final CC, CFLAGS and CPPFLAGS
> for that.
See responses to (1) and (2)
> Moreover, your variant won't do at all - change .config and you
> might need to rebuild that file on biarch platforms (parisc and
> s390 at the very least; ppc/ppc64 are also moving in that direction).
We do far worse messes in per-arch Makefiles, and this seems
reasonably easy to put in an arch-generic makefile, unless there is
some significant complication you didn't mention.
Cheers,
Kyle Moffett
--
I have yet to see any problem, however complicated, which, when you
looked at it in the right way, did not become still more complicated.
-- Poul Anderson
On Tue, Sep 27, 2005 at 01:31:25PM -0400, Kyle Moffett wrote:
> > 3) way, way, *WAY* too much spew. gcc pre-defines shitloads of
> >stuff and some of that stuff very definitely should not be there
> >for sparse.
>
> Why not? Some of that stuff may get used in kernel headers, which
> sparse should definitely have defined. Besides, sparse is designed
> to check C source code, which will be compiled with said GCC using
> those preprocessing defines. Why should it use a different set of
> defines?
First of all, some of that stuff should not be used in kernel headers
and getting a warning about such uses is a Good Thing(tm). What's
more, some are actively *wrong* for kernel - __STDC_HOSTED__, for one,
is simply a lie. And no, sparse (or any other C compiler) is not
required to have the same pile as gcc does.
There's another reason why limited subset is good - it documents the
subset we are using. And having to reach gcc source just to figure
out which architectures might have given symbol... No, thanks.
On Sep 27, 2005, at 13:55:26, Al Viro wrote:
> On Tue, Sep 27, 2005 at 01:31:25PM -0400, Kyle Moffett wrote:
>> Why not? Some of that stuff may get used in kernel headers, which
>> sparse should definitely have defined. Besides, sparse is
>> designed to check C source code, which will be compiled with said
>> GCC using those preprocessing defines. Why should it use a
>> different set of defines?
>
> First of all, some of that stuff should not be used in kernel
> headers and getting a warning about such uses is a Good Thing(tm).
> What's more, some are actively *wrong* for kernel -
> __STDC_HOSTED__, for one, is simply a lie.
So maybe put a "grep" in the middle to select the ones you want or
"grep -v" to remove the ones you don't? I don't see why this is such
a big deal. There are obviously missing defines that sparse needs to
correctly operate with kernel headers, and I'm not sure why we should
specify them in several places in the Makefiles, including one set
for each arch.?
> And no, sparse (or any other C compiler) is not required to have
> the same pile as gcc does.
But when we're using sparse to check kernel sources, it should have a
similar set to what the regular compiler (IE: gcc) has when building
kernel sources.
Cheers,
Kyle Moffett
--
Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are, by
definition, not smart enough to debug it.
-- Brian Kernighan
> Sam, any help in that area? Ideally we want to have something equivalent
> to
> PREDEFINED_WE_MIGHT_WANT = __m32r__ __LITTLE_ENDIAN__ __BIG_ENDIAN__
> and CHECKFLAGS done from that - basically, the subset of cross-gcc
> predefined symbols reproduced for sparse. Ideally with -m64 added
> if we have sizeof(long) == 8 on target, to take care of all that
> crap in one go.
>
> Suggestions?
The most simple solution would be to provide a small script that
create the defines as we need and run it for each invocation of sparse.
The script should use same trick as scripts/gcc-version.sh does.
So we could have:
#!/bin/sh
compiler="$*"
BIG=$(echo __BIG_ENDIAN__ | $compiler -E -xc - | tail -n 1)
LITTLE=$(echo __LITTLE_ENDIAN__ | $compiler -E -xc - | tail -n 1)
Then BIG would be set to "1" if this is big endian, and "__BIG_ENDIAN__"
if little endian.
A little bit of shell script and we have the defines we want for m32r.
Then we could add calling this script as part of sparse invocation.
The better solution would be to find the relevant flags before we
start building the kernel. This is not so easy if we want access to
final CFLAGS. But for the architecture the important ones are
defined in arch/Makefile so placing this late in the file and
use $(CC) $(CFLAGS) should be OK in almost all cases.
Too late for me to cook up a patch right now - but simple.
Oh, and I agree. We do NOT want all the gcc defined flags.
We should restrict to a subset in the kernel - so it's good
if sparse warns/errors out on usage of the non-trivial defines.
Sam
On Tue, Sep 27, 2005 at 02:37:09PM -0400, Kyle Moffett wrote:
> >And no, sparse (or any other C compiler) is not required to have
> >the same pile as gcc does.
>
> But when we're using sparse to check kernel sources, it should have a
> similar set to what the regular compiler (IE: gcc) has when building
> kernel sources.
Have you ever looked at that set of defines? Note that even with gcc
most of them are supposed to be only used in gcc headers. Most of
those are never used in the kernel code, and for a good reason. Only
two are actually used - stdarg.h and (for raid6 with CONFIG_ALTIVEC)
altivec.h. Guess what? The former doesn't use any of these defines
and in case of the latter the only define used would be a lie - sparse
does *not* handle -maltivec.
It gets even better when you get to defines that describe compiler capacities.
sparse, for example, does *not* support wchar_t et.al.; gcc does. Should
we lie about that support? Should sparse binary that kinda-sorta imitates
e.g. builtins of gcc 3.3 (and declares __GNUC... accordingly) pick bogus
ones from gcc4 you are using for build?
There are very few defines we really want out of that pile. "Take everything"
is nowhere near sanity, especially if you consider the differences between
gcc versions (and gcc builds, while we are at it).
On Tue, Sep 27, 2005 at 08:00:03AM -0700, Linus Torvalds wrote:
>
>
> On Tue, 27 Sep 2005, Hirokazu Takata wrote:
> >
> > Now, the endianness is to be determined by a (cross)compiler:
> > - For the big-endian, a compiler (m32r-linux-gcc or m32r-linux-gnu-gcc)
> > provides a predefined macro __BIG_ENDIAN__.
> > - For little-endian, a compiler (m32rle-linux-gcc or m32rle-linux-gnu-gcc)
> > provides a predefined macro __LITTLE_ENDIAN__.
>
> Hmm.. You need to tell sparse _some_ way which one you use, since sparse
> won't do it.
>
> Picking one at random is fine, of course. It doesn't even have to match
> the one you'll compile with, although that means that sparse will
> obviously be testing a different configuration than the one you'd actually
> compile.
>
> So I think having -D__BIG_ENDIAN__ in the sparse flags is better than not
> having anything at all (since otherwise it won't be able to check
> anything). And having something that matches the compiler would be better
> still.
Really interesting question is why do we need two toolchains at all.
Note that little-endian m32r gcc at least appears to understand
-mbe/-mbig-endian and binutils handles both endianness just fine.
Does that really work and is there any reason why big-endian one
could not handle -mle the same way with minimal changes? IOW, do
they have to differ in anything except the default target endianness?
Note that dependencies on "host endianness == target endianness" are
practically guaranteed to cause bugs in cross-compiler, so any of
those would very likely to be a bug in need of fixing anyway...
On Tue, Sep 27, 2005 at 10:02:30PM +0200, Sam Ravnborg wrote:
>
> > Sam, any help in that area? Ideally we want to have something equivalent
> > to
> > PREDEFINED_WE_MIGHT_WANT = __m32r__ __LITTLE_ENDIAN__ __BIG_ENDIAN__
> > and CHECKFLAGS done from that - basically, the subset of cross-gcc
> > predefined symbols reproduced for sparse. Ideally with -m64 added
> > if we have sizeof(long) == 8 on target, to take care of all that
> > crap in one go.
> >
> > Suggestions?
> The most simple solution would be to provide a small script that
> create the defines as we need and run it for each invocation of sparse.
> The script should use same trick as scripts/gcc-version.sh does.
>
> So we could have:
> #!/bin/sh
> compiler="$*"
>
> BIG=$(echo __BIG_ENDIAN__ | $compiler -E -xc - | tail -n 1)
> LITTLE=$(echo __LITTLE_ENDIAN__ | $compiler -E -xc - | tail -n 1)
>
> Then BIG would be set to "1" if this is big endian, and "__BIG_ENDIAN__"
> if little endian.
> A little bit of shell script and we have the defines we want for m32r.
> Then we could add calling this script as part of sparse invocation.
Oww... Somehow that doesn't look like a good idea. Two spawned gcc
per sparse run? Not pretty...
> The better solution would be to find the relevant flags before we
> start building the kernel. This is not so easy if we want access to
> final CFLAGS. But for the architecture the important ones are
> defined in arch/Makefile so placing this late in the file and
> use $(CC) $(CFLAGS) should be OK in almost all cases.
Alternatively, we could generate something like
#define HAVE(x) -D##x=x
#ifdef <symbol>
HAVE(<symbol>)
#endif
...
into arch/$ARCH/kernel/predefines.S, have arch/$ARCH/kernel/predefines.s
generated by normal rules and have echo `grep -- -D arch/.../predefines.s`
do the rest...
From: Al Viro <[email protected]>
Date: Wed, 28 Sep 2005 01:18:40 +0100
> On Tue, Sep 27, 2005 at 08:00:03AM -0700, Linus Torvalds wrote:
> >
> > On Tue, 27 Sep 2005, Hirokazu Takata wrote:
> > >
> > > Now, the endianness is to be determined by a (cross)compiler:
> > > - For the big-endian, a compiler (m32r-linux-gcc or m32r-linux-gnu-gcc)
> > > provides a predefined macro __BIG_ENDIAN__.
> > > - For little-endian, a compiler (m32rle-linux-gcc or m32rle-linux-gnu-gcc)
> > > provides a predefined macro __LITTLE_ENDIAN__.
> >
> > Hmm.. You need to tell sparse _some_ way which one you use, since sparse
> > won't do it.
In the top Makefile, CHECKFLAGS is defined as follows:
CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ $(CF)
Can I use CF for user specific CHECKFLAGS ?
What do you think about we specify it manyally for a biendian target?
ex. m32r
sparse check for biendian target
$ make ARCH=m32r CF=-D__BIG_ENDIAN__=1 C=1 ... for big-endian
$ make ARCH=m32r CF=-D__LITTLE__=1 C=1 ... for little-endian
arch/m32r/Makefile:
CHECKFLAGS += -D__m32r__
> > Picking one at random is fine, of course. It doesn't even have to match
> > the one you'll compile with, although that means that sparse will
> > obviously be testing a different configuration than the one you'd actually
> > compile.
> >
> > So I think having -D__BIG_ENDIAN__ in the sparse flags is better than not
> > having anything at all (since otherwise it won't be able to check
> > anything). And having something that matches the compiler would be better
> > still.
I understand what you said.
I think having -D__BIG_ENDIAN__ is one of the realistic solutions,
because it seems that there is no good solution about this problem
as other people said in this ML-thread.
> Really interesting question is why do we need two toolchains at all.
> Note that little-endian m32r gcc at least appears to understand
> -mbe/-mbig-endian and binutils handles both endianness just fine.
IMHO, I think it is for convinience of building userland programs.
For example, in case of building programs (like Debian/GNU Linux
packages), it is useful that we can simply use a compiler without
not only modifying any source/Makefile but also specifing any
target-specific options of the compiler.
> Does that really work and is there any reason why big-endian one
> could not handle -mle the same way with minimal changes? IOW, do
> they have to differ in anything except the default target endianness?
I have no idea...
> Note that dependencies on "host endianness == target endianness" are
> practically guaranteed to cause bugs in cross-compiler, so any of
> those would very likely to be a bug in need of fixing anyway...
I think so, too.
I hope we can do check both of target endianness intentionally.
-- Takata
On Wed, Sep 28, 2005 at 07:04:14PM +0900, Hirokazu Takata wrote:
> In the top Makefile, CHECKFLAGS is defined as follows:
> CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ $(CF)
>
> Can I use CF for user specific CHECKFLAGS ?
> What do you think about we specify it manyally for a biendian target?
>
> ex. m32r
> sparse check for biendian target
> $ make ARCH=m32r CF=-D__BIG_ENDIAN__=1 C=1 ... for big-endian
> $ make ARCH=m32r CF=-D__LITTLE__=1 C=1 ... for little-endian
That's doable, but... I'd rather not make it mandatory - CF is for
"specific for this build" flags and when used for parallel cross-builds
such requirements make it rather nasty to deal with.
> I understand what you said.
> I think having -D__BIG_ENDIAN__ is one of the realistic solutions,
> because it seems that there is no good solution about this problem
> as other people said in this ML-thread.
Note that it's not just sparse. There is a legitimate need to make
endianness of target visible to Kconfig - not to mention anything
else, there are drivers that are really broken on big-endian targets.