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

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

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


Arnstein Lindgard wrote:
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=7279 >
> 
> On Tue, 20 Jan 2004 17:44:09 -0800 rwetmore@xxxxxxxxxxxx wrote:
> 
> 
>>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.
>>[...]
>>#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                    \
>>)
> 
> 
> Assuming we need local variables:

Correct.

As was stated this was the elaborate version to try and illustrate all
the possibilities with key elements blocked out in overly obvious ways.
One in general should use what is needed or agreed upon as the accepted
best practice for the project.

The key point is that in an expression you can't define local storage
since that requires a code block. But that doesn't mean you can't use it,
you just have to provide it as data to the macro from a code block you
are running in.

> My linux man pages says alloca() is non POSIX and it's use is
> discouraged. Some existing Freeciv code (which looks imported)
> substitutes alloca() with malloc() if you don't have a requirement,
> and that introduces overhead for one function call in your macro.

malloc() is a function and I think is almost always called as one. You
don't want to call a memory function, especially one that requires a
corresponding free().

alloca() is a builtin that is implemented as a single stack instruction.
When you leave the code block the stack is automatically popped and the
memory freed.

Using or relying on alloca() is not good, and I wouldn't actually
recommend this even though it is presented for completeness, i.e.
fully implementing the disputed inline features. GCC is fine, but
it has the same compatibility issues as inline for general compilers.

> I suppose we could write:
> 
> typedef struct macenv {
>    int  arg1;
>    int  arg2;
>    int  temp;
> } MACENV_T;
> 
> #define MACENV(env)   MACENV_T *env
   #define MACENV(env)  MACENV_T env
> 
> #define macfn(ENV, ARG1, ARG2)                        \
> (  (ENV)->arg1 = (ARG1),                      \
>    (ENV)->arg2 = (ARG2),                      \
>    (ENV)->temp = (ENV)->arg1 + (ENV)->arg2,   \
>    (ENV)->temp *= (ENV)->temp                 \
   (  (ENV).arg1 = (ARG1),                      \
      (ENV).arg2 = (ARG2),                      \
      (ENV).temp = (ENV).arg1 + (ENV).arg2,     \
      (ENV).temp *= (ENV).temp                  \
> 
> void userfn()
> {
>    MACENV(my_env);
>    int val = macfn(my_env, 1, 2);
> }
> 
> Either way, I understand we'd have to add a "MACENV(my_env);" line to
> every function that uses the macro, to ensure the optimization we

You need to use this every time you want a distinct local storage block.

But if you decide that you want to manage storage in other ways, you can,
for example, define whatever global blocks you need and reference them
instead with no actual MACENV(my_env) statements placed in the code at all.

> may not neccessarily get with inlining. And we have to add the "my_env"
> argument to all the macro functions?
> 
> bool normalize_map_pos(int *x, int *y)    would become
> normalize_map_pos(ENV, X, Y)

Right. But one could also start to use the env argument for more useful
environment than just local storage.

bool normalize_map_pos(struct map *map, int *x, int *y)
normalize_map_pos(PMAP, X, Y)

These are functions that use instance map data rather than getting things
like map.xsize from a global. Thus one can have several different maps or
worlds and do appropriate topology transformations in any of them in the
same game.

If the leading argument is an instance object rather than just a block
of storage, it starts to provide a much more powerful paradigm.

> So would further macro definitions be required to avoid this becoming
> a white elephant?

I think the elaborate scheme was sufficient, certainly more than necessary.

> What about type checking?

The simplest technique for type checking is the obvious one. Write all your
code as functions. Then provide macro forms for those that have an obvious
performance benefit with a switch to run the optimized code or by calling the
function. You then can compile for typechecking, or for performance if you
chose mutual exclusivity, or allow the function to coexist for cases where
you want to also be able to do a function call.

   #ifdef _OPT_MACFN_
   #define  macfn(args)   (<code>)
   #else
   #define  macfn(args)   _macfn(args)  // the real function
   #endif

There are more complex things one can do, but type safety in "C" is not
one of its selling features, and often the ability to do things without
elaborate schemes to workaround the type system is actuallity one of its
advantages.

If one wants more, one needs to ask why, and if it is that important has
one chosen the wrong language.

> On Tue, 20 Jan 2004 20:18:15 -0800 Jason Short wrote:
> 
>>map_pos_to_index, as written, needs temp variables.  If written in macro 
>>form (without any change to the interface) these temp variables must be 
>>made global.  This doesn't work since multiple calls to this function 
>>can be made within one sequence.  For instance
>>
>>   hmap(x, y) = hmap(x1, y1) + foo(); /* hmap calls map_pos_to_index */
>>
>>is not valid C if map_pos_to_index isn't coded properly.
> 
> 
> That's just the situation I ran into with undefined behaviour, so
> I begin to see the use for inlining, at least with map_pos_to_index().

Of course, one can just code it properly as an alternative to inline, or
break the code into segments that do not interact.

It is interesting though that the error never arose in the corecleanup
codebase and all the ingredients are being used. I would still like to see
your code example and macro definitions. Dumping out the preprocessor code
might also reveal just what the problem is as one can then look at the raw
"C" to see what you macro actually did. Jason's explanation just doesn't
make sense without something more like mismatched brackets or the like.

> I'm sorry if you've had this discussion before. I'm pretty sure you
> have, from what I recall when the search engine worked.

One really should document the best practices and reasons for them as a
summary of this sort of discussion.

> Arnstein

Cheers,
RossW
=====