Macro Reexecution

[meeok]

I have noticed an area of potential performance and stability improvements. It would have a wide effect so I would like some feedback before committing.

There are a few macros defined in dryos.h.
#define MIN(a,b) ((a) < (b) ? (a) : (b))
#define MAX(a,b) ((a) > (b) ? (a) : (b))
#define COERCE(x,lo,hi) MAX(MIN((x),(hi)),(lo))
#define ABS(a) ((a) > 0 ? (a) : -(a))
#define SGN(a) ((a) > 0 ? 1 : (a) < 0 ? -1 : 0)
#define SGNX(a) ((a) > 0 ? 1 : -1)
On principle they are a good idea: they are significantly more legible than their expanded counterparts.

They have some drawbacks:

• MAX(a++,b) or MAX(do_something(),2) increments "a" twice or has side effects - something that may not be obvious.
• MAX(a+b*c+MEMX(0xFF),2) executes that calculation twice reducing performance.

The compiler certainly optimises where possible but there are limitations. For example MEMX is volatile.

I suggest enhancing the macros. For example:
     #define max(a,b) \
       ({ typeof (a) _a = (a); \
           typeof (b) _b = (b); \
         _a > _b ? _a : _b; }) Source
In terms of (pseudo) assembly instructions this would require: Compare, Jump, Neg, [label].
Alternatively if a function was used: [marshal], Call, Compare, Return, Neg, Return, [unmarshal]

This change would not require other code to be modified, only the macros.

What do you think?

[a1ex]

+1, sounds like a very good idea.

[eduperez]

I do not want to sound condescending, but side effects of those macros are well known; any developer should be aware of those effects, and code around them when necessary. Besides, I am not very versed on compilers, and do not know how exactly is translated into assembler the version you propose, but looks like it could be producing more code than strictly necessary.

Just my two cents...

[meeok]

I am having issues with nested macros which cause multiple declarations of temporary variables.
eg: COERCE(COERCE(x,0,10),0,xlim)
A few modifications will be required to existing code but I don't see the lack of nesting as a big issue. What does everyone think of this?

There was a warning generated by mixing signed and unsigned types. This is clear with explicit default types: MAX((unsigned int)a,(int)3)
The following macro uses the correct type in this situation:
#define MAX(a,b) ({ typeof ((a)+(b)) _a = (a); \
  typeof ((a)+(b)) _b = (b); \
  _a > _b ? _a : _b; })

@eduperez: It is producing more code on the source level but not on the assembly level.
I compiled and measured file sizes using only the above macro:
New   Orig   Delta   %   
368544   369248   704   0.19%   ../magic-lantern/platform/1100D.105/autoexec.bin
333856   334688   832   0.25%   ../magic-lantern/platform/500D.111/autoexec.bin
318496   319264   768   0.24%   ../magic-lantern/platform/50D.109/autoexec.bin
349712   350928   1216   0.35%   ../magic-lantern/platform/550D.109/autoexec.bin
345760   346592   832   0.24%   ../magic-lantern/platform/5D2.212/autoexec.bin
352480   353760   1280   0.36%   ../magic-lantern/platform/600D.102/autoexec.bin
350144   350912   768   0.22%   ../magic-lantern/platform/60D.111/autoexec.bin
2469892   2478084   8192   0.33%   ../magic-lantern/platform/all/autoexec.bin

Each instruction is 4bytes so we are looking at saving 200 to 300 instructions per build. It may not seem like much but if running in a loop it could be significant.