Patchwork [RFC] : Define "bool" as _Bool when bootstrapping with gcc >= 4.4

login
register
mail settings
Submitter Uros Bizjak
Date Sept. 21, 2010, 6:43 p.m.
Message ID <AANLkTi=_UENrSqsihzU5JtP7CXCW6WHQPF1OG7ciFbYW@mail.gmail.com>
Download mbox | patch
Permalink /patch/65366/
State New
Headers show

Comments

Uros Bizjak - Sept. 21, 2010, 6:43 p.m.
On Tue, Sep 21, 2010 at 8:33 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Tue, Sep 21, 2010 at 11:29 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> Hello!
>>
>> Attached patch defines bool as _Bool when bootstrapping (or compiling
>> with gcc >= 4.4).
>>
>> 2010-09-21  Uros Bizjak  <ubizjak@gmail.com>
>>
>>        * system.h (bool): Define as _Bool for gcc >= 4.4.
>>        (BOOL_BITFILED): Ditto.
>
> I think one issue is that libcpp needs to make sure it has the same
> definition for bool also.

Hm, indeed. I have started bootstrap on x86_64-pc-linux-gnu with the
attached patch (that copies relevant parts of gcc/system.h to
libgcc/system.h).

Uros.
Tom Tromey - Sept. 21, 2010, 7:23 p.m.
>>>>> "Uros" == Uros Bizjak <ubizjak@gmail.com> writes:

Andrew> I think one issue is that libcpp needs to make sure it has the same
Andrew> definition for bool also.

Uros> Hm, indeed. I have started bootstrap on x86_64-pc-linux-gnu with the
Uros> attached patch (that copies relevant parts of gcc/system.h to
Uros> libgcc/system.h).

Uros>  /* Provide a fake boolean type.  We make no attempt to use the
Uros>     C99 _Bool, as it may not be available in the bootstrap compiler,
Uros> -   and even if it is, it is liable to be buggy.  
Uros> +   and even if it is, it is liable to be buggy.

This comment needs to be updated to reflect the new reality.

I didn't read the patch too closely this time around, but the idea seems
reasonable to me.

Tom
Uros Bizjak - Sept. 21, 2010, 8:42 p.m.
On Tue, Sep 21, 2010 at 9:23 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Uros" == Uros Bizjak <ubizjak@gmail.com> writes:
>
> Andrew> I think one issue is that libcpp needs to make sure it has the same
> Andrew> definition for bool also.
>
> Uros> Hm, indeed. I have started bootstrap on x86_64-pc-linux-gnu with the
> Uros> attached patch (that copies relevant parts of gcc/system.h to
> Uros> libgcc/system.h).
>
> Uros>  /* Provide a fake boolean type.  We make no attempt to use the
> Uros>     C99 _Bool, as it may not be available in the bootstrap compiler,
> Uros> -   and even if it is, it is liable to be buggy.
> Uros> +   and even if it is, it is liable to be buggy.
>
> This comment needs to be updated to reflect the new reality.

Thanks - perhaps something like following text:

<quote>
/* Only use C99 _Bool with GCC.  Otherwise, provide a fake boolean type, as
   _Bool may not be available in the bootstrap compiler, and even if it is,
   it is liable to be buggy.

   This must be defined after all inclusion of system headers, as some of
   them will mess us up.  */
</quote>

> I didn't read the patch too closely this time around, but the idea seems
> reasonable to me.

