Set operations on bool-vectors

Set operations on bool-vectors

[Daniel Colascione]

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

I've implemented built-in set operations on bool vectors.

Many interesting algorithms boil down to applying set-valued functions
to graphs until the graph no longer changes. The interface below is
designed with this use in mind.  In particular, these functions
simultaneously compute the destination set and whether the destination
set is different from its input.  Operations take a destination bool
vector and allocate memory only if explicitly asked to do so.  These
operations also allow callers to efficiently enumerate all the nil or
t bits in a set.

According to my benchmarks, these operations are about 1,000 times
faster than the compiled elisp equivalents.

=== modified file 'src/alloc.c'
--- src/alloc.c	2013-09-04 22:34:04 +0000
+++ src/alloc.c	2013-09-19 22:24:27 +0000
@@ -2003,6 +2003,29 @@
   return val;
 }

+static
+ptrdiff_t
+bool_vector_payload_bytes (EMACS_INT length)
+{
+  EMACS_INT exact_needed_bytes;
+  EMACS_INT needed_bytes;
+
+  exact_needed_bytes = (length + CHAR_BIT - 1) / CHAR_BIT;
+  needed_bytes =  (exact_needed_bytes + sizeof (size_t) - 1)
+    / sizeof (size_t);
+
+  needed_bytes *= sizeof (size_t);
+
+  if (needed_bytes == 0)
+    {
+      /* Always allocate at least one machine word of payload so that
+         bool-vector operations in data.c don't need a special case
+         for empty vectors.  */
+      needed_bytes = sizeof (size_t);
+    }
+
+  return needed_bytes;
+}

 DEFUN ("make-bool-vector", Fmake_bool_vector, Smake_bool_vector, 2, 2, 0,
        doc: /* Return a new bool-vector of length LENGTH, using INIT for each element.
@@ -2011,37 +2034,39 @@
 {
   register Lisp_Object val;
   struct Lisp_Bool_Vector *p;
-  ptrdiff_t length_in_chars;
-  EMACS_INT length_in_elts;
-  int bits_per_value;
-  int extra_bool_elts = ((bool_header_size - header_size + word_size - 1)
-			 / word_size);
+  EMACS_INT exact_payload_bytes;
+  EMACS_INT total_payload_bytes;
+  EMACS_INT needed_elements;

   CHECK_NATNUM (length);

-  bits_per_value = sizeof (EMACS_INT) * BOOL_VECTOR_BITS_PER_CHAR;
-
-  length_in_elts = (XFASTINT (length) + bits_per_value - 1) / bits_per_value;
-
-  val = Fmake_vector (make_number (length_in_elts + extra_bool_elts), Qnil);
-
-  /* No Lisp_Object to trace in there.  */
+  exact_payload_bytes = (XFASTINT (length) + CHAR_BIT - 1) / CHAR_BIT;
+  total_payload_bytes = bool_vector_payload_bytes (XFASTINT (length));
+
+  eassert (exact_payload_bytes <= total_payload_bytes);
+
+  needed_elements = ((bool_header_size - header_size + total_payload_bytes)
+                     + word_size - 1) / word_size;
+
+  p = (struct Lisp_Bool_Vector* ) allocate_vector(needed_elements);
+  XSETVECTOR (val, p);
   XSETPVECTYPESIZE (XVECTOR (val), PVEC_BOOL_VECTOR, 0, 0);

-  p = XBOOL_VECTOR (val);
   p->size = XFASTINT (length);
-
-  length_in_chars = ((XFASTINT (length) + BOOL_VECTOR_BITS_PER_CHAR - 1)
-		     / BOOL_VECTOR_BITS_PER_CHAR);
-  if (length_in_chars)
+  if (exact_payload_bytes)
     {
-      memset (p->data, ! NILP (init) ? -1 : 0, length_in_chars);
+      memset (p->data, ! NILP (init) ? -1 : 0, exact_payload_bytes);

       /* Clear any extraneous bits in the last byte.  */
-      p->data[length_in_chars - 1]
+      p->data[exact_payload_bytes - 1]
 	&= (1 << ((XFASTINT (length) - 1) % BOOL_VECTOR_BITS_PER_CHAR + 1)) - 1;
     }

+  /* Clear padding at the end.  */
+  memset (p->data + exact_payload_bytes,
+          0,
+          total_payload_bytes - exact_payload_bytes);
+
   return val;
 }

