diff mbox

[3/8] add default for PCC_BITFIELD_TYPE_MATTERS

Message ID 20150430001316.GC17482@tsaunders-iceball.corp.tor1.mozilla.com
State New
Headers show

Commit Message

Trevor Saunders April 30, 2015, 12:13 a.m. UTC
On Wed, Apr 29, 2015 at 04:29:26PM -0600, Jeff Law wrote:
> On 04/29/2015 04:25 PM, Trevor Saunders wrote:
> >On Wed, Apr 29, 2015 at 04:00:47PM -0600, Jeff Law wrote:
> >>On 04/29/2015 03:55 PM, Andreas Schwab wrote:
> >>>Trevor Saunders <tbsaunde@tbsaunde.org> writes:
> >>>
> >>>>it passes for me on x86_64-linux after that commit, what platform is
> >>>>this?
> >>>
> >>>Any one with #undef PCC_BITFIELD_TYPE_MATTERS.  See libobjc/encoding.c.
> >>Can't you just answer the question Andreas instead of making Trevor go
> >>looking around?  You already have this information, why make his job harder?
> >
> >
> >actually pointing out libojc/encoding.c was more useful since that makes
> >it pretty clear the ifndef PCC_BITFIELD_TYPE_MATTERS there just needs to
> >be changed to #if !
> >
> >>Trevor, try m68k-elf cross.
> >
> >ok, lets see if I can get this to work (its an execution test that
> >breaks, so I'll need to setup binutils and qemu)
> I've actually got a aranym m68k emulator here...  So I can do native
> bootstrapping and testing for m68k linux.  If you want me to test something,
> just let me know -- FWIW, it takes a week or so to bootstrap ;-)

Well the fix is just this


   I looked at the .i built for m68k-linux-elf before and after the
   patch and it does what I expect it should do and is a pretty obvious
   part that should go with the rest of this patch.

   Obviously something else should be done in the long run at least to
   separate gcc and libobjc configuration, but I can't see an argument
   for that patch not being ok for now so I'm inclined to check it in
   with the current level of testing.

   Trev

> 
> jeff

Comments

Trevor Saunders April 30, 2015, 2:10 a.m. UTC | #1
On Wed, Apr 29, 2015 at 08:13:16PM -0400, Trevor Saunders wrote:
> On Wed, Apr 29, 2015 at 04:29:26PM -0600, Jeff Law wrote:
> > On 04/29/2015 04:25 PM, Trevor Saunders wrote:
> > >On Wed, Apr 29, 2015 at 04:00:47PM -0600, Jeff Law wrote:
> > >>On 04/29/2015 03:55 PM, Andreas Schwab wrote:
> > >>>Trevor Saunders <tbsaunde@tbsaunde.org> writes:
> > >>>
> > >>>>it passes for me on x86_64-linux after that commit, what platform is
> > >>>>this?
> > >>>
> > >>>Any one with #undef PCC_BITFIELD_TYPE_MATTERS.  See libobjc/encoding.c.
> > >>Can't you just answer the question Andreas instead of making Trevor go
> > >>looking around?  You already have this information, why make his job harder?
> > >
> > >
> > >actually pointing out libojc/encoding.c was more useful since that makes
> > >it pretty clear the ifndef PCC_BITFIELD_TYPE_MATTERS there just needs to
> > >be changed to #if !
> > >
> > >>Trevor, try m68k-elf cross.
> > >
> > >ok, lets see if I can get this to work (its an execution test that
> > >breaks, so I'll need to setup binutils and qemu)
> > I've actually got a aranym m68k emulator here...  So I can do native
> > bootstrapping and testing for m68k linux.  If you want me to test something,
> > just let me know -- FWIW, it takes a week or so to bootstrap ;-)
> 
> Well the fix is just this
> 
> diff --git a/libobjc/encoding.c b/libobjc/encoding.c
> index 7333908..20ace46 100644
> --- a/libobjc/encoding.c
> +++ b/libobjc/encoding.c
> @@ -1167,7 +1167,7 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout)
>    /* Record must have at least as much alignment as any field.
>       Otherwise, the alignment of the field within the record
>       is meaningless.  */
> -#ifndef PCC_BITFIELD_TYPE_MATTERS
> +#if !PCC_BITFIELD_TYPE_MATTERS
>    layout->record_align = MAX (layout->record_align, desired_align);
>  #else	/* PCC_BITFIELD_TYPE_MATTERS */
>    if (*type == _C_BFLD)
> 
>    I looked at the .i built for m68k-linux-elf before and after the
>    patch and it does what I expect it should do and is a pretty obvious
>    part that should go with the rest of this patch.
> 
>    Obviously something else should be done in the long run at least to
>    separate gcc and libobjc configuration, but I can't see an argument
>    for that patch not being ok for now so I'm inclined to check it in
>    with the current level of testing.

I decided to commit this, it seems like testing it can be slow on some
targets and I did a bootstrap on x86_64-linux-gnu (with regtest queued)
and it seems very very unlikely to break anything else.

Trev

> 
>    Trev
> 
> > 
> > jeff
Jeff Law April 30, 2015, 3:54 a.m. UTC | #2
On 04/29/2015 08:10 PM, Trevor Saunders wrote:
>
> I decided to commit this, it seems like testing it can be slow on some
> targets and I did a bootstrap on x86_64-linux-gnu (with regtest queued)
> and it seems very very unlikely to break anything else.
Seems reasonable.    Thanks for taking care of it quickly.

/me wonders if we should have an aranym instance for m68k testing in the 
compile farm.

