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

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

[Samuel Bronson]

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.

This should allow the gdbhelpers script to work with more versions of
GDB and/or build configurations.

Signed-off-by: Samuel Bronson <naesten@gmail.com>
---
 symbol.h |  112 ++++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 59 insertions(+), 53 deletions(-)

diff --git a/symbol.h b/symbol.h
index c4d7f28..7c396a0 100644
--- a/symbol.h
+++ b/symbol.h
@@ -69,6 +69,59 @@ enum keyword {
 	KW_MODE		= 1 << 7,
 };
 
+enum sym_attr {
+	SYM_ATTR_WEAK	= 0,
+	SYM_ATTR_NORMAL	= 1,
+	SYM_ATTR_STRONG	= 2,
+};
+
+/* Modifiers */
+enum modifier {
+	MOD_AUTO	= 1 << 0,
+	MOD_REGISTER	= 1 << 1,
+	MOD_STATIC	= 1 << 2,
+	MOD_EXTERN	= 1 << 3,
+
+	MOD_CONST	= 1 << 4,
+	MOD_VOLATILE	= 1 << 5,
+	MOD_SIGNED	= 1 << 6,
+	MOD_UNSIGNED	= 1 << 7,
+
+	MOD_CHAR	= 1 << 8,
+	MOD_SHORT	= 1 << 9,
+	MOD_LONG	= 1 << 10,
+	MOD_LONGLONG	= 1 << 11,
+
+	MOD_TYPEDEF	= 1 << 12,
+
+	MOD_INLINE	= 1 << 18,
+	MOD_ADDRESSABLE	= 1 << 19,
+
+	MOD_NOCAST	= 1 << 20,
+	MOD_NODEREF	= 1 << 21,
+	MOD_ACCESSED	= 1 << 22,
+	MOD_TOPLEVEL	= 1 << 23,	// scoping..
+
+	MOD_LABEL	= 1 << 24,
+	MOD_ASSIGNED	= 1 << 25,
+	MOD_TYPE	= 1 << 26,
+	MOD_SAFE	= 1 << 27,	// non-null/non-trapping pointer
+
+	MOD_USERTYPE	= 1 << 28,
+	MOD_FORCE       = 1 << 29,
+	MOD_EXPLICITLY_SIGNED	= 1 << 30,
+	MOD_BITWISE	= 1 << 31,
+
+	MOD_NONLOCAL	= (MOD_EXTERN | MOD_TOPLEVEL),
+	MOD_STORAGE	= (MOD_AUTO | MOD_REGISTER | MOD_STATIC | MOD_EXTERN | MOD_INLINE | MOD_TOPLEVEL | MOD_FORCE),
+	MOD_SIGNEDNESS	= (MOD_SIGNED | MOD_UNSIGNED | MOD_EXPLICITLY_SIGNED),
+	MOD_SPECIFIER	= (MOD_CHAR | MOD_SHORT | MOD_LONG | MOD_LONGLONG | MOD_SIGNEDNESS),
+	MOD_SIZE	= (MOD_CHAR | MOD_SHORT | MOD_LONG | MOD_LONGLONG),
+	MOD_IGNORE	= (MOD_TOPLEVEL | MOD_STORAGE | MOD_ADDRESSABLE |
+			   MOD_ASSIGNED | MOD_USERTYPE | MOD_ACCESSED | MOD_EXPLICITLY_SIGNED),
+	MOD_PTRINHERIT	= (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_STORAGE),
+};
+
 struct context {
 	struct expression *context;
 	unsigned int in, out, out_false;
@@ -80,7 +133,7 @@ extern struct context *alloc_context(void);
 DECLARE_PTR_LIST(context_list, struct context);
 
 struct ctype {
-	unsigned long modifiers;
+	enum modifier modifiers;
 	unsigned long alignment;
 	struct context_list *contexts;
 	unsigned int as;
@@ -103,14 +156,13 @@ struct symbol_op {
 extern int expand_safe_p(struct expression *expr, int cost);
 extern int expand_constant_p(struct expression *expr, int cost);
 
-#define SYM_ATTR_WEAK		0
-#define SYM_ATTR_NORMAL		1
-#define SYM_ATTR_STRONG		2
-
 struct symbol {
 	enum type type:8;
 	enum namespace namespace:9;
-	unsigned char used:1, attr:2, enum_member:1, bound:1;
+	unsigned int used:1;
+	enum sym_attr attr:2;
+	unsigned int enum_member:1;
+	unsigned int bound:1;
 	struct position pos;		/* Where this symbol was declared */
 	struct position endpos;		/* Where this symbol ends*/
 	struct ident *ident;		/* What identifier this symbol is associated with */
@@ -162,58 +214,12 @@ struct symbol {
 		void *aux;			/* Auxiliary info, e.g. backend information */
 		struct {			/* sparse ctags */
 			char kind;
-			unsigned char visited:1;
+			unsigned int visited:1;
 		};
 	};
 	pseudo_t pseudo;
 };
 
-/* Modifiers */
-#define MOD_AUTO	0x0001
-#define MOD_REGISTER	0x0002
-#define MOD_STATIC	0x0004
-#define MOD_EXTERN	0x0008
-
-#define MOD_CONST	0x0010
-#define MOD_VOLATILE	0x0020
-#define MOD_SIGNED	0x0040
-#define MOD_UNSIGNED	0x0080
-
-#define MOD_CHAR	0x0100
-#define MOD_SHORT	0x0200
-#define MOD_LONG	0x0400
-#define MOD_LONGLONG	0x0800
-
-#define MOD_TYPEDEF	0x1000
-
-#define MOD_INLINE	0x40000
-#define MOD_ADDRESSABLE	0x80000
-
-#define MOD_NOCAST	0x100000
-#define MOD_NODEREF	0x200000
-#define MOD_ACCESSED	0x400000
-#define MOD_TOPLEVEL	0x800000	// scoping..
-
-#define MOD_LABEL	0x1000000
-#define MOD_ASSIGNED	0x2000000
-#define MOD_TYPE	0x4000000
-#define MOD_SAFE	0x8000000	// non-null/non-trapping pointer
-
-#define MOD_USERTYPE	0x10000000
-#define MOD_FORCE	0x20000000
-#define MOD_EXPLICITLY_SIGNED	0x40000000
-#define MOD_BITWISE	0x80000000
-
-#define MOD_NONLOCAL	(MOD_EXTERN | MOD_TOPLEVEL)
-#define MOD_STORAGE	(MOD_AUTO | MOD_REGISTER | MOD_STATIC | MOD_EXTERN | MOD_INLINE | MOD_TOPLEVEL | MOD_FORCE)
-#define MOD_SIGNEDNESS	(MOD_SIGNED | MOD_UNSIGNED | MOD_EXPLICITLY_SIGNED)
-#define MOD_SPECIFIER	(MOD_CHAR | MOD_SHORT | MOD_LONG | MOD_LONGLONG | MOD_SIGNEDNESS)
-#define MOD_SIZE	(MOD_CHAR | MOD_SHORT | MOD_LONG | MOD_LONGLONG)
-#define MOD_IGNORE (MOD_TOPLEVEL | MOD_STORAGE | MOD_ADDRESSABLE |	\
-	MOD_ASSIGNED | MOD_USERTYPE | MOD_ACCESSED | MOD_EXPLICITLY_SIGNED)
-#define MOD_PTRINHERIT (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_STORAGE)
-

[PATCH 4/4] Have Makefile import config.mak if it exists.

[Samuel Bronson]

This will allow users to override build settings without dirtying their
trees, making life with `git stash' a bit easier.

Signed-off-by: Samuel Bronson <naesten@gmail.com>
---
 Makefile |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 18cec68..f45a74a 100644
--- a/Makefile
+++ b/Makefile
@@ -10,9 +10,10 @@ LDFLAGS += -g
 AR = ar
 
 #
-# For debugging, uncomment the next one
+# For debugging, put this in config.mak:
+#
+#     CFLAGS += -O0 -DDEBUG -g3 -gdwarf-2
 #
-#CFLAGS += -O0 -DDEBUG -g3 -gdwarf-2
 
 HAVE_LIBXML=$(shell pkg-config --exists libxml-2.0 && echo 'yes')
 
@@ -73,6 +74,11 @@ QUIET_LINK    = $(Q:@=@echo    '     LINK     '$@;)
 QUIET_INST_SH = $(Q:@=echo -n  '     INSTALL  ';)
 QUIET_INST    = $(Q:@=@echo -n '     INSTALL  ';)
 
+
+# Allow users to override build settings without dirtying their trees
+-include config.mak
+
+
 all: $(PROGRAMS) sparse.pc
 
 install: $(INST_PROGRAMS) $(LIBS) $(LIB_H) sparse.pc
-- 
1.6.3.1.169.g33fd.dirty

Re: [PATCH 4/4] Have Makefile import config.mak if it exists.

[Christopher Li]

[-- Attachment #1: Type: text/plain, Size: 338 bytes --]

On Mon, Jun 1, 2009 at 11:37 AM, Samuel Bronson <naesten@gmail.com> wrote:
> This will allow users to override build settings without dirtying their
> trees, making life with `git stash' a bit easier.
>
I modify the patch to use "local.mk" and add it in .gitignore.

If that works for you, I am going to apply it as yours.

Thanks

Chris

[-- Attachment #2: 4-4-Have-Makefile-import-config.mak-if-it-exists..patch --]
[-- Type: application/octet-stream, Size: 1582 bytes --]

From patchwork Mon Jun  1 18:37:25 2009
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [4/4] Have Makefile import config.mak if it exists.
Date: Mon, 01 Jun 2009 18:37:25 -0000
From: Samuel Bronson <naesten@gmail.com>
X-Patchwork-Id: 27264

This will allow users to override build settings without dirtying their
trees, making life with `git stash' a bit easier.

Signed-off-by: Samuel Bronson <naesten@gmail.com>

---
Makefile |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

Index: sparse.chrisl/Makefile
===================================================================
--- sparse.chrisl.orig/Makefile
+++ sparse.chrisl/Makefile
@@ -10,9 +10,10 @@ LDFLAGS += -g
 AR = ar
 
 #
-# For debugging, uncomment the next one
+# For debugging, put this in local.mk:
+#
+#     CFLAGS += -O0 -DDEBUG -g3 -gdwarf-2
 #
-#CFLAGS += -O0 -DDEBUG -g3 -gdwarf-2
 
 HAVE_LIBXML=$(shell pkg-config --exists libxml-2.0 && echo 'yes')
 HAVE_GCC_DEP=$(shell touch .gcc-test.c && 				\
@@ -89,6 +90,11 @@ SED_PC_CMD = 's|@version@|$(VERSION)|g;	
 	      s|@includedir@|$(INCLUDEDIR)|g'
 
 
+
+# Allow users to override build settings without dirtying their trees
+-include local.mk
+
+
 all: $(PROGRAMS) sparse.pc
 
 install: $(INST_PROGRAMS) $(LIBS) $(LIB_H) sparse.pc
Index: sparse.chrisl/.gitignore
===================================================================
--- sparse.chrisl.orig/.gitignore
+++ sparse.chrisl/.gitignore
@@ -33,3 +33,7 @@ patches-*
 # quilt's files
 patches
 series
+
+# local makefile
+local.mk
+

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

[Christopher Li]

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.

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.

Chris

Re: [PATCH 4/4] Have Makefile import config.mak if it exists.

[Samuel Bronson]

On Tue, Jun 2, 2009 at 4:30 PM, Christopher Li <sparse@chrisli.org> wrote:
> On Mon, Jun 1, 2009 at 11:37 AM, Samuel Bronson <naesten@gmail.com> wrote:
>> This will allow users to override build settings without dirtying their
>> trees, making life with `git stash' a bit easier.
>>
> I modify the patch to use "local.mk" and add it in .gitignore.
>
> If that works for you, I am going to apply it as yours.

Yeah, that's fine.

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

[Samuel Bronson]

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