diff mbox

[4/5] sparse, i386: Fix boolean bit size

Message ID 1314021451-24808-4-git-send-email-penberg@kernel.org (mailing list archive)
State Rejected, archived
Headers show

Commit Message

Pekka Enberg Aug. 22, 2011, 1:57 p.m. UTC
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(-)

Comments

Josh Triplett Aug. 22, 2011, 3:28 p.m. UTC | #1
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
Christopher Li Aug. 26, 2011, 3:59 a.m. UTC | #2
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
Pekka Enberg Aug. 26, 2011, 5:28 a.m. UTC | #3
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 mbox

Patch

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
  */