@@ -2785,10 +2810,10 @@
   if (size & PSEUDOVECTOR_FLAG)
     {
       if (PSEUDOVECTOR_TYPEP (&v->header, PVEC_BOOL_VECTOR))
-	size = (bool_header_size
-		+ (((struct Lisp_Bool_Vector *) v)->size
-		   + BOOL_VECTOR_BITS_PER_CHAR - 1)
-		/ BOOL_VECTOR_BITS_PER_CHAR);
+        {
+          struct Lisp_Bool_Vector *bv = (struct Lisp_Bool_Vector *) v;
+          size = bool_header_size + bool_vector_payload_bytes (bv->size);
+        }
       else
 	size = (header_size
 		+ ((size & PSEUDOVECTOR_SIZE_MASK)
@@ -2895,10 +2920,11 @@
 		 pseudovector type grows beyond VBLOCK_BYTES_MAX.  */
 	      eassert (PSEUDOVECTOR_TYPEP (&vector->header, PVEC_BOOL_VECTOR));

-	      total_vector_slots
-		+= (bool_header_size
-		    + ((b->size + BOOL_VECTOR_BITS_PER_CHAR - 1)
-		       / BOOL_VECTOR_BITS_PER_CHAR)) / word_size;
+	      total_vector_slots += (bool_header_size
+                                     + bool_vector_payload_bytes (b->size)
+                                     + word_size
+                                     - 1)
+                / word_size;
 	    }
 	  else
 	    total_vector_slots

=== modified file 'src/data.c'
--- src/data.c	2013-09-11 07:20:20 +0000
+++ src/data.c	2013-09-20 22:31:51 +0000
@@ -54,6 +54,7 @@
 static Lisp_Object Qnatnump;
 Lisp_Object Qstringp, Qarrayp, Qsequencep, Qbufferp;
 Lisp_Object Qchar_or_string_p, Qmarkerp, Qinteger_or_marker_p, Qvectorp;
+Lisp_Object Qbool_vector_p;
 Lisp_Object Qbuffer_or_string_p;
 static Lisp_Object Qkeywordp, Qboundp;
 Lisp_Object Qfboundp;
@@ -2956,6 +2957,384 @@
   return make_number (order);
 }

+/* Because we round up the BOOL_VECTOR allocate size to word_size
+   units, we can safely read past the "end" of the vector in the
+   operations below.  These extra bits are always zero.  Also, we
+   always BOOL_VECTORS with at least one size_t of storage so that we
+   don't have to special-case empty bit vectors.  */
+
+#if (SIZE_MAX >> 32) & 1
+# define BITS_PER_SIZE_T 64
+#else
+# define BITS_PER_SIZE_T 32
+#endif
+
+static inline
+size_t
+bool_vector_spare_mask (EMACS_INT nr_bits)
+{
+  return (((size_t) 1) << (nr_bits % BITS_PER_SIZE_T)) - 1;
+}
+
+static inline
+EMACS_INT
+popcount_size_t(size_t val)
+{
+  EMACS_INT count;
+
+#if defined __GNUC__ && BITS_PER_SIZE_T == 64
+  count = __builtin_popcountll (val);
+#elif defined __GNUC__ && BITS_PER_SIZE_T == 32
+  count = __builtin_popcount (val);
+#elif defined __MSC_VER && BITS_PER_SIZE_T == 64
+# pragma intrinsic __popcnt64
+  count = __popcnt64 (val);
+#elif defined __MSC_VER && BITS_PER_SIZE_T == 32
+# pragma intrinsic __popcnt
+  count = __popcnt (val);
+#else
+  {
+    EMACS_INT j;
+    count = 0;
+    for (j = 0; j < BITS_PER_SIZE_T; ++j)
+      count += !!((((size_t) 1) << j) & val);
+  }
+#endif
+
+  return count;
+}
+
+enum bool_vector_op { bool_vector_exclusive_or,
+                      bool_vector_union,
+                      bool_vector_intersection,
+                      bool_vector_set_difference,
+                      bool_vector_subsetp };
+
+static inline
+Lisp_Object
+bool_vector_binop_driver (Lisp_Object a,
+                          Lisp_Object b,
+                          Lisp_Object c,
+                          enum bool_vector_op op)
+{
+  EMACS_INT nr_bits;
+  size_t *adata, *bdata, *cdata;
+  EMACS_INT i;
+  size_t changed = 0;
+  size_t mword;
+
+  CHECK_BOOL_VECTOR (b);
+  CHECK_BOOL_VECTOR (c);
+
+  nr_bits = min (XBOOL_VECTOR (b)->size,
+                 XBOOL_VECTOR (c)->size);
+
+  if (NILP (a))
+    {
+      a = Fmake_bool_vector (make_number (nr_bits), Qnil);
+      changed = 1;
+    }
+  else
+    {
+      CHECK_BOOL_VECTOR (a);
+      nr_bits = min (nr_bits, XBOOL_VECTOR (a)->size);
+    }
+
+  adata = (size_t*) XBOOL_VECTOR (a)->data;
+  bdata = (size_t*) XBOOL_VECTOR (b)->data;
+  cdata = (size_t*) XBOOL_VECTOR (c)->data;
+  i = 0;
+  do
+    {
+      if (op == bool_vector_exclusive_or)
+        mword = bdata[i] ^ cdata[i];
+      else if (op == bool_vector_union || op == bool_vector_subsetp)
+        mword = bdata[i] | cdata[i];
+      else if (op == bool_vector_intersection)
+        mword = bdata[i] & cdata[i];
+      else if (op == bool_vector_set_difference)
+        mword = bdata[i] &~ cdata[i];
+      else
+        abort ();
+
+      changed |= adata[i] ^ mword;
+
+      if (op != bool_vector_subsetp)
+        adata[i] = mword;
+
+      i += 1;
+    }
+  while (i < (nr_bits + BITS_PER_SIZE_T - 1) / BITS_PER_SIZE_T);
+
+  return changed ? a : Qnil;
+}
+
+/* Compute the number of trailing zero bits in val.  If val is zero,
+   return the number of bits in val.  */
+static inline
+EMACS_INT
+count_trailing_zero_bits (size_t val)
+{
+  if (val == 0)
+    return CHAR_BIT * sizeof (val);
+
+#if __GNUC__ && BITS_PER_SIZE_T == 64
+  return __builtin_ctzll (val);
+#elif __GNUC__ && BITS_PER_SIZE_T == 32
+  return __builtin_ctz (val);
+#elif __MSC_VER && BITS_PER_SIZE_T == 64
+# pragma intrinsic _BitScanForward64
+  {
+    unsigned long result;
+    _BitScanForward64 (&result, val);
+    return (EMACS_INT) result;
+  }
+#elif __MSC_VER && BITS_PER_SIZE_T == 32
+# pragma intrinsic _BitScanForward
+  {
+    unsigned long result;
+    _BitScanForward (&result, val);
+    return (EMACS_INT) result;
+  }
+#else
+  {
+    EMACS_INT count;
+
+    count = 0;
+    for(val = ~val; val & 1; val >>= 1)
+      ++count;
+
+    return count;
+  }
+#endif
+}
+
+static inline
+size_t
+size_t_to_host_endian (size_t val)
+{
+#ifdef WORDS_BIGENDIAN
+# if BITS_PER_SIZE_T == 64
+  return swap64 (val);
+# else
+  return swap32 (val);
+# endif
+#else
+  return val;
+#endif
+}
+
+DEFUN ("bool-vector-exclusive-or", Fbool_vector_exclusive_or,
+       Sbool_vector_exclusive_or, 3, 3, 0,
+       doc: /* Compute A = B ^ C, bitwise exclusive or.
+A, B, and C must be bool vectors.  A is
+any bool vector except C.  If A is nil, allocate a new bool
+vector in which to store the result.
+Return the destination vector if it changed or nil otherwise.  */)
+  (Lisp_Object a, Lisp_Object b, Lisp_Object c)
+{
+  return bool_vector_binop_driver (a, b, c, bool_vector_exclusive_or);
+}
+
+DEFUN ("bool-vector-union", Fbool_vector_union,
+       Sbool_vector_union, 3, 3, 0,
+       doc: /* Compute A = B | C, bitwise or.
+A, B, and C must be bool vectors.  A is
+any bool vector except C.  If A is nil, allocate a new bool
+vector in which to store the result.
+Return the destination vector if it changed and nil otherwise.  */)
+  (Lisp_Object a, Lisp_Object b, Lisp_Object c)
+{
+  return bool_vector_binop_driver (a, b, c, bool_vector_union);
+}
+
+DEFUN ("bool-vector-intersection", Fbool_vector_intersection,
+       Sbool_vector_intersection, 3, 3, 0,
+       doc: /* Compute A = B & C, bitwise and.
+A, B, and C must be bool vectors.  A is
+any bool vector except C.  If A is nil, allocate a new bool
+vector in which to store the result.
+Return the destination vector if it changed and nil otherwise.  */)
+  (Lisp_Object a, Lisp_Object b, Lisp_Object c)
+{
+  return bool_vector_binop_driver (a, b, c, bool_vector_intersection);
+}
+
+DEFUN ("bool-vector-set-difference", Fbool_vector_set_difference,
+       Sbool_vector_set_difference, 3, 3, 0,
+       doc: /* Compute A = B &~ C.
+A, B, and C must be bool vectors.  A is
+any bool vector except C.  If A is nil, allocate a new bool
+vector in which to store the result.
+Return the destination vector if it changed and nil otherwise.  */)
+  (Lisp_Object a, Lisp_Object b, Lisp_Object c)
+{
+  return bool_vector_binop_driver (a, b, c, bool_vector_set_difference);
+}
+
+DEFUN ("bool-vector-subsetp", Fbool_vector_subsetp,
+       Sbool_vector_subsetp, 2, 2, 0,
+       doc: /* Check whether all bits in A are set in B.
+A and B must be bool vectors.  Return a generalize boolean.  */)
+  (Lisp_Object a, Lisp_Object b)
+{
+  /* Like bool_vector_union, but doesn't modify b.  */
+  return bool_vector_binop_driver (b, a, b, bool_vector_subsetp);
+}
+
+DEFUN ("bool-vector-not", Fbool_vector_not,
+       Sbool_vector_not, 2, 2, 0,
+       doc: /* Compute A = ~B.
+A and B must be bool vectors.
+If A is nil, allocate a new bool vector in which to store the result.
+Return the destination vector.  */)
+  (Lisp_Object a, Lisp_Object b)
+{
+  EMACS_INT nr_bits;
+  size_t *adata, *bdata;
+  EMACS_INT i;
+  size_t mword;
+
+  CHECK_BOOL_VECTOR (b);
+  nr_bits = XBOOL_VECTOR (b)->size;
+
+  if (NILP (a))
+    a = Fmake_bool_vector (make_number (nr_bits), Qnil);
+  else
+    {
+      CHECK_BOOL_VECTOR (a);
+      nr_bits = min (nr_bits, XBOOL_VECTOR (a)->size);
+    }
+
+  adata = (size_t*) XBOOL_VECTOR (a)->data;
+  bdata = (size_t*) XBOOL_VECTOR (b)->data;
+  i = 0;
+  do
+    {
+      adata[i] = ~bdata[i];
+      i += 1;
+    }
+  while (i < nr_bits / BITS_PER_SIZE_T);
+
+  if (nr_bits % BITS_PER_SIZE_T)
+    {
+      mword = size_t_to_host_endian (bdata[i]);
+      mword = ~mword;
+      mword &= bool_vector_spare_mask (nr_bits);
+      adata[i] = size_t_to_host_endian (bdata[i]);
+    }
+
+  return a;
+}
+
+DEFUN ("bool-vector-count-matches", Fbool_vector_count_matches,
+       Sbool_vector_count_matches, 2, 2, 0,
+       doc: /* Count how many elements in A equal B.
+A must be a bool vector. B is a generalized bool.  */)
+  (Lisp_Object a, Lisp_Object b)
+{
+  EMACS_INT count;
+  EMACS_INT nr_bits;
+  size_t *adata;
+  size_t match;
+  EMACS_INT i;
+
+  CHECK_BOOL_VECTOR (a);
+
+  nr_bits = XBOOL_VECTOR (a)->size;
+  count = 0;
+  match = NILP (b) ? (size_t) -1 : 0;
+  adata = (size_t*) XBOOL_VECTOR (a)->data;
+
+  for(i = 0; i < nr_bits / BITS_PER_SIZE_T; ++i)
+    count += popcount_size_t (adata[i] ^ match);
+
+  /* Mask out trailing parts of final mword.  */
+  if (nr_bits % BITS_PER_SIZE_T)
+    {
+      size_t mword = adata[i] ^ match;
+      mword = size_t_to_host_endian (mword);
+      count += popcount_size_t (mword & bool_vector_spare_mask (nr_bits));
+    }
+
+  return make_number (count);
+}
+
+DEFUN ("bool-vector-count-matches-at",
+       Fbool_vector_count_matches_at,
+       Sbool_vector_count_matches_at, 3, 3, 0,
+       doc: /* Count how many consecutive elements in A equal B at i.
+A must be a bool vector.  B is a generalized boolean.  i is an
+index into the vector.*/)
+  (Lisp_Object a, Lisp_Object b, Lisp_Object i)
+{
+  EMACS_INT count;
+  EMACS_INT nr_bits;
+  EMACS_INT offset;
+  size_t *adata;
+  size_t twiddle;
+  size_t mword; /* Machine word.  */
+  EMACS_INT pos;
+  EMACS_INT nr_words;
+
+  CHECK_BOOL_VECTOR (a);
+  CHECK_NATNUM (i);
+
+  nr_bits = XBOOL_VECTOR (a)->size;
+  if (XFASTINT (i) > nr_bits) /* Allow one past the end for convenience */
+    args_out_of_range (a, i);
+
+  adata = (size_t*) XBOOL_VECTOR (a)->data;
+  nr_words = (nr_bits + BITS_PER_SIZE_T - 1) / BITS_PER_SIZE_T;
+  pos = XFASTINT (i) / BITS_PER_SIZE_T;
+  offset = XFASTINT (i) % BITS_PER_SIZE_T;
+  count = 0;
+
+  /* By XORing with twiddle, we transform the problem of "count
+     consecutive equal values" into "count the zero bits".  The latter
+     operation usually has hardware support.  */
+  twiddle = NILP (b) ? 0 : (size_t) -1;
+
+  /* Scan the remainder of the mword at the current offset.  */
+  if (pos < nr_words && offset != 0)
+    {
+      mword = size_t_to_host_endian (adata[pos]);
+      mword ^= twiddle;
+      mword >>= offset;
+      count = count_trailing_zero_bits (mword);
+      count = min (count, BITS_PER_SIZE_T - offset);
+      pos += 1;
+      if (count + offset < BITS_PER_SIZE_T)
+        return make_number (count);
+    }
+
+  /* Scan whole words until we either reach the end of the vector or
+     find an mword that doesn't completely match.  twiddle is
+     endian-independent.  */
+  while (pos < nr_words && adata[pos] == twiddle)
+    {
+      count += BITS_PER_SIZE_T;
+      ++pos;
+    }
+
+  if (pos < nr_words)
+    {
+      /* If we stopped because of a mismatch, see how many bits match
+         in the current mword.  */
+      mword = size_t_to_host_endian (adata[pos]);
+      mword ^= twiddle;
+      count += count_trailing_zero_bits (mword);
+    }
+  else if (nr_bits % BITS_PER_SIZE_T != 0)
+    {
+      /* If we hit the end, we might have overshot our count.  Reduce
+         the total by the number of spare bits at the end of the
+         vector.  */
+      count -= BITS_PER_SIZE_T - nr_bits % BITS_PER_SIZE_T;
+    }
+
+  return make_number (count);
+}

 \f
 void
@@ -3005,6 +3384,7 @@
   DEFSYM (Qsequencep, "sequencep");
   DEFSYM (Qbufferp, "bufferp");
   DEFSYM (Qvectorp, "vectorp");
+  DEFSYM (Qbool_vector_p, "bool-vector-p");
   DEFSYM (Qchar_or_string_p, "char-or-string-p");
   DEFSYM (Qmarkerp, "markerp");
   DEFSYM (Qbuffer_or_string_p, "buffer-or-string-p");
@@ -3222,6 +3602,15 @@
   defsubr (&Ssubr_arity);
   defsubr (&Ssubr_name);

+  defsubr (&Sbool_vector_exclusive_or);
+  defsubr (&Sbool_vector_union);
+  defsubr (&Sbool_vector_intersection);
+  defsubr (&Sbool_vector_set_difference);
+  defsubr (&Sbool_vector_not);
+  defsubr (&Sbool_vector_subsetp);
+  defsubr (&Sbool_vector_count_matches);
+  defsubr (&Sbool_vector_count_matches_at);
+
   set_symbol_function (Qwholenump, XSYMBOL (Qnatnump)->function);

   DEFVAR_LISP ("most-positive-fixnum", Vmost_positive_fixnum,

=== modified file 'src/lisp.h'
--- src/lisp.h	2013-09-11 05:03:23 +0000
+++ src/lisp.h	2013-09-20 22:52:47 +0000
@@ -733,6 +733,7 @@
 extern Lisp_Object Qarrayp, Qbufferp, Qbuffer_or_string_p, Qchar_table_p;
 extern Lisp_Object Qconsp, Qfloatp, Qintegerp, Qlambda, Qlistp, Qmarkerp, Qnil;
 extern Lisp_Object Qnumberp, Qstringp, Qsymbolp, Qvectorp;
+extern Lisp_Object Qbool_vector_p;
 extern Lisp_Object Qvector_or_char_table_p, Qwholenump;
 extern Lisp_Object Qwindow;
 extern Lisp_Object Ffboundp (Lisp_Object);
@@ -2362,6 +2363,11 @@
   CHECK_TYPE (VECTORP (x), Qvectorp, x);
 }
 LISP_INLINE void
+CHECK_BOOL_VECTOR (Lisp_Object x)
+{
+  CHECK_TYPE (BOOL_VECTOR_P (x), Qbool_vector_p, x);
+}
+LISP_INLINE void
 CHECK_VECTOR_OR_STRING (Lisp_Object x)
 {
   CHECK_TYPE (VECTORP (x) || STRINGP (x), Qarrayp, x);
@@ -4350,6 +4356,33 @@
     return 0;
 }

+LISP_INLINE
+uint16_t
+swap16 (uint16_t val)
+{
+    return (val << 8) | (val & 0xFF);
+}
+
+LISP_INLINE
+uint32_t
+swap32 (uint32_t val)
+{
+  uint32_t low = swap16 (val & 0xFFFF);
+  uint32_t high = swap16 (val >> 16);
+  return (low << 16) | high;
+}
+
+#ifdef UINT64_MAX
+LISP_INLINE
+uint64_t
+swap64 (uint64_t val)
+{
+  uint64_t low = swap32 (val & 0xFFFFFFFF);
+  uint64_t high = swap32 (val >> 32);
+  return (low << 32) | high;
+}
+#endif
+
 INLINE_HEADER_END

 #endif /* EMACS_LISP_H */

=== modified file 'src/xsettings.c'
--- src/xsettings.c	2013-09-17 07:06:42 +0000
+++ src/xsettings.c	2013-09-19 22:24:27 +0000
@@ -336,9 +336,6 @@
   XUngrabServer (dpy);
 }

