An iterator over attributes, as GCC scalarizes it the code is equivalent

to hand-written code that doesn't use a structure:

attr_iterator ai;
FOR_ATTRS (store, entry, &ai)
  {
    do_something (ai.name, ai.type);
    switch (ai.type)
    {
      case ATTR_STRING:   ai.as_string; break;
      case ATTR_INT:      ai.as_int;    break;
      case ATTR_LOCALIDS: iterate (ai.as_numlist); break;
    }
  }

diff --git a/tools/attr_store.c b/tools/attr_store.c
index 0b11e74..bf27fcd 100644 (file)
--- a/tools/attr_store.c
+++ b/tools/attr_store.c
@@ -516,16 +516,6 @@ attr_store_pack (Attrstore *s)
   s->packed = 1;
 }
 
-#define get_num(ptr,val) do { \
-  typedef int __wrong_buf__[(1-sizeof((ptr)[0])) * (sizeof((ptr)[0])-1)];\
-  val = 0; \
-  while (1) { \
-    unsigned char __c = *((ptr)++); \
-    (val) = ((val) << 7) | (__c & 0x7F); \
-    if ((__c & 0x80) == 0) \
-      break; \
-  } \
-} while (0)
 
 void
 attr_store_unpack (Attrstore *s)
@@ -540,55 +530,32 @@ attr_store_unpack (Attrstore *s)
 
   for (i = 0; i < s->entries; i++)
     {
-      unsigned char *attrs;
-      unsigned int this_abbr;
-      if (!s->ent2attr[i])
-        continue;
-      attrs = s->flat_attrs + s->ent2attr[i];
-      get_num (attrs, this_abbr);
-      unsigned short *abbr = s->flat_abbr + s->abbr[this_abbr];
-      while (*abbr)
+      attr_iterator ai;
+      FOR_ATTRS (s, i, &ai)
         {
-         NameId name = (*abbr) >> 4;
-         switch ((*abbr) & 15)
+         switch (ai.type)
            {
            case ATTR_INT:
-             {
-               int val;
-               get_num (attrs, val);
-               add_attr_int (s, i, name, val); 
-               break;
-             }
+             add_attr_int (s, i, ai.name, ai.as_int); 
+             break;
            case ATTR_ID:
-             {
-               Id val;
-               get_num (attrs, val);
-               add_attr_id (s, i, name, val); 
-               break;
-             }
+             add_attr_id (s, i, ai.name, ai.as_id); 
+             break;
            case ATTR_CHUNK:
-             {
-               unsigned int val1, val2;
-               get_num (attrs, val1);
-               get_num (attrs, val2);
-               add_attr_chunk (s, i, name, val1, val2);
-               break;
-             }
+             add_attr_chunk (s, i, ai.name, ai.as_chunk[0], ai.as_chunk[1]);
+             break;
            case ATTR_STRING:
-             {
-               add_attr_string (s, i, name, (const char*)attrs);
-               attrs += strlen ((const char*)attrs) + 1;
-               break;
-             }
+             add_attr_string (s, i, ai.name, ai.as_string);
+             break;
            case ATTR_INTLIST:
              {
                while (1)
                  {
                    int val;
-                   get_num (attrs, val);
+                   get_num (ai.as_numlist, val);
                    if (!val)
                      break;
-                   add_attr_intlist_int (s, i, name, val);
+                   add_attr_intlist_int (s, i, ai.name, val);
                  }
                break;
              }
@@ -597,17 +564,16 @@ attr_store_unpack (Attrstore *s)
                while (1)
                  {
                    Id val;
-                   get_num (attrs, val);
+                   get_num (ai.as_numlist, val);
                    if (!val)
                      break;
-                   add_attr_localids_id (s, i, name, val);
+                   add_attr_localids_id (s, i, ai.name, val);
                  }
                break;
              }
            default:
              break;
            }
-         abbr++;
        }
     }
 
@@ -891,8 +857,6 @@ attr_store_read (FILE *fp, Pool *pool)
 
   free (buf);
 
-  attr_store_unpack (s);
-
   return s;
 }
 

