2006-08-04 14:03:11

by Jes Sorensen

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

>>>>> "Jeff" == Jeff Garzik <[email protected]> writes:

Jeff> [email protected] wrote:
>> A first step to a generic boolean-type. The patch just introduce
>> the bool (in

Jeff> Since gcc supports boolean types and can optimize for such,
Jeff> introducing bool is IMO a good thing.

>> -Why would we want it? -There is already some how are depending on
>> a "boolean"-type (like NTFS). Also, it will clearify functions who
>> returns a boolean from one returning a value, ex: bool it_is_ok();
>> char it_is_ok(); The first one is obvious what it is doing, the
>> secound might return some sort of status.

Jeff> A better reason is that there is intrinsic compiler support for
Jeff> booleans.

Well late to the dicussion, but I still want to point out that forcing
a boolean type of a different size upon existing kernel code is not
always a great idea and can have nasty side effects for struct
alignments. Not to mention that on some architectures, accessing a u1
is a lot slower than accessing an int. If a developer really wants to
use the smaller type he/she should do so explicitly being aware of the
impact.

The kernel is written in C, not C++ or Jave or some other broken
language and C doesn't have 'bool'. This patch falls under the
'typedefs considered evil' or typedef for the sake of typedef, if you
ask me.

Regards,
Jes


2006-08-04 14:24:05

by Alan

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Ar Gwe, 2006-08-04 am 10:03 -0400, ysgrifennodd Jes Sorensen:
> alignments. Not to mention that on some architectures, accessing a u1
> is a lot slower than accessing an int. If a developer really wants to
> use the smaller type he/she should do so explicitly being aware of the
> impact.

Which is just fine. Nobody at the moment is using the bool type because
we don't have one. Nor is a C bool necessarily u1.

> The kernel is written in C, not C++ or Jave or some other broken
> language and C doesn't have 'bool'.

Oh yes it does, as of C99 via stdbool.h. The only reason its not always
"bool" is compatibility considerations. Welcome to the 21st century.

Alan


2006-08-04 14:35:39

by Jes Sorensen

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Alan Cox wrote:
> Ar Gwe, 2006-08-04 am 10:03 -0400, ysgrifennodd Jes Sorensen:
>> alignments. Not to mention that on some architectures, accessing a u1
>> is a lot slower than accessing an int. If a developer really wants to
>> use the smaller type he/she should do so explicitly being aware of the
>> impact.
>
> Which is just fine. Nobody at the moment is using the bool type because
> we don't have one. Nor is a C bool necessarily u1.

The proposed patch makes it u1 - if we end up with arch specific
defines, as the patch is proposing, developers won't know for sure what
the size is and will get alignment wrong. That is not fine.

If we really have to introduce a bool type, at least it has to be the
same size on all 32 bit archs and the same size on all 64 bit archs.

But again, the end result is we end up with yet another typedef for the
sake of introducing a typedef.

>> The kernel is written in C, not C++ or Jave or some other broken
>> language and C doesn't have 'bool'.
>
> Oh yes it does, as of C99 via stdbool.h. The only reason its not always
> "bool" is compatibility considerations. Welcome to the 21st century.

*Shiver*, I guess we'll need a machine that is PC2007 or whatever
compliant to run Linux next.

Jes

2006-08-04 15:32:18

by Alan

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Ar Gwe, 2006-08-04 am 16:35 +0200, ysgrifennodd Jes Sorensen:
> The proposed patch makes it u1 - if we end up with arch specific
> defines, as the patch is proposing, developers won't know for sure what
> the size is and will get alignment wrong. That is not fine.

The _Bool type is up to gcc implementation details.

> If we really have to introduce a bool type, at least it has to be the
> same size on all 32 bit archs and the same size on all 64 bit archs.

You don't use bool for talking to hardware, you use it for the most
efficient compiler behaviour when working with true/false values.

Alan

2006-08-04 15:55:24

by Jes Sorensen

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Alan Cox wrote:
> Ar Gwe, 2006-08-04 am 16:35 +0200, ysgrifennodd Jes Sorensen:
>> The proposed patch makes it u1 - if we end up with arch specific
>> defines, as the patch is proposing, developers won't know for sure what
>> the size is and will get alignment wrong. That is not fine.
>
> The _Bool type is up to gcc implementation details.

Which is even worse :(

>> If we really have to introduce a bool type, at least it has to be the
>> same size on all 32 bit archs and the same size on all 64 bit archs.
>
> You don't use bool for talking to hardware, you use it for the most
> efficient compiler behaviour when working with true/false values.

Thats the problem, people will start putting them into structs, and
voila all alignment predictability has gone out the window.

Regards,
Jes

2006-08-04 16:00:50

by Andreas Schwab

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Jes Sorensen <[email protected]> writes:

> Alan Cox wrote:
>> Ar Gwe, 2006-08-04 am 16:35 +0200, ysgrifennodd Jes Sorensen:
>>> The proposed patch makes it u1 - if we end up with arch specific
>>> defines, as the patch is proposing, developers won't know for sure what
>>> the size is and will get alignment wrong. That is not fine.
>>
>> The _Bool type is up to gcc implementation details.
>
> Which is even worse :(

It's part of the ABI, just like any other C type.

>>> If we really have to introduce a bool type, at least it has to be the
>>> same size on all 32 bit archs and the same size on all 64 bit archs.
>>
>> You don't use bool for talking to hardware, you use it for the most
>> efficient compiler behaviour when working with true/false values.
>
> Thats the problem, people will start putting them into structs, and
> voila all alignment predictability has gone out the window.

Just like trying to predict the alignment of any other C type.

Andreas.

--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstra?e 5, 90409 N?rnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2006-08-04 16:07:10

by Jes Sorensen

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Andreas Schwab wrote:
> Jes Sorensen <[email protected]> writes:
>> Thats the problem, people will start putting them into structs, and
>> voila all alignment predictability has gone out the window.
>
> Just like trying to predict the alignment of any other C type.

Well in that case, could you list the size of it for all Linux archs
please? We know that today long is the only one that differs and that
m68k has horrible natural alignment rules for historical reasons, but
besides that it's pretty sane.

Just because something exists doesn't mean using it is a good thing.
Just like gcc supporting exceptions :)

Regards,
Jes

2006-08-04 16:12:27

by Alan

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Ar Gwe, 2006-08-04 am 17:58 +0200, ysgrifennodd Jes Sorensen:
> > You don't use bool for talking to hardware, you use it for the most
> > efficient compiler behaviour when working with true/false values.
>
> Thats the problem, people will start putting them into structs, and
> voila all alignment predictability has gone out the window.

Jes, try reading as well as writing. Given you even quoted "You don't
use bool for talking to hardware" maybe you should read it.

Structure alignment is generally a bad idea anyway because even array
and word alignment are pretty variable between processors.


2006-08-04 16:16:33

by Andreas Schwab

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Jes Sorensen <[email protected]> writes:

> We know that today long is the only one that differs and that
> m68k has horrible natural alignment rules for historical reasons, but
> besides that it's pretty sane.

Try determining the alignment of u64 on i386. You will be surprised.

#include <stdio.h>

typedef long long u64;
struct u64_s { u64 x; } x;

int main ()
{
printf ("%d\n", __alignof__ (u64));
printf ("%d\n", __alignof__ (x));
return 0;
}

Btw, the iptables compat code was broken due to this.

Andreas.

--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstra?e 5, 90409 N?rnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2006-08-04 16:20:30

by Jes Sorensen

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Alan Cox wrote:
> Ar Gwe, 2006-08-04 am 17:58 +0200, ysgrifennodd Jes Sorensen:
>>> You don't use bool for talking to hardware, you use it for the most
>>> efficient compiler behaviour when working with true/false values.
>> Thats the problem, people will start putting them into structs, and
>> voila all alignment predictability has gone out the window.
>
> Jes, try reading as well as writing. Given you even quoted "You don't
> use bool for talking to hardware" maybe you should read it.

Alan,

I did read, I never suggested putting it into structs describing
hardware registers.

> Structure alignment is generally a bad idea anyway because even array
> and word alignment are pretty variable between processors.

It's fairly common in drivers to layout things in effective ways in
structs relying on alignment. It can make a noticeable difference if
you get cacheline alignment wrong. For example in network drivers where
parts of the struct is used for receive and the other part is used for
transmit and the two parts can run in parallel on different CPUs.
Obviously one ends up using the aligned attribute, but the more one can
avoid adding those on in the struct the easier it is to maintain and
read.

Regards,
Jes

2006-08-04 16:26:42

by Jes Sorensen

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Andreas Schwab wrote:
> Jes Sorensen <[email protected]> writes:
>
>> We know that today long is the only one that differs and that
>> m68k has horrible natural alignment rules for historical reasons, but
>> besides that it's pretty sane.
>
> Try determining the alignment of u64 on i386. You will be surprised.

If thats the case, then thats really scary :-( I'd claim it's a bug and
I am willing to be that iptables isn't the only place that is affected
or will be in the future.

Would a fix along the lines of this work?

typedef long long u64 __attribute__ ((aligned (8));

Cheers,
Jes

2006-08-04 16:57:51

by Andreas Schwab

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Jes Sorensen <[email protected]> writes:

> Andreas Schwab wrote:
>> Jes Sorensen <[email protected]> writes:
>>
>>> We know that today long is the only one that differs and that
>>> m68k has horrible natural alignment rules for historical reasons, but
>>> besides that it's pretty sane.
>>
>> Try determining the alignment of u64 on i386. You will be surprised.
>
> If thats the case, then thats really scary :-(

That's how the ABI is defined.

Andreas.

--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstra?e 5, 90409 N?rnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2006-08-04 18:44:50

by Jes Sorensen

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Andreas Schwab wrote:
> Jes Sorensen <[email protected]> writes:
>
>> Andreas Schwab wrote:
>>> Jes Sorensen <[email protected]> writes:
>>>
>>>> We know that today long is the only one that differs and that
>>>> m68k has horrible natural alignment rules for historical reasons, but
>>>> besides that it's pretty sane.
>>> Try determining the alignment of u64 on i386. You will be surprised.
>> If thats the case, then thats really scary :-(
>
> That's how the ABI is defined.

That the ABI for long long or the ABI for uint64_t? Given that u64 is a
Linux thing, it ought to be ok to do the alignment the right way within
the kernel.

Cheers,
Jes

2006-08-04 18:52:09

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Jes Sorensen wrote:
>
>> That's how the ABI is defined.
>
> That the ABI for long long or the ABI for uint64_t? Given that u64 is a
> Linux thing, it ought to be ok to do the alignment the right way within
> the kernel.
>

And what will break if you make that switch?

-hpa

2006-08-04 18:55:35

by Jes Sorensen

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

H. Peter Anvin wrote:
> Jes Sorensen wrote:
>>
>>> That's how the ABI is defined.
>>
>> That the ABI for long long or the ABI for uint64_t? Given that u64 is a
>> Linux thing, it ought to be ok to do the alignment the right way within
>> the kernel.
>>
>
> And what will break if you make that switch?

If we are lucky, some binary only modules? :-)

But you're right, it may just have to be documented as one of those
nasty issues to watch out for.

Cheers,
Jes

2006-08-04 19:05:31

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Jes Sorensen wrote:
>>
>> And what will break if you make that switch?
>
> If we are lucky, some binary only modules? :-)
>
> But you're right, it may just have to be documented as one of those
> nasty issues to watch out for.
>

What is really poisonous is structures which get padded when all the
members are naturally aligned. Unfortunately gcc produces really crap
code with __attribute__((packed)) on some architectures, so just using
that isn't a good solution. On the other hand, non-AEABI ARM sometimes
needs it!

For the lack of a __attribute__((nopad)) that would throw a warning or
error on excessive padding, I fear that our best option is an __abi
annotation which would enforce certain rules using sparse, and
presumably provide __attribute__((packed)) on ARM:

- All padding must be explicit.
- All members must be naturally aligned.
- No unportable constructs, like non-int-sized bitfields.

-hpa

2006-08-06 09:30:47

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean


>typedef long long u64;

That's s64.

>int main ()

Not ANSI C conformant.

SCNR.


Jan Engelhardt
--

2006-08-06 09:37:23

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean


> We know that today long is the only one that differs

For "modern architectures" maybe, but older compilers (like Turbo C
compiler (1990)), int is a 16 bit quantity, and therefore does differ, from
today's implementations at least.


Jan Engelhardt
--

2006-08-06 09:48:17

by Andreas Schwab

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Jan Engelhardt <[email protected]> writes:

>>typedef long long u64;
>
> That's s64.

Right.

>>int main ()
>
> Not ANSI C conformant.

Wrong.

Andreas.

--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstra?e 5, 90409 N?rnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2006-08-06 15:29:55

by Jes Sorensen

[permalink] [raw]
Subject: Re: [RFC][PATCH] A generic boolean

Jan Engelhardt wrote:
>> We know that today long is the only one that differs
>
> For "modern architectures" maybe, but older compilers (like Turbo C
> compiler (1990)), int is a 16 bit quantity, and therefore does differ, from
> today's implementations at least.

Excuse me, but this conversation was about compiling the Linux kernel.
What non GCC compilers do is irrelevant to this.

Jes