-#define SWAP32(nr) (((nr) << 24) | (((nr) << 8) & 0xff0000)     \
-                    | (((nr) >> 8) & 0xff00) | ((nr) >> 24))
-#define SWAP16(nr) (((nr) << 8) | ((nr) >> 8))
 #define PAD(nr)    (((nr) + 3) & ~3)

 /* Parse xsettings and extract those that deal with Xft.
@@ -408,7 +405,7 @@

   if (bytes < 12) return BadLength;
   memcpy (&n_settings, prop+8, 4);
-  if (my_bo != that_bo) n_settings = SWAP32 (n_settings);
+  if (my_bo != that_bo) n_settings = swap32 (n_settings);
   bytes_parsed = 12;

   memset (settings, 0, sizeof (*settings));
@@ -430,7 +427,7 @@

       memcpy (&nlen, prop+bytes_parsed, 2);
       bytes_parsed += 2;
-      if (my_bo != that_bo) nlen = SWAP16 (nlen);
+      if (my_bo != that_bo) nlen = swap16 (nlen);
       if (bytes_parsed+nlen > bytes) return BadLength;
       to_cpy = nlen > 127 ? 127 : nlen;
       memcpy (name, prop+bytes_parsed, to_cpy);
@@ -457,7 +454,7 @@
           if (want_this)
             {
               memcpy (&ival, prop+bytes_parsed, 4);
-              if (my_bo != that_bo) ival = SWAP32 (ival);
+              if (my_bo != that_bo) ival = swap32 (ival);
             }
           bytes_parsed += 4;
           break;
@@ -466,7 +463,7 @@
           if (bytes_parsed+4 > bytes) return BadLength;
           memcpy (&vlen, prop+bytes_parsed, 4);
           bytes_parsed += 4;
-          if (my_bo != that_bo) vlen = SWAP32 (vlen);
+          if (my_bo != that_bo) vlen = swap32 (vlen);
           if (want_this)
             {
               to_cpy = vlen > 127 ? 127 : vlen;

=== modified file 'test/automated/data-tests.el'
--- test/automated/data-tests.el	2013-09-11 05:03:23 +0000
+++ test/automated/data-tests.el	2013-09-20 22:46:42 +0000
@@ -21,6 +21,9 @@

 ;;; Code:

+(require 'cl-lib)
+(eval-when-compile (require 'cl))
+
 (ert-deftest data-tests-= ()
   (should-error (=))
   (should (= 1))
@@ -71,5 +74,166 @@
   ;; Short circuits before getting to bad arg
   (should-not (>= 8 9 'foo)))

+;; Bool vector tests.  Compactly represent bool vectors as hex
+;; strings.
+
+(ert-deftest bool-vector-count-matches-all-0-nil ()
+  (cl-loop for sz in '(0 45 1 64 9 344)
+           do (let* ((bv (make-bool-vector sz nil)))
+                (should
+                 (eql
+                  (bool-vector-count-matches bv nil)
+                  sz)))))
+
+(ert-deftest bool-vector-count-matches-all-0-t ()
+  (cl-loop for sz in '(0 45 1 64 9 344)
+           do (let* ((bv (make-bool-vector sz nil)))
+                (should
+                 (eql
+                  (bool-vector-count-matches bv t)
+                  0)))))
+
+(ert-deftest bool-vector-count-matches-1-nil ()
+  (let* ((bv (make-bool-vector 45 nil)))
+    (aset bv 40 t)
+    (aset bv 0 t)
+    (should
+     (eql
+      (bool-vector-count-matches bv t)
+      2)))
+  )
+
+(ert-deftest bool-vector-count-matches-1-t ()
+  (let* ((bv (make-bool-vector 45 nil)))
+    (aset bv 40 t)
+    (aset bv 0 t)
+    (should
+     (eql
+      (bool-vector-count-matches bv nil)
+      43))))
+
+(defun mock-bool-vector-count-matches-at (a b i)
+  (loop for i from i below (length a)
+        while (eq (aref a i) b)
+        sum 1))
+
+(defun test-bool-vector-bv-from-hex-string (desc)
+  (let (bv nchars nibbles)
+    (dolist (c (string-to-list desc))
+      (push (string-to-number
+             (char-to-string c)
+             16)
+            nibbles))
+    (setf bv (make-bool-vector (* 4 (length nibbles)) nil))
+    (let ((i 0))
+      (dolist (n (nreverse nibbles))
+        (dotimes (_ 4)
+          (aset bv i (> (logand 1 n) 0))
+          (incf i)
+          (setf n (lsh n -1)))))
+    bv))
+
+(defun test-bool-vector-to-hex-string (bv)
+  (let (nibbles (v (cl-coerce bv 'list)))
+    (while v
+      (push (logior
+             (lsh (if (nth 0 v) 1 0) 0)
+             (lsh (if (nth 1 v) 1 0) 1)
+             (lsh (if (nth 2 v) 1 0) 2)
+             (lsh (if (nth 3 v) 1 0) 3))
+            nibbles)
+      (setf v (nthcdr 4 v)))
+    (mapconcat (lambda (n) (format "%X" n))
+               (nreverse nibbles)
+               "")))
+
+(defun test-bool-vector-count-matches-at-tc (desc)
+  "Run a test case for bool-vector-count-matches-at.
+DESC is a string describing the test.  It is a sequence of
+hexadecimal digits describing the bool vector.  We exhaustively
+test all counts at all possible positions in the vector by
+comparing the subr with a much slower lisp implementation."
+  (let ((bv (test-bool-vector-bv-from-hex-string desc)))
+    (loop
+     for lf in '(nil t)
+     do (loop
+         for pos from 0 upto (length bv)
+         for cnt = (mock-bool-vector-count-matches-at bv lf pos)
+         for rcnt = (bool-vector-count-matches-at bv lf pos)
+         unless (eql cnt rcnt)
+         do (error "FAILED testcase %S %3S %3S %3S"
+                   pos lf cnt rcnt)))))
+
+(defconst bool-vector-test-vectors
+'(""
+  "0"
+  "F"
+  "0F"
+  "F0"
+  "00000000000000000000000000000FFFFF0000000"
+  "44a50234053fba3340000023444a50234053fba33400000234"
+  "12341234123456123412346001234123412345612341234600"
+  "44a50234053fba33400000234"
+  "1234123412345612341234600"
+  "44a50234053fba33400000234"
+  "1234123412345612341234600"
+  "44a502340"
+  "123412341"
+  "0000000000000000000000000"
+  "FFFFFFFFFFFFFFFF1"))
+
+(ert-deftest bool-vector-count-matches-at ()
+  (mapc #'test-bool-vector-count-matches-at-tc
+        bool-vector-test-vectors))
+
+(defun test-bool-vector-apply-mock-op (mock a b c)
+  "Compute (slowly) the correct result of a bool-vector set operation."
+  (let (changed nv)
+    (assert (eql (length b) (length c)))
+    (if a (setf nv a)
+      (setf a (make-bool-vector (length b) nil))
+      (setf changed t))
+
+    (loop for i below (length b)
+          for mockr = (funcall mock
+                               (if (aref b i) 1 0)
+                               (if (aref c i) 1 0))
+          for r = (not (= 0 mockr))
+          do (progn
+               (unless (eq (aref a i) r)
+                 (setf changed t))
+               (setf (aref a i) r)))
+    (if changed a)))
+
+(defun test-bool-vector-binop (mock real)
+  "Test a binary set operation."
+  (loop for s1 in bool-vector-test-vectors
+        for bv1 = (test-bool-vector-bv-from-hex-string s1)
+        for vecs2 = (cl-remove-if-not
+                     (lambda (x) (eql (length x) (length s1)))
+                     bool-vector-test-vectors)
+        do (loop for s2 in vecs2
+                 for bv2 = (test-bool-vector-bv-from-hex-string s2)
+                 for mock-result = (test-bool-vector-apply-mock-op
+                                    mock nil bv1 bv2)
+                 for real-result = (funcall real nil bv1 bv2)
+                 do (progn
+                      (should (equal mock-result real-result))))))
+
+(ert-deftest bool-vector-intersection-op ()
+  (test-bool-vector-binop
+   #'logand
+   #'bool-vector-intersection))
+
+(ert-deftest bool-vector-union-op ()
+  (test-bool-vector-binop
+   #'logior
+   #'bool-vector-union))
+
+(ert-deftest bool-vector-xor-op ()
+  (test-bool-vector-binop
+   #'logxor
+   #'bool-vector-exclusive-or))
+
+
 ;;; data-tests.el ends here
-




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Set operations on bool-vectors

[Stefan Monnier]

> I've implemented built-in set operations on bool vectors.

Thanks.  Looks good.

> According to my benchmarks, these operations are about 1,000 times
> faster than the compiled elisp equivalents.

I'd expect no less.

> === modified file 'src/alloc.c'
> --- src/alloc.c	2013-09-04 22:34:04 +0000
> +++ src/alloc.c	2013-09-19 22:24:27 +0000
> @@ -2003,6 +2003,29 @@
>    return val;
>  }

Could you describe the intention of the changes in alloc.c (basically,
provide ChangeLog entries)?

> +       doc: /* Compute A = B ^ C, bitwise exclusive or.

Why not make it C = A ^ B and then make C optional?
Same for other similar functions: make the destination argument optional.


        Stefan

Re: Set operations on bool-vectors

[Dmitry Antipov]

On 09/21/2013 02:59 AM, Daniel Colascione wrote:

> I've implemented built-in set operations on bool vectors.

[...]

> +/* Because we round up the BOOL_VECTOR allocate size to word_size
> +   units, we can safely read past the "end" of the vector in the
> +   operations below.  These extra bits are always zero.  Also, we
> +   always BOOL_VECTORS with at least one size_t of storage so that we
> +   don't have to special-case empty bit vectors.  */
> +
> +#if (SIZE_MAX >> 32) & 1
> +# define BITS_PER_SIZE_T 64
> +#else
> +# define BITS_PER_SIZE_T 32
> +#endif