diff --git a/tools/attr_store.h b/tools/attr_store.h
index 64a669e..d2dccbb 100644 (file)
--- a/tools/attr_store.h
+++ b/tools/attr_store.h
@@ -1,3 +1,6 @@
+#ifndef ATTR_STORE_H
+#define ATTR_STORE_H
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -32,3 +35,5 @@ void add_attr_localids_id (Attrstore *s, unsigned int entry, NameId name, LocalI
 #ifdef __cplusplus
 }
 #endif
+
+#endif

diff --git a/tools/attr_store_p.h b/tools/attr_store_p.h
index 132c609..7afe599 100644 (file)
--- a/tools/attr_store_p.h
+++ b/tools/attr_store_p.h
@@ -1,3 +1,10 @@
+#ifndef ATTR_STORE_P_H
+#define ATTR_STORE_P_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 typedef struct
 {
   NameId name;
@@ -47,3 +54,128 @@ struct _Attrstore
 
   unsigned int packed:1;
 };
+
+#define get_num(ptr,val) do { \
+  typedef int __wrong_buf__[(1-sizeof((ptr)[0])) * (sizeof((ptr)[0])-1)];\
+  val = 0; \
+  while (1) { \
+    unsigned char __c = *((ptr)++); \
+    (val) = ((val) << 7) | (__c & 0x7F); \
+    if ((__c & 0x80) == 0) \
+      break; \
+  } \
+} while (0)
+
+typedef struct {
+  unsigned char *attrs;
+  unsigned short *abbrp;
+  unsigned char *attrs_next;
+  NameId name;
+  unsigned short type;
+
+  /* The following fields could be a union, but if we do that GCC
+     doesn't scalarize these fields anymore, hence real memory accesses
+     would remain, so don't do that.  */
+  int as_int;
+  Id as_id;
+  unsigned int as_chunk[2];
+  const char *as_string;
+  const unsigned char *as_numlist;
+} attr_iterator;
+
+static inline void
+ai_init (Attrstore *s, unsigned int i, attr_iterator *ai)
+{
+  ai->attrs = s->flat_attrs + s->ent2attr[i];
+  unsigned int this_abbr;
+  get_num (ai->attrs, this_abbr);
+  ai->abbrp = s->flat_abbr + s->abbr[this_abbr];
+
+  /* Initialize all fields so we get no uninit warnings later.
+     Don't use memset() to initialize this structure, it would make
+     GCC not scalarize it.  */
+  ai->as_int = 0;
+  ai->as_id = 0;
+  ai->as_chunk[0] = ai->as_chunk[1] = 0;
+  ai->as_string = 0;
+  ai->as_numlist = 0;
+}
+
+static inline int
+ai_step (Attrstore *s, attr_iterator *ai)
+{
+  unsigned short nt = *(ai->abbrp);
+  if (!nt)
+    return 0;
+  ai->name = nt >> 4;
+  ai->type = nt & 0xF;
+  ai->attrs_next = ai->attrs;
+  switch (ai->type)
+    {
+    case ATTR_INT:
+      {
+       int val;
+       get_num (ai->attrs_next, val);
+       ai->as_int = val;
+       break;
+      }
+    case ATTR_ID:
+      {
+       Id val;
+       get_num (ai->attrs_next, val);
+       ai->as_id = val;
+       break;
+      }
+    case ATTR_CHUNK:
+      {
+       unsigned int val1, val2;
+       get_num (ai->attrs_next, val1);
+       ai->as_chunk[0] = val1;
+       get_num (ai->attrs_next, val2);
+       ai->as_chunk[1] = val2;
+       break;
+      }
+    case ATTR_STRING:
+      {
+       ai->as_string = (const char *) ai->attrs_next;
+       ai->attrs_next += strlen ((const char*)ai->attrs_next) + 1;
+       break;
+      }
+    case ATTR_INTLIST:
+      {
+        ai->as_numlist = ai->attrs_next;
+       while (1)
+         {
+           int val;
+           get_num (ai->attrs_next, val);
+           if (!val)
+             break;
+         }
+       break;
+      }
+    case ATTR_LOCALIDS:
+      {
+        ai->as_numlist = ai->attrs_next;
+       while (1)
+         {
+           Id val;
+           get_num (ai->attrs_next, val);
+           if (!val)
+             break;
+         }
+       break;
+      }
+    default:
+      break;
+    }
+  return 1;
+}
+
+#define FOR_ATTRS(s,i,ai) \
+  for (ai_init (s, i, ai); ai_step (s, ai); (ai)->abbrp++,(ai)->attrs = (ai)->attrs_next)
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif

