On 04/13/2012 02:03 PM, Dick Hollenbeck wrote:
> On 04/13/2012 12:00 PM, Wayne Stambaugh wrote:
>> On 4/13/2012 10:04 AM, Dick Hollenbeck wrote:
>>>> Also, you may want to
>>>> change Mils2iu to use wxRound() so it will work for negative values.
>>> wxRound()'s use of C lib's round() seems like overkill to me.
>>> Since the objective is to produce an integer, not a double, there is an
>>> easier way that
>>> has the possibility of letting the compiler do some of the work, i.e. in
>>> advance, on
>>> constants,
>>> without the overhead of a mandatory floating point function call.
>>>
>>> See my latest DMils2iu() as a suggestion.
>>>
>>>
>>> Dick
>> wxRound may be overkill
I did not say it was overkill. I said it can be making an unnecessary function
call.
Some of these are in static constructors. Some are deferred in time, done in
normal
constructors.
In any case, it can increase code size, maybe even delay startup if enough are
in static
constructors.
If constants are passed to wxRound() then there is no reason for a runtime
function call,
this kind of calculation can be done at compile time, resulting in the
optimizer finding a
suitable integer constant that is put into the code at compile time, at least
for the
Release build. For the Debug build, this optimization will not happen
especially if there
is an assert in our functions. The compiler cannot do the reduction with the
assert test
in play, the runtime has to evaluate that.
So the Debug build will catch the problems with constants, but then knowing
there are
none, there is no reason to keep doing this runtime check *ON CONSTANTS*.
Look here:
inline int wxRound(double x)
{
wxASSERT_MSG( x > INT_MIN - 0.5 && x < INT_MAX + 0.5,
wxT("argument out of supported range") );
#if defined(HAVE_ROUND)
return int(round(x));
#else
return (int)(x < 0 ? x - 0.5 : x + 0.5);
#endif
}
The call to round() should not happen. The alternate path seems more than
adequate to
me. The assert is a good thing.
However, I do not see where HAVE_ROUND is defined. So I do not know which code
path is
taken in the above, FOR SURE.
There are a couple of issues at work here.
I'm just saying "wxRound()'s use of round()", seems inappropriate, it should be
avoided.
I would hope we can agree that 150 unnecessary function calls that are easily
avoided
might be worth 5 to 10 lines of code.
Dick
_______________________________________________
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help : https://help.launchpad.net/ListHelp
