Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Richard Kenner]

    Which leads to the painfully obvious question -- what and where are
    the other places?

The obvious answer is to grep for the flag and see who uses it.

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Ziemowit Laski]

On Wednesday, Aug 14, 2002, at 12:20 US/Pacific, Richard Kenner wrote:

>     Which leads to the painfully obvious question -- what and where are
>     the other places?
>
> The obvious answer is to grep for the flag and see who uses it.

Yes, but which of those uses affect codegen? :-)  I guess I'm required
to present a proof that none of them do, which is an intractable
proposition.  So, I guess I'll try and see if the ObjC bug can
be addressed in some other way...

--Zem
--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Ziemowit Laski]

On Tuesday, August 20, 2002, at 09:50 , Jason Merrill wrote:

> On Wed, 14 Aug 2002 11:25:14 -0700, Ziemowit Laski <zlaski@apple.com> 
> wrote:
>
>> As I said, ObjC classes must also function as C structs (e.g., 
>> 'NSObject
>> *foo'); hence, the DECL_BIT_FIELD flag must still be set for bitfields.
>> We could add another, ObjC-specific bitfield ornament to the FIELD_DECL
>> tree (and hope that the rest of the compiler never clobbers it...), but
>> they we'd be storing the same piece of information twice.
>
> Why not just check DECL_C_BIT_FIELD?

Oh, there was another one...?  :-)

At any rate, I already committed a fix yesterday (strictly in ObjC land,
no less!) that fixed the bitfield encoding an promotion issues.

Thanks,

--Zem
--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Jason Merrill]

On Wed, 14 Aug 2002 11:25:14 -0700, Ziemowit Laski <zlaski@apple.com> wrote:

> As I said, ObjC classes must also function as C structs (e.g., 'NSObject
> *foo'); hence, the DECL_BIT_FIELD flag must still be set for bitfields.
> We could add another, ObjC-specific bitfield ornament to the FIELD_DECL
> tree (and hope that the rest of the compiler never clobbers it...), but
> they we'd be storing the same piece of information twice.

Why not just check DECL_C_BIT_FIELD?

Jason

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Ziemowit Laski]

On Wednesday, Aug 14, 2002, at 13:06 US/Pacific, Richard Henderson 
wrote:

> On Wed, Aug 14, 2002 at 12:17:58PM -0700, Ziemowit Laski wrote:
>> Which leads to the painfully obvious question -- what and where are 
>> the
>> other places?
>
> The two places in which bit fields are read and written:
> insert_bit_field and extract_bit_field.
>
> I suggest you write some test cases and examine the code
> generated with and without your clearing patch on at least
> x86 and ppc.

I did, and indeed the code _is_ different with my patch in
place, and exactly how I was warned it would be. :-(  (So yes,
thanks for holding me back on this one.)  The
difference disappears at -O2, presumably because the
optimizer throws away the unecessary ANDs and ORs.

So now, it seems there are two possible approaches for me to
consider:
   (1) Look at the logic in insert_bit_field and extract_bit_field
       to see if I can make up for what I took away from layout_decl;
   (2) Come up with a grody ObjC hack to work around the
       whole DECL_BIT_FIELD issue.

Currently, I am of the opinion that (1) would be the cleaner of
the two approaches, so I'll try this first.

Thank you for your patience,

--Zem

--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Ziemowit Laski]

On Wednesday, Aug 14, 2002, at 13:06 US/Pacific, Richard Henderson 
wrote:

> On Wed, Aug 14, 2002 at 12:17:58PM -0700, Ziemowit Laski wrote:
>> Which leads to the painfully obvious question -- what and where are 
>> the
>> other places?
>
> The two places in which bit fields are read and written:
> insert_bit_field and extract_bit_field.

Thanks.
>
> I suggest you write some test cases and examine the code
> generated with and without your clearing patch on at least
> x86 and ppc.

Will do.

--Zem
--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Richard Henderson]

On Wed, Aug 14, 2002 at 12:17:58PM -0700, Ziemowit Laski wrote:
> Which leads to the painfully obvious question -- what and where are the 
> other places?

The two places in which bit fields are read and written:
insert_bit_field and extract_bit_field.

I suggest you write some test cases and examine the code
generated with and without your clearing patch on at least
x86 and ppc.


r~

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Ziemowit Laski]

