[Freeciv-Dev] Re: (PR#1180) [PATCH] cleanup to canvas_pos_to_map_pos

[Jason Dorje Short <vze49r5w@xxxxxxxxxxx>]

Ross W. Wetmore wrote:
> It at least recognizes the positive denominator :-).
Hmm, interesting point.  DIVIDE does assume a positive denominator.

This is a good thing IMO.  But it should be documented.


<snip some alternatives for DIVIDE>

> The moral is, if you do low-level instruction fixes like this, then
> take the time to do them right. After that, forget about them :-).
Greg did some tests on different forms, and the one he eventually picked 
was found to be the fastest of all "correct" forms that he tried (i.e. 
not including your off-by-one optimization).  I don't know if he tried 
either of the forms you provide, but it did include several without any 
modular arithmetic.
A change to this macro is easily doable, but it should provide numbers 
showing that it is faster on average than the current form (although 
this may be machine- and compiler-dependent).  And you should also 
realize (1) the difference on Greg's computer is about 0.5us per call, 
and (2) this function is (right now) only occasionally called.  So your 
energy in optimizing might best be spent elsewhere (e.g., cutting down 
on the number of spurious redraws).
jason