jeff
Andreas Schwab April 30, 2015, 6:54 a.m. UTC | #3
Trevor Saunders <tbsaunde@tbsaunde.org> writes:

>> diff --git a/libobjc/encoding.c b/libobjc/encoding.c
>> index 7333908..20ace46 100644
>> --- a/libobjc/encoding.c
>> +++ b/libobjc/encoding.c
>> @@ -1167,7 +1167,7 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout)
>>    /* Record must have at least as much alignment as any field.
>>       Otherwise, the alignment of the field within the record
>>       is meaningless.  */
>> -#ifndef PCC_BITFIELD_TYPE_MATTERS
>> +#if !PCC_BITFIELD_TYPE_MATTERS

With `#define PCC_BITFIELD_TYPE_MATTERS true' this expands to `#if
!true' which evaluates to 1 since true isn't a defined identifier?

Andreas.
Trevor Saunders April 30, 2015, 11:58 a.m. UTC | #4
On Thu, Apr 30, 2015 at 08:54:05AM +0200, Andreas Schwab wrote:
> Trevor Saunders <tbsaunde@tbsaunde.org> writes:
> 
> >> diff --git a/libobjc/encoding.c b/libobjc/encoding.c
> >> index 7333908..20ace46 100644
> >> --- a/libobjc/encoding.c
> >> +++ b/libobjc/encoding.c
> >> @@ -1167,7 +1167,7 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout)
> >>    /* Record must have at least as much alignment as any field.
> >>       Otherwise, the alignment of the field within the record
> >>       is meaningless.  */
> >> -#ifndef PCC_BITFIELD_TYPE_MATTERS
> >> +#if !PCC_BITFIELD_TYPE_MATTERS
> 
> With `#define PCC_BITFIELD_TYPE_MATTERS true' this expands to `#if
> !true' which evaluates to 1 since true isn't a defined identifier?

I think true is a defined identifier since this is compiled as c11.

tbsaunde@iceball:/src/gcc1-opt$ cat test.c
#define FOO true
#if !FOO
hello
#endif
tbsaunde@iceball:/src/gcc1-opt$ gcc/xgcc -B gcc/ -E test.c
# 1 "test.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2
# 1 "test.c"


hello
tbsaunde@iceball:/src/gcc1-opt$

Trev

> 
> Andreas.
> 
> -- 
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
> "And now for something completely different."
Jakub Jelinek April 30, 2015, 12:13 p.m. UTC | #5
On Thu, Apr 30, 2015 at 07:58:33AM -0400, Trevor Saunders wrote:
> On Thu, Apr 30, 2015 at 08:54:05AM +0200, Andreas Schwab wrote:
> > Trevor Saunders <tbsaunde@tbsaunde.org> writes:
> > 
> > >> diff --git a/libobjc/encoding.c b/libobjc/encoding.c
> > >> index 7333908..20ace46 100644
> > >> --- a/libobjc/encoding.c
> > >> +++ b/libobjc/encoding.c
> > >> @@ -1167,7 +1167,7 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout)
> > >>    /* Record must have at least as much alignment as any field.
> > >>       Otherwise, the alignment of the field within the record
> > >>       is meaningless.  */
> > >> -#ifndef PCC_BITFIELD_TYPE_MATTERS
> > >> +#if !PCC_BITFIELD_TYPE_MATTERS
> > 
> > With `#define PCC_BITFIELD_TYPE_MATTERS true' this expands to `#if
> > !true' which evaluates to 1 since true isn't a defined identifier?
> 
> I think true is a defined identifier since this is compiled as c11.

true is not a defined identifier, neither in C89, nor in C99, nor in C11.
In C, true may be a macro if stdbool.h is included.
system.h has:
#undef TRUE
#undef FALSE

#ifdef __cplusplus
  /* Obsolete.  */
# define TRUE true
# define FALSE false
#else /* !__cplusplus */
# undef bool
# undef true
# undef false

# define bool unsigned char
# define true 1
# define false 0

  /* Obsolete.  */
# define TRUE true
# define FALSE false
#endif /* !__cplusplus */
if it is included.

	Jakub
Andreas Schwab April 30, 2015, 12:17 p.m. UTC | #6
Trevor Saunders <tbsaunde@tbsaunde.org> writes:

> I think true is a defined identifier since this is compiled as c11.
>
> tbsaunde@iceball:/src/gcc1-opt$ cat test.c
> #define FOO true
> #if !FOO
> hello
> #endif
> tbsaunde@iceball:/src/gcc1-opt$ gcc/xgcc -B gcc/ -E test.c
> # 1 "test.c"
> # 1 "<built-in>"
> # 1 "<command-line>"
> # 1 "/usr/include/stdc-predef.h" 1 3 4
> # 1 "<command-line>" 2
> # 1 "test.c"
>
>
> hello

Since you get hello this proves that true is *not* defined.

Andreas.
diff mbox

Patch

diff --git a/libobjc/encoding.c b/libobjc/encoding.c
index 7333908..20ace46 100644
--- a/libobjc/encoding.c
+++ b/libobjc/encoding.c
@@ -1167,7 +1167,7 @@  objc_layout_structure_next_member (struct objc_struct_layout *layout)
   /* Record must have at least as much alignment as any field.
      Otherwise, the alignment of the field within the record
      is meaningless.  */
-#ifndef PCC_BITFIELD_TYPE_MATTERS
+#if !PCC_BITFIELD_TYPE_MATTERS
   layout->record_align = MAX (layout->record_align, desired_align);
 #else	/* PCC_BITFIELD_TYPE_MATTERS */
   if (*type == _C_BFLD)