Re: [PATCH-v2 2/4] Replace SYM_ and MOD_ #defines with enums in symbol.h.

[Samuel Bronson <naesten@gmail.com>]

On Tue, Jun 2, 2009 at 4:44 PM, Christopher Li <sparse@chrisli.org> wrote:
> On Mon, Jun 1, 2009 at 11:37 AM, Samuel Bronson <naesten@gmail.com> wrote:
>> Adjust the associated fields to match. While we're here, change
>> some nearby bitfields from "char" to "int" so GDB doesn't bother
>> showing character literals for them.
>
>> +enum modifier {
>> +       MOD_AUTO        = 1 << 0,
>
> The patch does not apply to my tree. The modifier is definitely not
> an enum because we apply bit operation to it.

Hmm, which tree is yours? I pulled from
git://git.kernel.org/pub/scm/devel/sparse/sparse.git

As for the bit operations, those are perfectly legal for enum values,
and there are two similar enum types defined in this very header:

enum namespace {
        NS_NONE         = 0,
        NS_MACRO        = 1,
        NS_TYPEDEF      = 2,
        NS_STRUCT       = 4,  // Also used for unions and enums.
        NS_LABEL        = 8,
        NS_SYMBOL       = 16,
        NS_ITERATOR     = 32,
        NS_PREPROCESSOR = 64,
        NS_UNDEF        = 128,
        NS_KEYWORD      = 256,
};

enum keyword {
        KW_SPECIFIER    = 1 << 0,
        KW_MODIFIER     = 1 << 1,
        KW_QUALIFIER    = 1 << 2,
        KW_ATTRIBUTE    = 1 << 3,
        KW_TYPEOF       = 1 << 4,
        KW_STATEMENT    = 1 << 5,
        KW_ASM          = 1 << 6,
        KW_MODE         = 1 << 7,
};

> Changing bitfield from "char" to "int" has risk of making the symbol
> struct bigger. We don't want that because symbol is a very common
> structure for sparse.

When could this actually happen? Here (on i386), pahole says this:

struct symbol {
        enum type                  type:8;               /*     0:24  4 */
        enum namespace             namespace:9;          /*     0:15  4 */
        unsigned int               used:1;               /*     0:14  4 */
        enum sym_attr              attr:2;               /*     0:12  4 */
        unsigned int               enum_member:1;        /*     0:11  4 */
        unsigned int               bound:1;              /*     0:10  4 */

        /* XXX 10 bits hole, try to pack */

        struct position            pos;                  /*     4     8 */

[...]

        union {
                struct basic_block * bb_target;          /*           4 */
                void *             aux;                  /*           4 */
                struct {
                        char       kind;                 /*   116     1 */

                        /* Bitfield combined with previous fields */

                        unsigned int visited:1;          /*   116:23  4 */
                };                                       /*           4 */
        };                                               /*   116     4 */

If you run c2xml on the pahole output, you'll get:

  <symbol type="struct" id="_9" ident="position" file="-"
start-line="10" start-col="8" end-line="21" end-col="2" bit-si
ze="64" alignment="4" offset="0">

in the output, indicating that the 10-bit hole near the beginning is
not due to the use of "unsigned int" instead of "unsigned char" for
the bitfields before the hole, but due to the alignment required for
the "struct position" following the hole.

The "visited" bitfield doesn't cause a problem either, because it is
packed into the same word as the "kind" field, and the other members
of that union both take a word anyway.

Looking it up in K&R 2nd edition, I see that the treatment of
bitfields is almost entirely implementation-defined, so I
unfortunately can't really make any general statements about this --
does anyone (perhaps someone on another architecture) have something
to add?
--
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