On Wednesday, Aug 14, 2002, at 12:03 US/Pacific, Richard Kenner wrote:

>     Judging from Richard Henderson's earlier e-mail, it is not clear to
>     me whether not clearing DECL_BIT_FIELD will affect codegen.  This 
> is,
>     I think, the question that needs to be answered.  Alas, I'm in no
>     position to answer it. :-(
>
> It will certainly *affect* it.  RTH's claim is just that there is
> least one place where it might not matter, not that *all* places are 
> such.

Which leads to the painfully obvious question -- what and where are the 
other
places?

--Zem
--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Richard Kenner]

    Judging from Richard Henderson's earlier e-mail, it is not clear to
    me whether not clearing DECL_BIT_FIELD will affect codegen.  This is,
    I think, the question that needs to be answered.  Alas, I'm in no
    position to answer it. :-(

It will certainly *affect* it.  RTH's claim is just that there is
least one place where it might not matter, not that *all* places are such.

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Ziemowit Laski]

On Wednesday, Aug 14, 2002, at 11:38 US/Pacific, Richard Kenner wrote:

>     The only problem is that the bitfield declarations occasionally get
>     clobbered, which is what my patch addresses.
>
> But that's the point we're trying to make: since that flag is used for
> code generation, it doesn't represent the language concept any more.
> If you need one that does represent that concept, you have to add one.

Judging from Richard Henderson's earlier e-mail, it is not clear to
me whether not clearing DECL_BIT_FIELD will affect codegen.  This is,
I think, the question that needs to be answered.  Alas, I'm in no
position to answer it. :-(

--Zem
--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Richard Kenner]

    The only problem is that the bitfield declarations occasionally get
    clobbered, which is what my patch addresses.

But that's the point we're trying to make: since that flag is used for
code generation, it doesn't represent the language concept any more.
If you need one that does represent that concept, you have to add one.

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Ziemowit Laski]

On Tuesday, Aug 13, 2002, at 19:57 US/Pacific, Richard Henderson wrote:

> On Tue, Aug 13, 2002 at 06:43:19PM -0700, Ziemowit Laski wrote:
>> You mean the back-end isn't smart enough to detect bitfields that
>> could be accessed as bytes or words?
>
> I think it is.  Some of the first checks in extract_bit_field
> check for the nicely aligned cases.
>
>>> Why not just use a language-specific flag instead of DECL_BIT_FIELD?
>>
>> Because the instance variables of ObjC classes are parsed/handled by
>> the same machinery as fields in a C struct.  And for a good reason --
>> ObjC classes _are_ C structs (among other things).
>
> What has this got to do with not using a language-specific flag
> to represent a language-specific concept?  Seems to me you're
> only getting yourself in trouble by abusing the generic bit.

Except that bitfields are hardly a language-specific concept. :-)
As I said, ObjC classes must also function as C structs (e.g., 
'NSObject *foo');
hence, the DECL_BIT_FIELD flag must still be set for bitfields.  We 
could
add another, ObjC-specific bitfield ornament to the FIELD_DECL tree 
(and hope
that the rest of the compiler never clobbers it...), but they we'd be 
storing
the same piece of information twice.

Bitfields in ObjC are the same thing as bitfields in C, and so it does 
not
make sense to encode them differently (esp. since ObjC is implemented 
on top
of C).  The only problem is that the bitfield declarations occasionally 
get
clobbered, which is what my patch addresses.

--Zem
--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Richard Kenner]

    You mean the back-end isn't smart enough to detect bitfields that
    could be accessed as bytes or words?

You are proposing to delete the code that does that!

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Richard Henderson]

On Tue, Aug 13, 2002 at 06:43:19PM -0700, Ziemowit Laski wrote:
> You mean the back-end isn't smart enough to detect bitfields that
> could be accessed as bytes or words?

I think it is.  Some of the first checks in extract_bit_field
check for the nicely aligned cases.

> > Why not just use a language-specific flag instead of DECL_BIT_FIELD?
> 
> Because the instance variables of ObjC classes are parsed/handled by
> the same machinery as fields in a C struct.  And for a good reason --
> ObjC classes _are_ C structs (among other things).

What has this got to do with not using a language-specific flag
to represent a language-specific concept?  Seems to me you're
only getting yourself in trouble by abusing the generic bit.


r~

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Ziemowit Laski]

