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