IIUC this should go to the well-known place in lisp.h.

> +static inline
> +EMACS_INT
> +popcount_size_t(size_t val)
> +{
> +  EMACS_INT count;
> +
> +#if defined __GNUC__ && BITS_PER_SIZE_T == 64
> +  count = __builtin_popcountll (val);
> +#elif defined __GNUC__ && BITS_PER_SIZE_T == 32
> +  count = __builtin_popcount (val);
> +#elif defined __MSC_VER && BITS_PER_SIZE_T == 64
> +# pragma intrinsic __popcnt64
> +  count = __popcnt64 (val);
> +#elif defined __MSC_VER && BITS_PER_SIZE_T == 32
> +# pragma intrinsic __popcnt
> +  count = __popcnt (val);
> +#else
> +  {
> +    EMACS_INT j;
> +    count = 0;
> +    for (j = 0; j < BITS_PER_SIZE_T; ++j)
> +      count += !!((((size_t) 1) << j) & val);
> +  }
> +#endif

Why loop? See http://en.wikipedia.org/wiki/Hamming_weight.

Dmitry

Re: Set operations on bool-vectors

[Daniel Colascione]

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

On 9/20/13 6:57 PM, Stefan Monnier wrote:
>> === modified file 'src/alloc.c'
>> --- src/alloc.c	2013-09-04 22:34:04 +0000
>> +++ src/alloc.c	2013-09-19 22:24:27 +0000
>> @@ -2003,6 +2003,29 @@
>>    return val;
>>  }
> 
> Could you describe the intention of the changes in alloc.c (basically,
> provide ChangeLog entries)?

