[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Daniel Colascione
Subject: Re: [Emacs-diffs] trunk r115095: Simplify, port and tune bool vector implementation.
Date: Sun, 17 Nov 2013 04:06:55 -0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0

>  (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);

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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]