diff --git a/tools/dumpattr.c b/tools/dumpattr.c
index 8ce227c..c394de7 100644 (file)
--- a/tools/dumpattr.c
+++ b/tools/dumpattr.c
@@ -13,50 +13,54 @@
 static void
 dump_attrs (Attrstore *s, unsigned int entry)
 {
-  LongNV *nv = s->attrs[entry];
-  if (nv)
+  attr_iterator ai;
+  FOR_ATTRS (s, entry, &ai)
     {
-      while (nv->name)
-        {
-         fprintf (stdout, "%s:", id2str (s->pool, s->nameids[nv->name]));
-         switch (nv->type)
-           {
-             case ATTR_INT:
-               fprintf (stdout, "int  %u\n", nv->v.i[0]);
-               break;
-             case ATTR_CHUNK:
-               fprintf (stdout, "blob %u+%u\n", nv->v.i[0], nv->v.i[1]);
-               break;
-             case ATTR_STRING:
-               fprintf (stdout, "str  %s\n", nv->v.str);
-               break;
-             case ATTR_ID:
-               fprintf (stdout, "id   %u\n", nv->v.i[0]);
-               break;
-             case ATTR_INTLIST:
-               {
-                 unsigned i = 0;
-                 int val;
-                 fprintf (stdout, "lint\n ");
-                 while ((val = nv->v.intlist[i++]))
-                   fprintf (stdout, " %d", val);
-                 fprintf (stdout, "\n");
+      fprintf (stdout, "%s:", id2str (s->pool, s->nameids[ai.name]));
+      switch (ai.type)
+       {
+       case ATTR_INT:
+         fprintf (stdout, "int  %u\n", ai.as_int);
+         break;
+       case ATTR_ID:
+         fprintf (stdout, "id   %u\n", ai.as_id);
+         break;
+       case ATTR_CHUNK:
+         fprintf (stdout, "blob %u+%u\n", ai.as_chunk[0], ai.as_chunk[1]);
+         break;
+       case ATTR_STRING:
+         fprintf (stdout, "str  %s\n", ai.as_string);
+         break;
+       case ATTR_INTLIST:
+         {
+           fprintf (stdout, "lint\n ");
+           while (1)
+             {
+               int val;
+               get_num (ai.as_numlist, val);
+               if (!val)
                  break;
-               }
-             case ATTR_LOCALIDS:
-               {
-                 unsigned i = 0;
-                 LocalId id;
-                 fprintf (stdout, "lids");
-                 while ((id = nv->v.localids[i++]))
-                   fprintf (stdout, "\n  %s(%d)", localid2str (s, id), id);
-                 fprintf (stdout, "\n");
+               fprintf (stdout, " %d", val);
+             }
+           fprintf (stdout, "\n");
+           break;
+         }
+       case ATTR_LOCALIDS:
+         {
+           fprintf (stdout, "lids");
+           while (1)
+             {
+               Id val;
+               get_num (ai.as_numlist, val);
+               if (!val)
                  break;
-               }
-             default:
-               break;
-           }
-         nv++;
+               fprintf (stdout, "\n  %s(%d)", localid2str (s, val), val);
+             }
+           fprintf (stdout, "\n");
+           break;
+         }
+       default:
+         break;
        }
     }
 }
@@ -68,8 +72,8 @@ main (void)
   Pool *pool = pool_create ();
   Attrstore *s = attr_store_read (stdin, pool);
   /* For now test the packing code.  */
-  attr_store_pack (s);
   attr_store_unpack (s);
+  attr_store_pack (s);
   fprintf (stdout, "attribute store contains %d entities\n", s->entries);
   for (i = 0; i < s->entries; i++)
     {