Complete.Org: Mailing Lists: Archives: freeciv-dev: January 2004:
[Freeciv-Dev] (PR#7279) Macro optimizations
Home

[Freeciv-Dev] (PR#7279) Macro optimizations

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: undisclosed-recipients: ;
Subject: [Freeciv-Dev] (PR#7279) Macro optimizations
From: "Arnstein Lindgard" <a-l@xxxxxxx>
Date: Tue, 20 Jan 2004 09:08:03 -0800
Reply-to: rt@xxxxxxxxxxx

<URL: http://rt.freeciv.org/Ticket/Display.html?id=7279 >

This patch makes the server 17.6% faster.

I wrote 3 macros to see if I could learn something about the
preprocessor etc, looking at the Corecleanups and the profile
data (PR#7278).

1.  Profile shows that 96.7% of the calls to
        normalize_map_pos()
    are generated by
        is_normal_map_pos()
    so let's optimize the latter. This small macro alone is
    responsible for 10.2% speedup.

2.  Turned normalize_map_pos() into a macro as well. Another 5% gain.
    (It was neccessary to change "pf_map *map" into "pf_map *pf_map"
    throughout the code, since the global variable "map" is now used
    more often.)

3.  Macro base_city_map_to_map(). Another 2.4%.

It wasn't that hard, presuming I didn't completely miss something.
It looks like the
    map_pos_to_index()
macro, which recently was inlined, was more complex. When I tried to
write it as a single macro in the same fashion, I got warnings about
undefined behaviour. Didn't figure out why yet.

I am a bit surprised by the results; I had assumed Intel, AMD
wizardry would speculatively pre-proccess all of these functions
and put them more or less permanently in the CPU cache.

Furthermore, I additionally tried macrofying 4 more functions
which are not included in this patch:
    map_get_tile()
    map_get_terrain()
    map_get_continent()
    map_get_city()

...and guess what, I got NO additional speed improvement at all. Is
it conceivable that the previous macros freed the CPU cache, so that
the wizardry now handles these functions properly?

If that's correct, it just underlines the obvious - optimize the few
core functions that matter, and that's it. Still I think the 4 should
also be macros, since cache and other architectural features will
vary.

Question:
To make the last 4 work, I had to modify this:
#define MAP_TILE(x,y)   (map.tiles + map_pos_to_index(x, y))
#define MAP_TILE(x,y)   (map.tiles + map_pos_to_index((x), (y)))

Is it always correct to use paranthesis in macros?

Why not start using these simple tricks to gain free speed. The
Corecleanup is an old patch. Of course, inlining should give exactly
the same results, if you're using the gcc compiler :-) I guess
the trick is to decide on something.

* All numbers are percentage points off of the current cvs, measured
"user time" by the command "time" on my box. Savegames identical.
Compiled with gcc 3.3.2, CFLAGS="-pipe -O2 -march=athlon-tbird" and
--enable-debug.


Arnstein




diff -pruN -Xdiff_ignore freeciv/ai/aidiplomat.c macro/ai/aidiplomat.c
--- freeciv/ai/aidiplomat.c     Fri Nov 28 21:36:48 2003
+++ macro/ai/aidiplomat.c       Tue Jan 20 17:24:52 2004
@@ -60,7 +60,7 @@
 
 static void find_city_to_diplomat(struct player *pplayer, struct unit *punit,
                                   struct city **ctarget, int *move_dist,
-                                  struct pf_map *map);
+                                  struct pf_map *pf_map);
 
 /******************************************************************************
   Number of improvements that can be sabotaged in pcity.
@@ -153,7 +153,7 @@ void ai_choose_diplomat_offensive(struct
 
   /* Do we have a good reason for building diplomats? */
   {
-    struct pf_map *map;
+    struct pf_map *pf_map;
     struct pf_parameter parameter;
     struct unit_type *ut = get_unit_type(u);
     struct city *acity;
@@ -165,11 +165,11 @@ void ai_choose_diplomat_offensive(struct
 
     pft_fill_default_parameter(&parameter);
     pft_fill_unit_parameter(&parameter, punit);
-    map = pf_create_map(&parameter);
+    pf_map = pf_create_map(&parameter);
 
-    find_city_to_diplomat(pplayer, punit, &acity, &time_to_dest, map);
+    find_city_to_diplomat(pplayer, punit, &acity, &time_to_dest, pf_map);
 
-    pf_destroy_map(map);
+    pf_destroy_map(pf_map);
     destroy_unit_virtual(punit);
 
     if (acity == NULL || BV_ISSET(ai->stats.diplomat_reservations, acity->id)) 
{
@@ -320,7 +320,7 @@ static void ai_diplomat_city(struct unit
 **************************************************************************/
 static void find_city_to_diplomat(struct player *pplayer, struct unit *punit,
                                   struct city **ctarget, int *move_dist,
-                                  struct pf_map *map)
+                                  struct pf_map *pf_map)
 {
   bool has_embassy;
   int incite_cost = 0; /* incite cost */
@@ -330,7 +330,7 @@ static void find_city_to_diplomat(struct
   *ctarget = NULL;
   *move_dist = -1;
 
-  pf_iterator(map, pos) {
+  pf_iterator(pf_map, pos) {
     struct city *acity;
     struct player *aplayer;
     bool can_incite;
@@ -378,7 +378,8 @@ static void find_city_to_diplomat(struct
 **************************************************************************/
 static struct city *ai_diplomat_defend(struct player *pplayer,
                                        struct unit *punit,
-                                       Unit_Type_id utype, struct pf_map *map)
+                                       Unit_Type_id utype,
+                                       struct pf_map *pf_map)
 {
   int best_dist = 30; /* any city closer than this is better than none */
   int best_urgency = 0;
@@ -392,7 +393,7 @@ static struct city *ai_diplomat_defend(s
     return pcity;
   }
 
-  pf_iterator(map, pos) {
+  pf_iterator(pf_map, pos) {
     struct city *acity;
     struct player *aplayer;
     int dipls, urgency;
@@ -441,12 +442,13 @@ static struct city *ai_diplomat_defend(s
   Will try to bribe a ship on the coast as well as land stuff.
 **************************************************************************/
 static bool ai_diplomat_bribe_nearby(struct player *pplayer, 
-                                     struct unit *punit, struct pf_map *map)
+                                     struct unit *punit,
+                                     struct pf_map *pf_map)
 {
   int gold_avail = pplayer->economic.gold - pplayer->ai.est_upkeep;
   struct ai_data *ai = ai_data_get(pplayer);
 
-  pf_iterator(map, pos) {
+  pf_iterator(pf_map, pos) {
     struct tile *ptile = map_get_tile(pos.x, pos.y);
     bool threat = FALSE;
     int newval, bestval = 0, cost;
@@ -507,7 +509,7 @@ static bool ai_diplomat_bribe_nearby(str
       struct pf_path *path;
 
       MAPSTEP(x, y, pos.x, pos.y, DIR_REVERSE(pos.dir_to_here));
-      path = pf_get_path(map, pos.x, pos.y);
+      path = pf_get_path(pf_map, pos.x, pos.y);
       if (!path || !ai_unit_execute_path(punit, path) 
           || punit->moves_left <= 0) {
         pf_destroy_path(path);
@@ -550,7 +552,7 @@ void ai_manage_diplomat(struct player *p
 {
   struct city *pcity, *ctarget = NULL;
   struct pf_parameter parameter;
-  struct pf_map *map;
+  struct pf_map *pf_map;
   struct pf_position pos;
 
   CHECK_UNIT(punit);
@@ -559,14 +561,14 @@ void ai_manage_diplomat(struct player *p
   pft_fill_default_parameter(&parameter);
   pft_fill_unit_parameter(&parameter, punit);
   parameter.get_zoc = NULL; /* kludge */
-  map = pf_create_map(&parameter);
+  pf_map = pf_create_map(&parameter);
 
   pcity = map_get_city(punit->x, punit->y);
 
   /* Look for someone to bribe */
-  if (!ai_diplomat_bribe_nearby(pplayer, punit, map)) {
+  if (!ai_diplomat_bribe_nearby(pplayer, punit, pf_map)) {
     /* Died or ran out of moves */
-    pf_destroy_map(map);
+    pf_destroy_map(pf_map);
     return;
   }
 
@@ -577,7 +579,7 @@ void ai_manage_diplomat(struct player *p
     UNIT_LOG(LOG_DIPLOMAT, punit, "stays to protect %s (urg %d)", 
              pcity->name, pcity->ai.urgency);
     ai_unit_new_role(punit, AIUNIT_NONE, -1, -1); /* abort mission */
-    pf_destroy_map(map);
+    pf_destroy_map(pf_map);
     return;
   }
 
@@ -588,7 +590,7 @@ void ai_manage_diplomat(struct player *p
 
     ctarget = map_get_city(goto_dest_x(punit), goto_dest_y(punit));
     assert(is_goto_dest_set(punit));
-    if (pf_get_position(map, goto_dest_x(punit), goto_dest_y(punit), &pos)
+    if (pf_get_position(pf_map, goto_dest_x(punit), goto_dest_y(punit), &pos)
         && ctarget) {
       if (same_pos(ctarget->x, ctarget->y, punit->x, punit->y)) {
         failure = TRUE;
@@ -617,11 +619,11 @@ void ai_manage_diplomat(struct player *p
    * a new map for its iterator. */
   if (parameter.start_x != punit->x || parameter.start_y != punit->y
       || punit->ai.ai_role == AIUNIT_NONE) {
-    pf_destroy_map(map);
+    pf_destroy_map(pf_map);
     pft_fill_default_parameter(&parameter);
     pft_fill_unit_parameter(&parameter, punit);
     parameter.get_zoc = NULL; /* kludge */
-    map = pf_create_map(&parameter);
+    pf_map = pf_create_map(&parameter);
   }
 
   /* If we are not busy, acquire a target. */
@@ -629,18 +631,18 @@ void ai_manage_diplomat(struct player *p
     enum ai_unit_task task;
     int move_dist; /* dummy */
 
-    find_city_to_diplomat(pplayer, punit, &ctarget, &move_dist, map);
+    find_city_to_diplomat(pplayer, punit, &ctarget, &move_dist, pf_map);
 
     if (ctarget) {
       task = AIUNIT_ATTACK;
       punit->ai.bodyguard = -1; /* want one */
       UNIT_LOG(LOG_DIPLOMAT, punit, "going on attack");
     } else if ((ctarget = ai_diplomat_defend(pplayer, punit,
-                                             punit->type, map)) != NULL) {
+                                             punit->type, pf_map)) != NULL) {
       task = AIUNIT_DEFEND_HOME;
       UNIT_LOG(LOG_DIPLOMAT, punit, "going defensive");
     } else if ((ctarget = find_closest_owned_city(pplayer, punit->x, punit->y, 
-                                                  TRUE, NULL), map) != NULL) {
+                                                  TRUE, NULL), pf_map) != 
NULL) {
       /* This should only happen if the entire continent was suddenly
        * conquered. So we head for closest coastal city and wait for someone
        * to code ferrying for diplomats, or hostile attacks from the sea. */
@@ -648,7 +650,7 @@ void ai_manage_diplomat(struct player *p
       UNIT_LOG(LOG_DIPLOMAT, punit, "going idle");
     } else {
       UNIT_LOG(LOG_DIPLOMAT, punit, "could not find a job");
-      pf_destroy_map(map);
+      pf_destroy_map(pf_map);
       return;
     }
 
@@ -662,7 +664,7 @@ void ai_manage_diplomat(struct player *p
   if (!same_pos(punit->x, punit->y, ctarget->x, ctarget->y)) {
     struct pf_path *path;
 
-    path = pf_get_path(map, goto_dest_x(punit), goto_dest_y(punit));
+    path = pf_get_path(pf_map, goto_dest_x(punit), goto_dest_y(punit));
     if (path && ai_unit_execute_path(punit, path) && punit->moves_left > 0) {
       /* Check if we can do something with our destination now. */
       if (punit->ai.ai_role == AIUNIT_ATTACK) {
@@ -679,5 +681,5 @@ void ai_manage_diplomat(struct player *p
     }
     pf_destroy_path(path);
   }
-  pf_destroy_map(map);
+  pf_destroy_map(pf_map);
 }
diff -pruN -Xdiff_ignore freeciv/ai/aiunit.c macro/ai/aiunit.c
--- freeciv/ai/aiunit.c Sun Jan 11 18:56:46 2004
+++ macro/ai/aiunit.c   Tue Jan 20 17:24:52 2004
@@ -533,7 +533,7 @@ bool ai_manage_explorer(struct unit *pun
   int best_x = -1, best_y = -1;
 
   /* Path-finding stuff */
-  struct pf_map *map;
+  struct pf_map *pf_map;
   struct pf_parameter parameter;
 
 #define DIST_FACTOR   0.6
@@ -544,12 +544,12 @@ bool ai_manage_explorer(struct unit *pun
   /* When exploring, even AI should pretend to not cheat. */
   parameter.omniscience = FALSE;
 
-  map = pf_create_map(&parameter);
-  while (pf_next(map)) {
+  pf_map = pf_create_map(&parameter);
+  while (pf_next(pf_map)) {
     float desirable;
     struct pf_position pos;
 
-    pf_next_get_position(map, &pos);
+    pf_next_get_position(pf_map, &pos);
     
     /* Our callback should insure this. */
     assert(map_is_known(pos.x, pos.y, pplayer));
@@ -579,7 +579,7 @@ bool ai_manage_explorer(struct unit *pun
       break;
     }
   }
-  pf_destroy_map(map);
+  pf_destroy_map(pf_map);
 
   /* Go to the best tile found. */
   if (most_desirable > 0) {
@@ -2243,7 +2243,7 @@ static void ai_manage_caravan(struct pla
 static bool ai_ferry_findcargo(struct unit *punit)
 {
   /* Path-finding stuff */
-  struct pf_map *map;
+  struct pf_map *pf_map;
   struct pf_parameter parameter;
   int passengers = ai_data_get(unit_owner(punit))->stats.passengers;
 
@@ -2260,11 +2260,11 @@ static bool ai_ferry_findcargo(struct un
   parameter.get_TB = no_fights_or_unknown;
   parameter.omniscience = FALSE;
   
-  map = pf_create_map(&parameter);
-  while (pf_next(map)) {
+  pf_map = pf_create_map(&parameter);
+  while (pf_next(pf_map)) {
     struct pf_position pos;
 
-    pf_next_get_position(map, &pos);
+    pf_next_get_position(pf_map, &pos);
     
     unit_list_iterate(map_get_tile(pos.x, pos.y)->units, aunit) {
       if (aunit->ai.ferryboat == FERRY_WANTED) {
@@ -2275,7 +2275,7 @@ static bool ai_ferry_findcargo(struct un
         /* Exchange phone numbers */
         ai_set_passenger(punit, aunit);
         ai_set_ferry(aunit, punit);
-        pf_destroy_map(map);
+        pf_destroy_map(pf_map);
         return TRUE;
       }
     } unit_list_iterate_end;
@@ -2283,7 +2283,7 @@ static bool ai_ferry_findcargo(struct un
 
   UNIT_LOG(LOG_ERROR, punit,
            "AI Passengers counting reported false positive %d", passengers);
-  pf_destroy_map(map);
+  pf_destroy_map(pf_map);
   return FALSE;
 }
 
diff -pruN -Xdiff_ignore freeciv/client/goto.c macro/client/goto.c
--- freeciv/client/goto.c       Sat Jan 10 14:45:13 2004
+++ macro/client/goto.c Tue Jan 20 17:24:52 2004
@@ -507,7 +507,7 @@ void send_patrol_route(struct unit *puni
   int i;
   struct pf_path *path = NULL, *return_path;
   struct pf_parameter parameter = goto_map.template;
-  struct pf_map *map;
+  struct pf_map *pf_map;
 
   assert(is_active);
   assert(punit->id == goto_map.unit_id);
@@ -516,8 +516,8 @@ void send_patrol_route(struct unit *puni
   parameter.start_y = goto_map.parts[goto_map.num_parts - 1].end_y;
   parameter.moves_left_initially
     = goto_map.parts[goto_map.num_parts - 1].end_moves_left;
-  map = pf_create_map(&parameter);
-  return_path = pf_get_path(map, goto_map.parts[0].start_x,
+  pf_map = pf_create_map(&parameter);
+  return_path = pf_get_path(pf_map, goto_map.parts[0].start_x,
                            goto_map.parts[0].start_y);
   if (!return_path) {
     die("No return path found!");
@@ -528,7 +528,7 @@ void send_patrol_route(struct unit *puni
   }
   path = pft_concat(path, return_path);
 
-  pf_destroy_map(map);
+  pf_destroy_map(pf_map);
   pf_destroy_path(return_path);
 
   send_path_route(punit, path, ACTIVITY_PATROL);
@@ -646,7 +646,7 @@ struct pf_path *path_to_nearest_allied_c
 {
   struct city *pcity = NULL;
   struct pf_parameter parameter;
-  struct pf_map *map;
+  struct pf_map *pf_map;
   struct pf_path *path = NULL;
 
   if ((pcity = is_allied_city_tile(map_get_tile(punit->x, punit->y),
@@ -656,12 +656,12 @@ struct pf_path *path_to_nearest_allied_c
   }
 
   fill_client_goto_parameter(punit, &parameter);
-  map = pf_create_map(&parameter);
+  pf_map = pf_create_map(&parameter);
 
-  while (pf_next(map)) {
+  while (pf_next(pf_map)) {
     struct pf_position pos;
 
-    pf_next_get_position(map, &pos);
+    pf_next_get_position(pf_map, &pos);
 
     if ((pcity = is_allied_city_tile(map_get_tile(pos.x, pos.y),
                                     game.player_ptr))) {
@@ -670,10 +670,10 @@ struct pf_path *path_to_nearest_allied_c
   }
 
   if (pcity) {
-    path = pf_next_get_path(map);
+    path = pf_next_get_path(pf_map);
   }
 
-  pf_destroy_map(map);
+  pf_destroy_map(pf_map);
 
   return path;
 }
diff -pruN -Xdiff_ignore freeciv/common/aicore/path_finding.c 
macro/common/aicore/path_finding.c
--- freeciv/common/aicore/path_finding.c        Mon Jan 19 16:58:41 2004
+++ macro/common/aicore/path_finding.c  Tue Jan 20 17:24:52 2004
@@ -1123,7 +1123,7 @@ static struct pf_path *danger_get_path(s
 /************************************************************************
   Return current pf_parameter for given pf_map.
 ************************************************************************/
-struct pf_parameter *pf_get_parameter(struct pf_map *map)
+struct pf_parameter *pf_get_parameter(struct pf_map *pf_map)
 {
-  return map->params;
+  return pf_map->params;
 }
diff -pruN -Xdiff_ignore freeciv/common/aicore/path_finding.h 
macro/common/aicore/path_finding.h
--- freeciv/common/aicore/path_finding.h        Fri Nov 28 21:54:22 2003
+++ macro/common/aicore/path_finding.h  Tue Jan 20 17:24:52 2004
@@ -397,6 +397,6 @@ void pf_destroy_map(struct pf_map *pf_ma
 struct pf_position *pf_last_position(struct pf_path *path);
 
 /* Return the current parameters for the given map. */
-struct pf_parameter *pf_get_parameter(struct pf_map *map);
+struct pf_parameter *pf_get_parameter(struct pf_map *pf_map);
 
 #endif
diff -pruN -Xdiff_ignore freeciv/common/city.c macro/common/city.c
--- freeciv/common/city.c       Wed Jan 14 19:31:46 2004
+++ macro/common/city.c Tue Jan 20 17:24:52 2004
@@ -130,25 +130,6 @@ bool map_to_city_map(int *city_map_x, in
 Finds the map position for a given city map coordinate of a certain
 city. Returns true if the map position found is real.
 **************************************************************************/
-bool base_city_map_to_map(int *map_x, int *map_y,
-                        int city_center_x, int city_center_y,
-                        int city_map_x, int city_map_y)
-{
-  assert(is_valid_city_coords(city_map_x, city_map_y));
-  *map_x = city_center_x + city_map_x - CITY_MAP_SIZE / 2;
-  *map_y = city_center_y + city_map_y - CITY_MAP_SIZE / 2;
-
-  /* We check the border first to avoid doing an unnecessary
-   * normalization; this is just an optimization. */
-  return (!IS_BORDER_MAP_POS(city_center_x, city_center_y,
-                            CITY_MAP_SIZE / 2)
-         || normalize_map_pos(map_x, map_y));
-}
-
-/**************************************************************************
-Finds the map position for a given city map coordinate of a certain
-city. Returns true if the map position found is real.
-**************************************************************************/
 bool city_map_to_map(int *map_x, int *map_y,
                    const struct city *const pcity,
                    int city_map_x, int city_map_y)
diff -pruN -Xdiff_ignore freeciv/common/city.h macro/common/city.h
--- freeciv/common/city.h       Wed Jan 14 19:31:46 2004
+++ macro/common/city.h Tue Jan 20 17:27:17 2004
@@ -380,9 +380,6 @@ bool is_city_center(int city_x, int city
 bool map_to_city_map(int *city_map_x, int *city_map_y,
                    const struct city *const pcity, int map_x, int map_y);
 
-bool base_city_map_to_map(int *map_x, int *map_y, int city_center_x,
-                        int city_center_y, int city_map_x,
-                        int city_map_y);
 bool city_map_to_map(int *map_x, int *map_y, const struct city *const pcity,
                    int city_map_x, int city_map_y);
 
@@ -494,4 +491,37 @@ void city_styles_free(void);
 #define built_impr_iterate_end                                                \
   } impr_type_iterate_end;
 
+#ifdef UNUSED
+/**************************************************************************
+Finds the map position for a given city map coordinate of a certain
+city. Returns true if the map position found is real.
+**************************************************************************/
+bool base_city_map_to_map(int *map_x, int *map_y,
+                        int city_center_x, int city_center_y,
+                        int city_map_x, int city_map_y)
+{
+  assert(is_valid_city_coords(city_map_x, city_map_y));
+  *map_x = city_center_x + city_map_x - CITY_MAP_SIZE / 2;
+  *map_y = city_center_y + city_map_y - CITY_MAP_SIZE / 2;
+
+  /* We check the border first to avoid doing an unnecessary
+   * normalization; this is just an optimization. */
+  return (!IS_BORDER_MAP_POS(city_center_x, city_center_y,
+                            CITY_MAP_SIZE / 2)
+         || normalize_map_pos(map_x, map_y));
+}
+#endif
+
+/* Function turned macro for optimization. */
+#define base_city_map_to_map(map_x, map_y,                             \
+                            city_center_x, city_center_y,              \
+                            city_map_x, city_map_y)                    \
+( assert(is_valid_city_coords((city_map_x), (city_map_y))),            \
+  *(map_x) = (city_center_x) + (city_map_x) - CITY_MAP_SIZE / 2,       \
+  *(map_y) = (city_center_y) + (city_map_y) - CITY_MAP_SIZE / 2,       \
+  (!IS_BORDER_MAP_POS((city_center_x), (city_center_y),                        
\
+                      CITY_MAP_SIZE / 2)                               \
+   || normalize_map_pos((map_x), (map_y))) )
+
+
 #endif  /* FC__CITY_H */
diff -pruN -Xdiff_ignore freeciv/common/map.c macro/common/map.c
--- freeciv/common/map.c        Mon Jan 19 16:58:36 2004
+++ macro/common/map.c  Tue Jan 20 17:24:52 2004
@@ -1265,54 +1265,6 @@ bool is_real_map_pos(int x, int y)
 }
 
 /**************************************************************************
-Returns TRUE iff the map position is normal. "Normal" here means that
-it is both a real/valid coordinate set and that the coordinates are in
-their canonical/proper form. In plain English: the coordinates must be
-on the map.
-**************************************************************************/
-bool is_normal_map_pos(int x, int y)
-{
-  int x1 = x, y1 = y;
-
-  return (normalize_map_pos(&x1, &y1) && (x1 == x) && (y1 == y));
-}
-
-/**************************************************************************
-  If the position is real, it will be normalized and TRUE will be returned.
-  If the position is unreal, it will be left unchanged and FALSE will be
-  returned.
-
-  Note, we need to leave x and y with sane values even in the unreal case.
-  Some callers may for instance call nearest_real_pos on these values.
-**************************************************************************/
-bool normalize_map_pos(int *x, int *y)
-{
-  int nat_x, nat_y;
-
-  /* Normalization is best done in native coordinatees. */
-  map_to_native_pos(&nat_x, &nat_y, *x, *y);
-
-  /* If the position is out of range in a non-wrapping direction, it is
-   * unreal. */
-  if (!((topo_has_flag(TF_WRAPX) || (nat_x >= 0 && nat_x < map.xsize))
-       && (topo_has_flag(TF_WRAPY) || (nat_y >= 0 && nat_y < map.ysize)))) {
-    return FALSE;
-  }
-
-  /* Wrap in X and Y directions, as needed. */
-  if (topo_has_flag(TF_WRAPX)) {
-    nat_x = FC_WRAP(nat_x, map.xsize);
-  }
-  if (topo_has_flag(TF_WRAPY)) {
-    nat_y = FC_WRAP(nat_y, map.ysize);
-  }
-
-  /* Now transform things back to map coordinates. */
-  native_to_map_pos(x, y, nat_x, nat_y);
-  return TRUE;
-}
-
-/**************************************************************************
 Twiddle *x and *y to point the the nearest real tile, and ensure that the
 position is normalized.
 **************************************************************************/
diff -pruN -Xdiff_ignore freeciv/common/map.h macro/common/map.h
--- freeciv/common/map.h        Mon Jan 19 16:58:36 2004
+++ macro/common/map.h  Tue Jan 20 17:30:32 2004
@@ -197,6 +197,80 @@ unsigned short map_get_continent(int x, 
 void initialize_move_costs(void);
 void reset_move_costs(int x, int y);
 
+/* Temporary storage variables for macros. */
+int _FC_MAP_X, _FC_MAP_Y, _FC_NAT_X, _FC_NAT_Y;
+extern int _FC_MAP_X, _FC_MAP_Y, _FC_NAT_X, _FC_NAT_Y;
+
+/*
+ * Normalization of coordinates needs optimization more than anything.
+ * The macro should be the equivalent of the unused function definition.
+ */
+
+#ifdef UNUSED
+/**************************************************************************
+  If the position is real, it will be normalized and TRUE will be returned.
+  If the position is unreal, it will be left unchanged and FALSE will be
+  returned.
+
+  Note, we need to leave x and y with sane values even in the unreal case.
+  Some callers may for instance call nearest_real_pos on these values.
+**************************************************************************/
+bool normalize_map_pos(int *x, int *y)
+{
+  int nat_x, nat_y;
+
+  /* Normalization is best done in native coordinatees. */
+  map_to_native_pos(&nat_x, &nat_y, *x, *y);
+
+  /* If the position is out of range in a non-wrapping direction, it is
+   * unreal. */
+  if (!((topo_has_flag(TF_WRAPX) || (nat_x >= 0 && nat_x < map.xsize))
+       && (topo_has_flag(TF_WRAPY) || (nat_y >= 0 && nat_y < map.ysize)))) {
+    return FALSE;
+  }
+
+  /* Wrap in X and Y directions, as needed. */
+  if (topo_has_flag(TF_WRAPX)) {
+    nat_x = FC_WRAP(nat_x, map.xsize);
+  }
+  if (topo_has_flag(TF_WRAPY)) {
+    nat_y = FC_WRAP(nat_y, map.ysize);
+  }
+
+  /* Now transform things back to map coordinates. */
+  native_to_map_pos(x, y, nat_x, nat_y);
+  return TRUE;
+}
+#endif
+
+#define normalize_map_pos(PX, PY)                              \
+( map_to_native_pos(&_FC_NAT_X, &_FC_NAT_Y, *(PX), *(PY)),     \
+  !(   (topo_has_flag(TF_WRAPX) ||                             \
+       (_FC_NAT_X >= 0 && _FC_NAT_X < map.xsize))              \
+    && (topo_has_flag(TF_WRAPY) ||                             \
+       (_FC_NAT_Y >= 0 && _FC_NAT_Y < map.ysize)) )            \
+  ? FALSE                                                      \
+  : ((_FC_NAT_X = topo_has_flag(TF_WRAPX)                      \
+                  ? FC_WRAP(_FC_NAT_X, map.xsize)              \
+                  : _FC_NAT_X),                                        \
+     (_FC_NAT_Y = topo_has_flag(TF_WRAPY)                      \
+                  ? FC_WRAP(_FC_NAT_Y, map.ysize)              \
+                  : _FC_NAT_Y),                                        \
+    native_to_map_pos((PX), (PY), _FC_NAT_X, _FC_NAT_Y),       \
+    TRUE) )
+
+/*
+ * Yields TRUE if the map position is normal. "Normal" here means
+ * that it is both a real/valid coordinate set and that the
+ * coordinates are in their canonical/proper form. In plain English:
+ * the coordinates must be on the map.
+ */
+#define is_normal_map_pos(PX, PY)                              \
+( _FC_MAP_X = (PX), _FC_MAP_Y = (PY),                          \
+  normalize_map_pos(&_FC_MAP_X, &_FC_MAP_Y) &&                 \
+  _FC_MAP_X == (PX) &&                                         \
+  _FC_MAP_Y == (PY) )
+
 /* Maximum value of index (for sanity checks and allocations) */
 #define MAX_MAP_INDEX map.xsize * map.ysize
 
@@ -264,7 +338,6 @@ void map_set_special(int x, int y, enum 
 void map_clear_special(int x, int y, enum tile_special_type spe);
 void map_clear_all_specials(int x, int y);
 bool is_real_map_pos(int x, int y);
-bool is_normal_map_pos(int x, int y);
 
 /* implemented in server/maphand.c and client/climisc.c */
 enum known_type map_get_known(int x, int y, struct player *pplayer);
@@ -288,7 +361,6 @@ bool contains_special(enum tile_special_
    || (x) < (dist) || (x) >= map.xsize - (dist)               \
    || (y) < (dist) || (y) >= map.ysize - (dist))
 
-bool normalize_map_pos(int *x, int *y);
 void nearest_real_pos(int *x, int *y);
 void map_distance_vector(int *dx, int *dy, int x0, int y0, int x1, int y1);
 int map_num_tiles(void);


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