Message ID | 1314021451-24808-4-git-send-email-penberg@kernel.org (mailing list archive) |
---|---|
State | Rejected, archived |
Headers | show |
On Mon, Aug 22, 2011 at 04:57:30PM +0300, Pekka Enberg wrote: > The value of 'ctype->bit_size' is set to 1 for booleans which confuses the i386 > backend: > > ./compile allocate.c > compile: compile-i386.c:1406: emit_binop: Assertion `0' failed. > Aborted > > Looking at the code, we assume that "bit_size / 8" gives a sane result on > various places. This patch fixes the problem by bumping bit_size to 8 for > booleans. This also makes sizeof(_Bool) return 1 which is consistent with what > GCC 4.4.3, for example, does. Seems reasonable; that would make it consistent with the behavior of structs or arrays of bool, at least in GCC. As far as I know, GCC doesn't have any logic to attempt to compress a bool down to a single bit of storage. - Josh Triplett -- 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 Mon, Aug 22, 2011 at 6:57 AM, Pekka Enberg <penberg@kernel.org> wrote: > The value of 'ctype->bit_size' is set to 1 for booleans which confuses the i386 > backend: > > ./compile allocate.c > compile: compile-i386.c:1406: emit_binop: Assertion `0' failed. > Aborted > > Looking at the code, we assume that "bit_size / 8" gives a sane result on > various places. This patch fixes the problem by bumping bit_size to 8 for > booleans. This also makes sizeof(_Bool) return 1 which is consistent with what > GCC 4.4.3, for example, does. > > diff --git a/target.c b/target.c > index 17b228a..6a535bc 100644 > --- a/target.c > +++ b/target.c > @@ -14,7 +14,7 @@ int max_alignment = 16; > /* > * Integer data types > */ > -int bits_in_bool = 1; > +int bits_in_bool = 8; I object this part. I consider the sizeof(_Bool) == 1 as external behaviour. But internally we should know that the real useful part of bool is in just one bit, not any bit of that 1 byte storage. Allowing _Bool to be addressable is a separate issue. I consider those rest of 7 bit as paddings. Not the useful part of _Bool. > int bits_in_char = 8; > int bits_in_short = 16; > int bits_in_int = 32; > diff --git a/validation/sizeof-bool.c b/validation/sizeof-bool.c > index 6c68748..31b0585 100644 > --- a/validation/sizeof-bool.c > +++ b/validation/sizeof-bool.c > @@ -4,9 +4,6 @@ static int a(void) > } > /* > * check-name: sizeof(_Bool) is valid > - * check-description: sizeof(_Bool) was rejected because _Bool is not an even > - * number of bytes > * check-error-start > -sizeof-bool.c:3:16: warning: expression using sizeof bool > * check-error-end > */ We should just fix the validation. 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 Fri, Aug 26, 2011 at 6:59 AM, Christopher Li <sparse@chrisli.org> wrote: > On Mon, Aug 22, 2011 at 6:57 AM, Pekka Enberg <penberg@kernel.org> wrote: >> The value of 'ctype->bit_size' is set to 1 for booleans which confuses the i386 >> backend: >> >> ./compile allocate.c >> compile: compile-i386.c:1406: emit_binop: Assertion `0' failed. >> Aborted >> >> Looking at the code, we assume that "bit_size / 8" gives a sane result on >> various places. This patch fixes the problem by bumping bit_size to 8 for >> booleans. This also makes sizeof(_Bool) return 1 which is consistent with what >> GCC 4.4.3, for example, does. >> >> diff --git a/target.c b/target.c >> index 17b228a..6a535bc 100644 >> --- a/target.c >> +++ b/target.c >> @@ -14,7 +14,7 @@ int max_alignment = 16; >> /* >> * Integer data types >> */ >> -int bits_in_bool = 1; >> +int bits_in_bool = 8; > > I object this part. I consider the sizeof(_Bool) == 1 as external behaviour. > But internally we should know that the real useful part of bool is in just one > bit, not any bit of that 1 byte storage. You missed the most important part of my reasoning: sparse code already expects "bit_size / 8" to return a non-zero number and it's not just compile-i386.c! So while I don't disagree with you that we should internally know that a bool is just one bit, I don't consider that to be relevant for this particular patch. Pekka -- 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
diff --git a/target.c b/target.c index 17b228a..6a535bc 100644 --- a/target.c +++ b/target.c @@ -14,7 +14,7 @@ int max_alignment = 16; /* * Integer data types */ -int bits_in_bool = 1; +int bits_in_bool = 8; int bits_in_char = 8; int bits_in_short = 16; int bits_in_int = 32; diff --git a/validation/sizeof-bool.c b/validation/sizeof-bool.c index 6c68748..31b0585 100644 --- a/validation/sizeof-bool.c +++ b/validation/sizeof-bool.c @@ -4,9 +4,6 @@ static int a(void) } /* * check-name: sizeof(_Bool) is valid - * check-description: sizeof(_Bool) was rejected because _Bool is not an even - * number of bytes * check-error-start -sizeof-bool.c:3:16: warning: expression using sizeof bool * check-error-end */
The value of 'ctype->bit_size' is set to 1 for booleans which confuses the i386 backend: ./compile allocate.c compile: compile-i386.c:1406: emit_binop: Assertion `0' failed. Aborted Looking at the code, we assume that "bit_size / 8" gives a sane result on various places. This patch fixes the problem by bumping bit_size to 8 for booleans. This also makes sizeof(_Bool) return 1 which is consistent with what GCC 4.4.3, for example, does. Cc: Christopher Li <sparse@chrisli.org> Cc: Jeff Garzik <jgarzik@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Pekka Enberg <penberg@kernel.org> --- target.c | 2 +- validation/sizeof-bool.c | 3 --- 2 files changed, 1 insertions(+), 4 deletions(-)