Sure.  Something like this?

2013-09-21  Daniel Colascione  <dancol@dancol.org>

	* alloc.c (bool_vector_payload_bytes): New function: computes
	rounded-up payload size for a bool vector.  Always allocate
	at least size_t bytes even for a zero-size vector.
	(Fmake_bool_vector): Instead of calling Fmake_vector,
	which performs redundant initialization and argument checking,
	just call allocate_vector ourselves.  Make sure we clear any
	terminating padding to zero.
	(vector_nbytes,sweep_vectors): Use bool_vector_payload_bytes
	instead of open-coding the size calculation.

>> +       doc: /* Compute A = B ^ C, bitwise exclusive or.
> 
> Why not make it C = A ^ B and then make C optional?
> Same for other similar functions: make the destination argument optional.

Then we wouldn't be able to extend these functions to accept more than
three arguments: we wouldn't know whether the argument at the end was
another operand or a destination.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Set operations on bool-vectors

[Daniel Colascione]

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

On 9/20/13 7:26 PM, Dmitry Antipov wrote:
> On 09/21/2013 02:59 AM, Daniel Colascione wrote:
> 
>> I've implemented built-in set operations on bool vectors.
> 
> [...]
> 
>> +/* Because we round up the BOOL_VECTOR allocate size to word_size
>> +   units, we can safely read past the "end" of the vector in the
>> +   operations below.  These extra bits are always zero.  Also, we
>> +   always BOOL_VECTORS with at least one size_t of storage so that we
>> +   don't have to special-case empty bit vectors.  */
>> +
>> +#if (SIZE_MAX >> 32) & 1
>> +# define BITS_PER_SIZE_T 64
>> +#else
>> +# define BITS_PER_SIZE_T 32
>> +#endif
> 
> IIUC this should go to the well-known place in lisp.h.