On Tuesday, Aug 13, 2002, at 18:37 US/Pacific, Richard Kenner wrote:

>     What kind of performance problems were you thinking of?
>
> Well, DECL_BIT_FIELD means to use bitfield access techniques, which add
> code.  If we use them when we don't need to, we generate worse code.

You mean the back-end isn't smart enough to detect bitfields that
could be accessed as bytes or words?

>
>     I suppose the ObjC front-end could be taught to keep separate
>     copies of instance variables for super classes lest they get
>     side-effected too often, but that is terribly kludgey.
>
> Why not just use a language-specific flag instead of DECL_BIT_FIELD?

Because the instance variables of ObjC classes are parsed/handled by
the same machinery as fields in a C struct.  And for a good reason --
ObjC classes _are_ C structs (among other things).

--Zem
--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Richard Kenner]

    What kind of performance problems were you thinking of?

Well, DECL_BIT_FIELD means to use bitfield access techniques, which add
code.  If we use them when we don't need to, we generate worse code.

    I suppose the ObjC front-end could be taught to keep separate
    copies of instance variables for super classes lest they get
    side-effected too often, but that is terribly kludgey.

Why not just use a language-specific flag instead of DECL_BIT_FIELD?

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Ziemowit Laski]

On Tuesday, Aug 13, 2002, at 18:30 US/Pacific, Richard Kenner wrote:

> This looks like it will cause performance problems to me.  Isn't there
> some other way of keeping the ObjC front end from messing up?

What kind of performance problems were you thinking of?

I suppose the ObjC front-end could be taught to keep separate
copies of instance variables for super classes lest they get
side-effected too often, but that is terribly kludgey.

--Zem
--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re: PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Richard Kenner]

This looks like it will cause performance problems to me.  Isn't there
some other way of keeping the ObjC front end from messing up?

PING: PATCH: Do not clear the DECL_BIT_FIELD flag (prelude to ObjC fix)

[Ziemowit Laski]

Can someone please approve this?  I'd like to get this fix,
along with the ObjC bitfield alignment fix, before Stage 3
sets in.

I've bootstrapped and tested this before, but now I'm doing
so again with the Ada bits in place.  No surprises thus
far.

--Zem
===================================================================

The following trivial patch does not accomplish much in and of
itself, although it also causes no regressions, as confirmed by
a full bootstrap on i686-pc-linux-gnu.  The patch is needed
because clearing the DECL_BIT_FIELD flag often leads the ObjC
front-end to "promote" these bitfields to full-blown integers,
und das ist keine gut. :-)  I'm offering this separately so
that it can be reviewed by non-ObjC folks (and also because
Stan can't approve it :-) ).

OK for TOT?  An ObjC patch taking advantage of this fix
is forthcoming.

2002-08-13  Ziemowit Laski <zlaski@apple.com>

         * stor-layout.c (layout_decl): Do not clear the
         DECL_BIT_FIELD flag.


Index: stor-layout.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/stor-layout.c,v
retrieving revision 1.128
diff -c -3 -p -r1.128 stor-layout.c
*** stor-layout.c       7 Aug 2002 23:20:34 -0000       1.128
--- stor-layout.c       12 Aug 2002 22:51:27 -0000
*************** layout_decl (decl, known_align)
*** 423,439 ****
           DECL_ALIGN (decl) = MAX (GET_MODE_ALIGNMENT (xmode),
                                    DECL_ALIGN (decl));
           DECL_MODE (decl) = xmode;
-         DECL_BIT_FIELD (decl) = 0;
         }
       }
-
-   /* Turn off DECL_BIT_FIELD if we won't need it set.  */
-   if (code == FIELD_DECL && DECL_BIT_FIELD (decl)
-       && TYPE_MODE (type) == BLKmode && DECL_MODE (decl) == BLKmode
-       && known_align >= TYPE_ALIGN (type)
-       && DECL_ALIGN (decl) >= TYPE_ALIGN (type)
-       && DECL_SIZE_UNIT (decl) != 0)
-     DECL_BIT_FIELD (decl) = 0;

     /* Evaluate nonconstant size only once, either now or as soon as 
safe.  */
     if (DECL_SIZE (decl) != 0 && TREE_CODE (DECL_SIZE (decl)) != 
INTEGER_CST)
--- 423,430 ----

--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477