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

["rwetmore@xxxxxxxxxxxx" <rwetmore@xxxxxxxxxxxx>]

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


Jason Short wrote:
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=7279 >
> 
> Arnstein Lindgard wrote:
> 
>>+#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) )
>>+
> 
> 1.  I would rather see normalize_map_pos written as an inline function. 

Why virtually guarantee it will always be called as an out-of-line function
when this is a known performance hotspot?

>   That allows you to use global and local variables without having to 
> worry about shadowing.

Of course worries about shadowing is a red herring.

Since the way to provide local variables to macros still seems to elude
you, I have included a full-blown template that suggests how O-O methods
can be implemented as functional macros. It deals with the local issue
so there is no more need for you to make incorrect or misleading statements
like the above to your embarassment and the technical mess they cause. Note
this technique is already in use in Freeciv macros so it should not have
been that hard to work it out.

Macros implementing void functions of course have no limitation to
expression syntax, so can implement code blocks at will with any code
including variable definitions. The only issue is with the limitation
of coding in expressions, i.e. when returning an expression value.

> 2.  is_normal_map_pos can easily be rewritten without normalize_map_pos: 
> and it will be much faster.
> 
>    {
>      map_to_native_pos(&x, &y, x, y);
>      return (x >= 0 && y >= 0 && x < map.xsize && y < map.ysize);
>    }
> 
> The only reason it's done the way it is is that Raimar wanted the 
> "logic" to be concentrated in normalize_map_pos only.
> 
> Note that unless you compile with DEBUG is_normal_map_pos is rarely 
> called.  So this isn't a big issue IMO.

I really wish this were the case. I hope I am wrong and you are right
on this one.

> jason

=====

This is a fairly elaborate template of a functional macro
which emulates the supposed advantages of an inline function.
Use of alloca() to create temporary stack space is a potential
compatibility issue, but can be removed by making preallocation
of local storage in env a requirement or part of the definition
macro rather than option. Note alloca() becomes a single stack
register op in most compilers.

By using a structure to hold the local environment rather than
an explicit list of local storage elements, the user code is
freed from any need to understand or code parts of the macro
implementation. The macro can be recoded with changes to its
internal storage structure and a recompile rather than edit
and recompile is all that is needed to update all user modules.
A clean separation between implementation and interface is
always a prime concern.

In effect all macros would have the same format for their
leading argument, which is incidentally the way O-O code is
structured to pass its local or instance data ("this" pointer).
Implementing macros as methods on a given object means there
is one struct macenv for all such object methods. It holds all
needed instance and working data elements.

As an example, all coordinate macros could pass the current map
or topo structure as their environment eliminating need for the
current global references and as a bonus effect letting one use
multiple maps in the same game. Temporary working fields might
need to be added to current structures to provide the additional
local storage (aka private variables).

A pseudo object *C* implementation with functions and replacement
macros for guaranteed performance is a good programming model to
handle API specifications.

The template also illustrates correct behaviour for making a
macro emulate a function in its argument handling. This is
currently used in Freeciv macros like iterators.


typedef struct macenv {
   int  arg1;
   int  arg2;
   int  temp;
} MACENV_T;

#define MACENV(env)     MACENV_T *env = NULL

#define macfn(env, arg1, arg2)              \
( (env || (env = alloca(sizeof(MACENV)))),  \
   env->arg1 = (arg1),                       \
   env->arg2 = (arg2),                       \
   //  ... functional code goes here         \
   //  ... e.g.  return (arg1 + arg2) ** 2;  \
   env->temp = env->arg1 + env->arg2,        \
   env->temp *= env->temp                    \
)


void userfn()
{
   MACENV(my_env);
     ...
   int val = macfn(my_env, 1, 2);
}
=====