Sure.

>> +static inline
>> +EMACS_INT
>> +popcount_size_t(size_t val)
>> +{
>> +  EMACS_INT count;
>> +
>> +#if defined __GNUC__ && BITS_PER_SIZE_T == 64
>> +  count = __builtin_popcountll (val);
>> +#elif defined __GNUC__ && BITS_PER_SIZE_T == 32
>> +  count = __builtin_popcount (val);
>> +#elif defined __MSC_VER && BITS_PER_SIZE_T == 64
>> +# pragma intrinsic __popcnt64
>> +  count = __popcnt64 (val);
>> +#elif defined __MSC_VER && BITS_PER_SIZE_T == 32
>> +# pragma intrinsic __popcnt
>> +  count = __popcnt (val);
>> +#else
>> +  {
>> +    EMACS_INT j;
>> +    count = 0;
>> +    for (j = 0; j < BITS_PER_SIZE_T; ++j)
>> +      count += !!((((size_t) 1) << j) & val);
>> +  }
>> +#endif
> 
> Why loop? See http://en.wikipedia.org/wiki/Hamming_weight.

I didn't want to put a lot of effort into a code path we'll probably
never use.  Recall that if we're using icc or gcc or Visual C++ or
Clang, we'll be using a compiler intrinsic, which will probably compile
down to a single machine instruction.

By the way: can someone test that the Visual C++ alternate actually
works? I don't have access to a Windows machine at the moment.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Set operations on bool-vectors

[Eli Zaretskii]

> Date: Fri, 20 Sep 2013 19:49:00 -0700
> From: Daniel Colascione <dancol@dancol.org>
> Cc: Emacs development discussions <emacs-devel@gnu.org>
> 
> >> +static inline
> >> +EMACS_INT
> >> +popcount_size_t(size_t val)
> >> +{
> >> +  EMACS_INT count;
> >> +
> >> +#if defined __GNUC__ && BITS_PER_SIZE_T == 64
> >> +  count = __builtin_popcountll (val);
> >> +#elif defined __GNUC__ && BITS_PER_SIZE_T == 32
> >> +  count = __builtin_popcount (val);
> >> +#elif defined __MSC_VER && BITS_PER_SIZE_T == 64
> >> +# pragma intrinsic __popcnt64
> >> +  count = __popcnt64 (val);
> >> +#elif defined __MSC_VER && BITS_PER_SIZE_T == 32
> >> +# pragma intrinsic __popcnt
> >> +  count = __popcnt (val);
> >> +#else
> >> +  {
> >> +    EMACS_INT j;
> >> +    count = 0;
> >> +    for (j = 0; j < BITS_PER_SIZE_T; ++j)
> >> +      count += !!((((size_t) 1) << j) & val);
> >> +  }
> >> +#endif
> > 
> > Why loop? See http://en.wikipedia.org/wiki/Hamming_weight.
> 
> I didn't want to put a lot of effort into a code path we'll probably
> never use.  Recall that if we're using icc or gcc or Visual C++ or
> Clang, we'll be using a compiler intrinsic, which will probably compile
> down to a single machine instruction.
> 
> By the way: can someone test that the Visual C++ alternate actually
> works? I don't have access to a Windows machine at the moment.

I don't see why it won't work, per documentation on this page:

  http://msdn.microsoft.com/en-us/library/bb385231%28v=vs.90%29.aspx

However, I think you will need to make usage of these intrinsics
compiler version dependent.  GCC supports them starting from 3.4,
whereas MSVC seems to support them since Studio 2008, i.e. _MSC_VER =
1500 or higher.

It is also not clear to me what will the MSVC intrinsic do if the
binary ever runs on a CPU that doesn't support SSE4, the MSDN
documentation seems to say that the results are unpredictable,
i.e. that there's no fallback, like GCC has in libgcc.  So perhaps we
should also guard that with a Windows version (assuming that old
machines will only ever run Windows 9x).

Re: Set operations on bool-vectors

[Andreas Schwab]

Daniel Colascione <dancol@dancol.org> writes:

> +static
> +ptrdiff_t
> +bool_vector_payload_bytes (EMACS_INT length)
> +{
> +  EMACS_INT exact_needed_bytes;
> +  EMACS_INT needed_bytes;
> +
> +  exact_needed_bytes = (length + CHAR_BIT - 1) / CHAR_BIT;
> +  needed_bytes =  (exact_needed_bytes + sizeof (size_t) - 1)
> +    / sizeof (size_t);
> +
> +  needed_bytes *= sizeof (size_t);

Please check for overflow.  Also, the return type should be EMACS_INT,
not ptrdiff_t.

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."

Re: Set operations on bool-vectors

[Daniel Colascione]

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

On 9/21/13 12:36 AM, Andreas Schwab wrote:
> Daniel Colascione <dancol@dancol.org> writes:
> 
>> +static
>> +ptrdiff_t
>> +bool_vector_payload_bytes (EMACS_INT length)
>> +{
>> +  EMACS_INT exact_needed_bytes;
>> +  EMACS_INT needed_bytes;
>> +
>> +  exact_needed_bytes = (length + CHAR_BIT - 1) / CHAR_BIT;
>> +  needed_bytes =  (exact_needed_bytes + sizeof (size_t) - 1)
>> +    / sizeof (size_t);
>> +
>> +  needed_bytes *= sizeof (size_t);
> 
> Please check for overflow. 

It can't overflow.  The initial number of bits comes from a Lisp
integer, which has a bit less range than an int.  What exactly is the
routine supposed to do if the value overflows?

> Also, the return type should be EMACS_INT,
> not ptrdiff_t.

Well, vector_nbytes works with ptrdiff_t, and the allocation code works
in EMACS_INT.  I'm not sure the distinction actually matters in this case.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Set operations on bool-vectors

[Daniel Colascione]

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

On 9/21/13 12:16 AM, Eli Zaretskii wrote:
>> Date: Fri, 20 Sep 2013 19:49:00 -0700
>> From: Daniel Colascione <dancol@dancol.org>
>> Cc: Emacs development discussions <emacs-devel@gnu.org>
>>
>>>> +static inline
>>>> +EMACS_INT
>>>> +popcount_size_t(size_t val)
>>>> +{
>>>> +  EMACS_INT count;
>>>> +
>>>> +#if defined __GNUC__ && BITS_PER_SIZE_T == 64
>>>> +  count = __builtin_popcountll (val);
>>>> +#elif defined __GNUC__ && BITS_PER_SIZE_T == 32
>>>> +  count = __builtin_popcount (val);
>>>> +#elif defined __MSC_VER && BITS_PER_SIZE_T == 64
>>>> +# pragma intrinsic __popcnt64
>>>> +  count = __popcnt64 (val);
>>>> +#elif defined __MSC_VER && BITS_PER_SIZE_T == 32
>>>> +# pragma intrinsic __popcnt
>>>> +  count = __popcnt (val);
>>>> +#else
>>>> +  {
>>>> +    EMACS_INT j;
>>>> +    count = 0;
>>>> +    for (j = 0; j < BITS_PER_SIZE_T; ++j)
>>>> +      count += !!((((size_t) 1) << j) & val);
>>>> +  }
>>>> +#endif
>>>
>>> Why loop? See http://en.wikipedia.org/wiki/Hamming_weight.
>>
>> I didn't want to put a lot of effort into a code path we'll probably
>> never use.  Recall that if we're using icc or gcc or Visual C++ or
>> Clang, we'll be using a compiler intrinsic, which will probably compile
>> down to a single machine instruction.
>>
>> By the way: can someone test that the Visual C++ alternate actually
>> works? I don't have access to a Windows machine at the moment.
> 
> I don't see why it won't work, per documentation on this page:
> 
>   http://msdn.microsoft.com/en-us/library/bb385231%28v=vs.90%29.aspx
> 
> However, I think you will need to make usage of these intrinsics
> compiler version dependent.  GCC supports them starting from 3.4,
> whereas MSVC seems to support them since Studio 2008, i.e. _MSC_VER =
> 1500 or higher.

Fair enough.

> It is also not clear to me what will the MSVC intrinsic do if the
> binary ever runs on a CPU that doesn't support SSE4, the MSDN
> documentation seems to say that the results are unpredictable,
> i.e. that there's no fallback, like GCC has in libgcc.  So perhaps we
> should also guard that with a Windows version (assuming that old
> machines will only ever run Windows 9x).

Good point. SSE4 is much too recent to require. Making the cpuid check
shouldn't be too hard. I have no way to actually test the fallback,
though. (It's easy to test the fallback code, but not that easy to test
falling back to it.)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Set operations on bool-vectors

[Andreas Schwab]

Daniel Colascione <dancol@dancol.org> writes:

> On 9/21/13 12:36 AM, Andreas Schwab wrote:
>> Also, the return type should be EMACS_INT,
>> not ptrdiff_t.
>
> Well, vector_nbytes works with ptrdiff_t, and the allocation code works
> in EMACS_INT.  I'm not sure the distinction actually matters in this case.

ptrdiff_t may be smaller than EMACS_INT.

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."

Re: Set operations on bool-vectors

[Daniel Colascione]

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

On 9/21/13 1:35 AM, Andreas Schwab wrote:
> Daniel Colascione <dancol@dancol.org> writes:
> 
>> On 9/21/13 12:36 AM, Andreas Schwab wrote:
>>> Also, the return type should be EMACS_INT,
>>> not ptrdiff_t.
>>
>> Well, vector_nbytes works with ptrdiff_t, and the allocation code works
>> in EMACS_INT.  I'm not sure the distinction actually matters in this case.
> 
> ptrdiff_t may be smaller than EMACS_INT.

We'll never overflow when we look at an already-allocated object --- but
we _can_ overflow in the initial allocation calculation. I'll fix it.
Thanks.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Set operations on bool-vectors

[Stefan Monnier]

> 2013-09-21  Daniel Colascione  <dancol@dancol.org>

> 	* alloc.c (bool_vector_payload_bytes): New function: computes
> 	rounded-up payload size for a bool vector.  Always allocate
> 	at least size_t bytes even for a zero-size vector.

That should just be "New function" and the rest should be moved to
a comment in the C code.

> 	(Fmake_bool_vector): Instead of calling Fmake_vector,
> 	which performs redundant initialization and argument checking,
> 	just call allocate_vector ourselves.  Make sure we clear any
> 	terminating padding to zero.

Good.  So the functional change is to set the padding to zero.

> 	(vector_nbytes,sweep_vectors): Use bool_vector_payload_bytes
> 	instead of open-coding the size calculation.

OK that makes sense, thank you.

>>> +       doc: /* Compute A = B ^ C, bitwise exclusive or.
>> Why not make it C = A ^ B and then make C optional?
>> Same for other similar functions: make the destination argument optional.
> Then we wouldn't be able to extend these functions to accept more than
> three arguments: we wouldn't know whether the argument at the end was
> another operand or a destination.

Sounds like a minor potential inconvenience in exchange for a much
cleaner interface.


        Stefan