Uros.
Richard Guenther - Sept. 21, 2010, 10:07 p.m.
On Tue, Sep 21, 2010 at 9:23 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Uros" == Uros Bizjak <ubizjak@gmail.com> writes:
>
> Andrew> I think one issue is that libcpp needs to make sure it has the same
> Andrew> definition for bool also.
>
> Uros> Hm, indeed. I have started bootstrap on x86_64-pc-linux-gnu with the
> Uros> attached patch (that copies relevant parts of gcc/system.h to
> Uros> libgcc/system.h).
>
> Uros>  /* Provide a fake boolean type.  We make no attempt to use the
> Uros>     C99 _Bool, as it may not be available in the bootstrap compiler,
> Uros> -   and even if it is, it is liable to be buggy.
> Uros> +   and even if it is, it is liable to be buggy.
>
> This comment needs to be updated to reflect the new reality.
>
> I didn't read the patch too closely this time around, but the idea seems
> reasonable to me.

Hm.  Given different semantic of int and _Bool can we force the use of
int (or char) for stage1 to make sure we keep both variants working?

Thanks,
Richard.

> Tom
>
Uros Bizjak - Sept. 22, 2010, 6:25 a.m.
On Wed, Sep 22, 2010 at 12:07 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Tue, Sep 21, 2010 at 9:23 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> "Uros" == Uros Bizjak <ubizjak@gmail.com> writes:
>>
>> Andrew> I think one issue is that libcpp needs to make sure it has the same
>> Andrew> definition for bool also.
>>
>> Uros> Hm, indeed. I have started bootstrap on x86_64-pc-linux-gnu with the
>> Uros> attached patch (that copies relevant parts of gcc/system.h to
>> Uros> libgcc/system.h).
>>
>> Uros>  /* Provide a fake boolean type.  We make no attempt to use the
>> Uros>     C99 _Bool, as it may not be available in the bootstrap compiler,
>> Uros> -   and even if it is, it is liable to be buggy.
>> Uros> +   and even if it is, it is liable to be buggy.
>>
>> This comment needs to be updated to reflect the new reality.
>>
>> I didn't read the patch too closely this time around, but the idea seems
>> reasonable to me.
>
> Hm.  Given different semantic of int and _Bool can we force the use of
> int (or char) for stage1 to make sure we keep both variants working?

No worries, since *.md processors didn't hear about bool (yet?).

OTOH, I don't think we should consider a gcc bootstrap as one giant
testcase. IMO, _Bool is the most appropriate type for predicate
functions, so why cripple these types to increase coverage for
"alternative" implementation? We have testsuite for this, and I'm sure
that a failure in such a basic functionality will be detected in no
time.

OTOOH, It is also nice for gcc to eat is own dogfood [1] ;)

[1] http://en.wikipedia.org/wiki/Dogfooding

Uros.
Richard Guenther - Sept. 22, 2010, 8:56 a.m.
On Wed, Sep 22, 2010 at 8:25 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Sep 22, 2010 at 12:07 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Tue, Sep 21, 2010 at 9:23 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>>> "Uros" == Uros Bizjak <ubizjak@gmail.com> writes:
>>>
>>> Andrew> I think one issue is that libcpp needs to make sure it has the same
>>> Andrew> definition for bool also.
>>>
>>> Uros> Hm, indeed. I have started bootstrap on x86_64-pc-linux-gnu with the
>>> Uros> attached patch (that copies relevant parts of gcc/system.h to
>>> Uros> libgcc/system.h).
>>>
>>> Uros>  /* Provide a fake boolean type.  We make no attempt to use the
>>> Uros>     C99 _Bool, as it may not be available in the bootstrap compiler,
>>> Uros> -   and even if it is, it is liable to be buggy.
>>> Uros> +   and even if it is, it is liable to be buggy.
>>>
>>> This comment needs to be updated to reflect the new reality.
>>>
>>> I didn't read the patch too closely this time around, but the idea seems
>>> reasonable to me.
>>
>> Hm.  Given different semantic of int and _Bool can we force the use of
>> int (or char) for stage1 to make sure we keep both variants working?
>
> No worries, since *.md processors didn't hear about bool (yet?).
>
> OTOH, I don't think we should consider a gcc bootstrap as one giant
> testcase. IMO, _Bool is the most appropriate type for predicate
> functions, so why cripple these types to increase coverage for
> "alternative" implementation? We have testsuite for this, and I'm sure
> that a failure in such a basic functionality will be detected in no
> time.

Because bool and int (or char) are not the same.  There have been
bugs in GCC because of this, see also Paolos reply.

Richard.

> OTOOH, It is also nice for gcc to eat is own dogfood [1] ;)
>
> [1] http://en.wikipedia.org/wiki/Dogfooding
>
> Uros.
>

Patch

Index: gcc/system.h
===================================================================
--- gcc/system.h	(revision 164480)
+++ gcc/system.h	(working copy)
@@ -621,29 +621,29 @@ 
    This must be after all inclusion of system headers, as some of
    them will mess us up.  */
 
-#undef TRUE
-#undef FALSE
+#ifndef __cplusplus
+# undef bool
 
-#ifdef __cplusplus
-  /* Obsolete.  */
-# define TRUE true
-# define FALSE false
-#else /* !__cplusplus */
-# undef bool
+# if GCC_VERSION >= 4004
+#  define bool _Bool
+#  define BOOL_BITFIELD _Bool
+# else
+#  define bool unsigned char
+/* Some compilers do not allow the use of unsigned char in bitfields.  */
+#  define BOOL_BITFIELD unsigned int
+# endif
+
 # undef true
 # undef false
-
-# define bool unsigned char
 # define true 1
 # define false 0
-
-  /* Obsolete.  */
-# define TRUE true
-# define FALSE false
 #endif /* !__cplusplus */
 
-/* Some compilers do not allow the use of unsigned char in bitfields.  */
-#define BOOL_BITFIELD unsigned int
+/* Obsolete.  */
+#undef TRUE
+#undef FALSE
+#define TRUE true
+#define FALSE false
 
 /* As the last action in this file, we poison the identifiers that
    shouldn't be used.  Note, luckily gcc-3.0's token-based integrated
Index: libcpp/system.h
===================================================================
--- libcpp/system.h	(revision 164480)
+++ libcpp/system.h	(working copy)
@@ -379,24 +379,34 @@ 
 
 /* Provide a fake boolean type.  We make no attempt to use the
    C99 _Bool, as it may not be available in the bootstrap compiler,
-   and even if it is, it is liable to be buggy.  
+   and even if it is, it is liable to be buggy.
    This must be after all inclusion of system headers, as some of
    them will mess us up.  */
-#undef bool
-#undef true
-#undef false
-#undef TRUE
-#undef FALSE
 
 #ifndef __cplusplus
-#define bool unsigned char
-#endif
-#define true 1
-#define false 0
+# undef bool
 
+# if GCC_VERSION >= 4004
+#  define bool _Bool
+#  define BOOL_BITFIELD _Bool
+# else
+#  define bool unsigned char
 /* Some compilers do not allow the use of unsigned char in bitfields.  */
-#define BOOL_BITFIELD unsigned int
+#  define BOOL_BITFIELD unsigned int
+# endif
 
+# undef true
+# undef false
+# define true 1
+# define false 0
+#endif /* !__cplusplus */
+
+/* Obsolete.  */
+#undef TRUE
+#undef FALSE
+#define TRUE true
+#define FALSE false
+
 /* Poison identifiers we do not want to use.  */
 #if (GCC_VERSION >= 3000)
 #undef calloc