Re: [Emacs-diffs] trunk r115095: Simplify, port and tune bool vector implementation.

Re: [Emacs-diffs] trunk r115095: Simplify, port and tune bool vector implementation.

[Daniel Colascione]

 >  (bits_word_to_host_endian): Prefer if to #if.  Don't assume
 >  chars are narrower than ints.

We will never run on machines where sizeof(char) == sizeof(int).

-      changed |= adata[i] ^ mword;
+      if (! changed)
+	changed = adata[i] != mword;

That's not an optimization. Branches are expensive on misprediction, and 
this branch will frequently be mispredicted. ALU operations are cheap, 
especially when they're done on registers and don't introduce new data 
hazards. It's not trivial for an optimizer to notice that using 
bitwise-OR here is safe. Please change the code back to the way it was 
before.

+	* data.c (bool_vector_binop_driver): Don't assume bit vectors always
+	contain at least one word.

Zero-length bool vectors are _extremely_ uncommon. Please don't reduce 
efficiency in the general case.

Also, from a previous patch:

 >  (Fbool_vector_count_matches_at): Don't assume CHAR_BIT == 8.

We write "CHAR_BIT" instead of "8" for clarity, not because the number 
of bits per byte might change. In general-purpose machines, CHAR_BIT 
will be 8 and might as well be a physical constant.

        mword >>= offset;
+      mword |= (bits_word) 1 << (BITS_PER_BITS_WORD - offset);
        count = count_trailing_zero_bits (mword);
-      count = min (count, BITS_PER_BITS_WORD - offset);
        pos++;

That's a good change: cutting off the bit count is better than clamping 
the result of the count operatin. Thanks. Can you comment the line you 
added, though? Its purpose isn't immediately obvious.

Re: [Emacs-diffs] trunk r115095: Simplify, port and tune bool vector implementation.

[Paul Eggert]

On 11/17/2013 04:06 AM, Daniel Colascione wrote:

> Can you comment the line you added, though?

Done, in trunk bzr 115139.

> Zero-length bool vectors are _extremely_ uncommon. Please don't
> reduce efficiency in the general case.

Surely any efficiency differences are insignificant here, and it
may well be worth keeping the code simple.  However, it's not that
big a deal either way so I changed it back in trunk bzr 115139.

> -      changed |= adata[i] ^ mword;
> +      if (! changed)
> +    changed = adata[i] != mword;
>
> That's not an optimization.

It was part of an optimization overall, as the surrounding code was changed to
avoid initializing a newly-created bool vector to zeros when it was
going to be immediately overwritten with data, and the above change
was needed to avoid accessing possibly-uninitialized data.

The point is moot now, though, after trunk bzr 115139, where I
optimized away 'changed' entirely.

> We will never run on machines where sizeof(char) == sizeof(int).

True for Emacs's current targets, but when it's easy to write code
portable to those machines (which do exist), we might as well do so,
if only to document what's going on -- sort of the same reason that we
write CHAR_BIT rather than 8.

Re: [Emacs-diffs] trunk r115095: Simplify, port and tune bool vector implementation.

[Glenn Morris]

Paul Eggert wrote:

> Done, in trunk bzr 115139.

This revision seems to cause a segfault during `make check':
http://hydra.nixos.org/build/6840553

Re: trunk r115095: Simplify, port and tune bool vector implementation.

[Andy Moreton]

On Tue 19 Nov 2013, Glenn Morris wrote:

> Paul Eggert wrote:
>
>> Done, in trunk bzr 115139.
>
> This revision seems to cause a segfault during `make check':
> http://hydra.nixos.org/build/6840553

As an aside, the fixes for relative paths and mingw support do not seem
to have been added to tests/automated/Makefile.in. Adding something
similar to the changes to lisp/Makefile.in allowed these test to run
(and report the abort) on a mingw build.

    AndyM

Re: trunk r115095: Simplify, port and tune bool vector implementation.

[Glenn Morris]

Andy Moreton wrote:

> As an aside, the fixes for relative paths and mingw support do not
> seem to have been added to tests/automated/Makefile.in. Adding
> something similar to the changes to lisp/Makefile.in allowed these
> test to run (and report the abort) on a mingw build.

Well, I encourage you to make a bug report (with actual details).

Re: trunk r115095: Simplify, port and tune bool vector implementation.

[Andy Moreton]

On Wed 20 Nov 2013, Glenn Morris wrote:

> Andy Moreton wrote:
>
>> As an aside, the fixes for relative paths and mingw support do not
>> seem to have been added to tests/automated/Makefile.in. Adding
>> something similar to the changes to lisp/Makefile.in allowed these
>> test to run (and report the abort) on a mingw build.
>
> Well, I encourage you to make a bug report (with actual details).

Done - bug#15933.

    AndyM

Re: [Emacs-diffs] trunk r115095: Simplify, port and tune bool vector implementation.

[Paul Eggert]

Glenn Morris wrote:
> This revision seems to cause a segfault during `make check':

Thanks, I reproduced that problem, and fixed it in
trunk bzr 115168.  This removes the optimization of
appending a dummy word to the empty bool vector, but
I'm becoming more inclined to think that the
optimization isn't worth the trouble.