[Freeciv-Dev] (PR#10180) [Patch] Optimize MAX(), MIN() calls

["Marko Lindqvist" <marko.lindqvist@xxxxxxxxxxx>]

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


  MAX() and MIN() macros evaluate either parameter twice. This in mind I 
looked all their users through, and found 'nothing'. Patch attached 
nonetheless.
  Maybe this does help for some compilers. Gcc already knows that 
(builtin) functions strlen(), abs() and sqrt() are 'pure' so it can 
optimize these situations even without this patch.

  I had to change macro SQSIZE into inline function get_sqsize() in 
order to use temporary variable.


  - Caz


diff -Nurd -X.diff_ignore freeciv/client/gui-xaw/cityrep.c 
freeciv/client/gui-xaw/cityrep.c
--- freeciv/client/gui-xaw/cityrep.c    2004-09-18 13:28:48.734375000 +0300
+++ freeciv/client/gui-xaw/cityrep.c    2004-09-18 13:59:30.578125000 +0300
@@ -599,8 +599,9 @@
       char new_city_line[MAX_LEN_CITY_TEXT];
 
       XtVaGetValues(city_list, XtNnumberStrings, &n, XtNlist, &list, NULL);
+      size_t cityname_len = strlen(pcity->name);
       if (0 != strncmp(pcity->name, list[i],
-                      MIN(strlen(pcity->name), REPORT_CITYNAME_ABBREV - 1))) {
+                      MIN(cityname_len, REPORT_CITYNAME_ABBREV - 1))) {
         break;
       }
       get_city_text(pcity, new_city_line, sizeof(new_city_line));
diff -Nurd -X.diff_ignore freeciv/client/messagewin_common.c 
freeciv/client/messagewin_common.c
--- freeciv/client/messagewin_common.c  2004-09-18 13:28:51.687500000 +0300
+++ freeciv/client/messagewin_common.c  2004-09-18 13:37:22.046875000 +0300
@@ -112,7 +112,8 @@
   const char *game_prefix2 = _("Game: ");
   size_t gp_len1 = strlen(game_prefix1);
   size_t gp_len2 = strlen(game_prefix2);
-  char *s = fc_malloc(MAX(strlen(message), min_msg_len) + 1);
+  size_t msg_len = strlen(message);
+  char *s = fc_malloc(MAX(msg_len, min_msg_len) + 1);
   int i, nspc;
 
   change = TRUE;
diff -Nurd -X.diff_ignore freeciv/common/map.c freeciv/common/map.c
--- freeciv/common/map.c        2004-09-18 13:28:53.765625000 +0300
+++ freeciv/common/map.c        2004-09-18 14:03:54.906250000 +0300
@@ -494,6 +494,8 @@
 ****************************************************************************/
 int map_vector_to_real_distance(int dx, int dy)
 {
+  int absdx = abs(dx);
+  int absdy = abs(dy);
   if (topo_has_flag(TF_HEX)) {
     if (topo_has_flag(TF_ISO)) {
       /* Iso-hex: you can't move NE or SW. */
@@ -501,10 +503,10 @@
          || (dx > 0 && dy < 0)) {
        /* Diagonal moves in this direction aren't allowed, so it will take
         * the full number of moves. */
-       return abs(dx) + abs(dy);
+        return absdx + absdy;
       } else {
        /* Diagonal moves in this direction *are* allowed. */
-       return MAX(abs(dx), abs(dy));
+        return MAX(absdx, absdy);
       }
     } else {
       /* Hex: you can't move SE or NW. */
@@ -512,14 +514,14 @@
          || (dx < 0 && dy < 0)) {
        /* Diagonal moves in this direction aren't allowed, so it will take
         * the full number of moves. */
-       return abs(dx) + abs(dy);
+       return absdx + absdy;
       } else {
        /* Diagonal moves in this direction *are* allowed. */
-       return MAX(abs(dx), abs(dy));
+        return MAX(absdx, absdy);
       }
     }
   } else {
-    return MAX(abs(dx), abs(dy));
+    return MAX(absdx, absdy);
   }
 }
 
diff -Nurd -X.diff_ignore freeciv/common/tech.c freeciv/common/tech.c
--- freeciv/common/tech.c       2004-09-18 13:28:53.953125000 +0300
+++ freeciv/common/tech.c       2004-09-18 14:18:05.390625000 +0300
@@ -552,10 +552,10 @@
   } tech_type_iterate_end;
 
   tech_type_iterate(tech) {
-    techcoststyle1[tech] = MAX((advances[tech].num_reqs + 1)
-                               * sqrt(advances[tech].num_reqs + 1)
-                               * (game.researchcost / 2),
-                               game.researchcost);
+    int style1_cost = (advances[tech].num_reqs + 1)
+                      * sqrt(advances[tech].num_reqs + 1)
+                      * (game.researchcost / 2);
+    techcoststyle1[tech] = MAX(style1_cost, game.researchcost);
   } tech_type_iterate_end;
 }
 
diff -Nurd -X.diff_ignore freeciv/server/generator/mapgen_topology.c 
freeciv/server/generator/mapgen_topology.c
--- freeciv/server/generator/mapgen_topology.c  2004-09-18 13:29:26.953125000 
+0300
+++ freeciv/server/generator/mapgen_topology.c  2004-09-18 14:30:21.906250000 
+0300
@@ -272,11 +272,12 @@
    * exept if separate poles is set
    */
   if (!topo_has_flag(TF_WRAPX) || !topo_has_flag(TF_WRAPY)) {
+    int sqsize = get_sqsize();
     if (map.separatepoles) {
       /* with separatepoles option strip poles are useless */
       ice_base_colatitude =
          (MAX(0, 100 * COLD_LEVEL / 3 - 1 *  MAX_COLATITUDE) 
-          + 1 *  MAX_COLATITUDE * SQSIZE) / (100 * SQSIZE);
+          + 1 *  MAX_COLATITUDE * sqsize) / (100 * sqsize);
       /* correction for single pole 
        * TODO uncomment it when generator 5 was well tuned 
        *      sometime it can put too many land near pole 
@@ -290,9 +291,9 @@
       /* any way strip poles are not so playable has isle poles */
       ice_base_colatitude =
          (MAX(0, 100 * COLD_LEVEL / 3 - 2 *  MAX_COLATITUDE) 
-          + 2 *  MAX_COLATITUDE * SQSIZE) / (100 * SQSIZE);
-    }  
-  }    
+          + 2 *  MAX_COLATITUDE * sqsize) / (100 * sqsize);
+    }
+  }
  
   map_init_topology(TRUE);
 }
diff -Nurd -X.diff_ignore freeciv/server/generator/mapgen_topology.h 
freeciv/server/generator/mapgen_topology.h
--- freeciv/server/generator/mapgen_topology.h  2004-09-18 13:29:26.968750000 
+0300
+++ freeciv/server/generator/mapgen_topology.h  2004-09-18 14:33:09.625000000 
+0300
@@ -19,10 +19,14 @@
 #define MAX_COLATITUDE 1000
 
 /* An estimate of the linear (1-dimensional) size of the map. */
-#define SQSIZE MAX(1, sqrt(map.xsize * map.ysize / 1000))
+static inline int get_sqsize()
+{
+  int sqsize = sqrt(map.xsize * map.ysize / 1000); \
+  return MAX(1, sqsize);
+}
 
 /* size safe Unit of colatitude */ 
-#define L_UNIT (MAX_COLATITUDE / (30 * SQSIZE) )
+#define L_UNIT (MAX_COLATITUDE / (30 * get_sqsize()) )
 
 /* define the 5 region of a Earth like map 
    =========================================================