On Thu, 28 Feb 2013, Alexander Motin wrote:
On 28.02.2013 18:25, Alexey Dokuchaev wrote:
On Thu, Feb 28, 2013 at 11:27:02AM +0000, Davide Italiano wrote:
New Revision: 247460
URL: http://svnweb.freebsd.org/changeset/base/247460
Log:
MFcalloutng (r247427 by mav):
We don't need any precision here. Let it be fast and dirty shift then
slow and excessively precise 64-bit division.
- if (sbt >= 0 && us > sbt / SBT_1US)
- us = sbt / SBT_1US;
+ if (sbt >= 0 && us > (sbt >> 12))
+ us = (sbt >> 12);
Does this really buy us anything? Modern compilers should be smart enough to
generate correct code. Do you have evidence that this is not the case here?
Not to mention that it obfuscates the code by using some magic constant.
SBT_1US is 4294 (0x10c6). The best that compiler may do is replace
division with multiplication. In fact, Clang even does this on amd64.
But on i386 it calls __divdi3(), doing 64bit division in software. Shift
is definitely cheaper and 5% precision is fine here.
I missed the additional magic in my previous reply.
But you should write the sloppy scaling as division by a sloppy factor:
#define SSBT_1us 4096 /* power of 2 closest to SSBT_1US */
if (sbt >= 0 && us > (uint64_t)sbt / SSBT_1us)
us = (uint64_t)sbt / SSBT_1us;
or provide and use conversion functions that do sloppy and non-sloppy
scaling. I don't like having conversion functions for every possible
conversion, but this one is much more magic than for example
TIMEVAL_TO_TIMESPEC(). The casts to (uint64_t) are to help the compiler
understand that the sign bit is not there.
The need for magic scaling shows that the binary representation given
by sbintime_t isn't very good. Mose clients want natural units of
microseconds or nanoseconds and need scale factors like
(4294.967206 / 4096) to adjust (4294 is already sloppy). The binary
representation allows some minor internal optimizations and APIs are
made unnatural to avoid double conversions.
While here, I will point out style bugs introduced in the above:
- parentheses in "us = (sbt >> 12);" are redundant and reduce clarity,
like parentheses in "us = (sbt / N);" would have, since the shift
operator binds much more tightly than the assignment operator.
- parentheses in "us > (sbt >> 12);" are redundant but may increase
clarity, since the shift operator doesn't bind much more tightly
than the '<' comparison operator. This one is hard to remember, but
looking it up confirms that the precedence is not broken as designed
in this case, but that the precedence is only 1 level higher for the
shift operator. The main broken as designed cases are the shift
operator being 1 level lower than addition and subtraction, and
bitwise operators being many more levels lower than other aritmetic
operators and even below all comparision operators.
Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
