* Re: [Emacs-diffs] trunk r115095: Simplify, port and tune bool vector implementation.
[not found] <E1VgmqD-0001Ps-LA@vcs.savannah.gnu.org>
@ 2013-11-17 12:06 ` Daniel Colascione
2013-11-18 18:45 ` Paul Eggert
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Colascione @ 2013-11-17 12:06 UTC (permalink / raw)
To: Paul Eggert, emacs-devel
> (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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Emacs-diffs] trunk r115095: Simplify, port and tune bool vector implementation.
2013-11-17 12:06 ` [Emacs-diffs] trunk r115095: Simplify, port and tune bool vector implementation Daniel Colascione
@ 2013-11-18 18:45 ` Paul Eggert
2013-11-19 18:51 ` Glenn Morris
0 siblings, 1 reply; 7+ messages in thread
From: Paul Eggert @ 2013-11-18 18:45 UTC (permalink / raw)
To: Daniel Colascione, emacs-devel
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Emacs-diffs] trunk r115095: Simplify, port and tune bool vector implementation.
2013-11-18 18:45 ` Paul Eggert
@ 2013-11-19 18:51 ` Glenn Morris
2013-11-19 21:05 ` Andy Moreton
2013-11-21 6:50 ` [Emacs-diffs] " Paul Eggert
0 siblings, 2 replies; 7+ messages in thread
From: Glenn Morris @ 2013-11-19 18:51 UTC (permalink / raw)
To: Paul Eggert; +Cc: Daniel Colascione, emacs-devel
Paul Eggert wrote:
> Done, in trunk bzr 115139.
This revision seems to cause a segfault during `make check':
http://hydra.nixos.org/build/6840553
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: trunk r115095: Simplify, port and tune bool vector implementation.
2013-11-19 18:51 ` Glenn Morris
@ 2013-11-19 21:05 ` Andy Moreton
2013-11-20 1:52 ` Glenn Morris
2013-11-21 6:50 ` [Emacs-diffs] " Paul Eggert
1 sibling, 1 reply; 7+ messages in thread
From: Andy Moreton @ 2013-11-19 21:05 UTC (permalink / raw)
To: emacs-devel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: trunk r115095: Simplify, port and tune bool vector implementation.
2013-11-19 21:05 ` Andy Moreton
@ 2013-11-20 1:52 ` Glenn Morris
2013-11-20 13:00 ` Andy Moreton
0 siblings, 1 reply; 7+ messages in thread
From: Glenn Morris @ 2013-11-20 1:52 UTC (permalink / raw)
To: Andy Moreton; +Cc: emacs-devel
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).
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: trunk r115095: Simplify, port and tune bool vector implementation.
2013-11-20 1:52 ` Glenn Morris
@ 2013-11-20 13:00 ` Andy Moreton
0 siblings, 0 replies; 7+ messages in thread
From: Andy Moreton @ 2013-11-20 13:00 UTC (permalink / raw)
To: emacs-devel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Emacs-diffs] trunk r115095: Simplify, port and tune bool vector implementation.
2013-11-19 18:51 ` Glenn Morris
2013-11-19 21:05 ` Andy Moreton
@ 2013-11-21 6:50 ` Paul Eggert
1 sibling, 0 replies; 7+ messages in thread
From: Paul Eggert @ 2013-11-21 6:50 UTC (permalink / raw)
To: Glenn Morris; +Cc: Daniel Colascione, emacs-devel
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-11-21 6:50 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <E1VgmqD-0001Ps-LA@vcs.savannah.gnu.org>
2013-11-17 12:06 ` [Emacs-diffs] trunk r115095: Simplify, port and tune bool vector implementation Daniel Colascione
2013-11-18 18:45 ` Paul Eggert
2013-11-19 18:51 ` Glenn Morris
2013-11-19 21:05 ` Andy Moreton
2013-11-20 1:52 ` Glenn Morris
2013-11-20 13:00 ` Andy Moreton
2013-11-21 6:50 ` [Emacs-diffs] " Paul Eggert
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.