[avr-gcc-list] Re: C vs. assembly performance

[David Brown]

Vincent Trouilliez wrote:
> Thanks all for the input on switch statement, I appreciate.
>
>
> On Sun, 01 Mar 2009 14:37:57 +0100
> David Brown <address@hidden> wrote:
>
> > Apart from that, I've a couple of other comments on your code.  The 
> > variable names "tmp16", "tmp32" and "tmpS16" are truly awful.
>
> Oh :-) I was just trying to give their meaningful names.
> They are just that: temporary/scratch integers used only to
> process the raw data byte before it can be sent to the function
> var_format_S16() which will convert it to ASCII.
>
> > It is also (usually) best to declare such temporaries in as small a block as 
> > possible.  Thus they should not be at the start of the function, but 
> > instead make your cases like this:
> >
> > {// (N * 0.75) - 40     DB41            -40 to +150 °C
> >         int16_t temp = (int16_t)(KLineFrameM1[41]) * 3 / 4 - 40;
> >         var_format_S16(buff, temp, 0);
> >         break;
> > }
> >         
>
> Hmm, actually I didn't even know one could declare local variables
> inanother place than the start of the function, I take note.

Even with older C standards, you can declare variables at the beginning 
of any block.  With C99, you can declare them almost anywhere.

> > That will give clearer code
>
> I find it just adds bloat and confusion: I have to declare the variable
> 25 times every 5 lines of code, that's overwheleming, plus it make the
> statement harder to parse (my brain is easily overloaded ;-), and also,
> it is a little confusing/harder to understand: if the variable is
> declared 25 times, one could think: "well if it's a diffeent variable
> everytime, there must be something special/obscure that makes it
> impossible to use the same temp/scratch integer. This leaves a false
> impression that the processing of the 25 individual data bytes is more
> complex than it really is, and that a single temp integer could
> perfectly be used.

It doesn't add bloat to your source code - it's an extra type 
declaration at the start of each use of your temporaries.  You can 
change lines such as "tmp32 = " into "int32_t tmp = ", which is hardly 
"bloat".  And it will reduce bloat in the generated code.

Each clause in your switch uses the variables in a slightly different 
way - there is no benefit to your source code in trying to manually 
force the compiler to re-use these variables.  They are different uses, 
so you can declare them individually.

Also remember that the smaller the scope of an identifier, the shorter 
the name you can use while keeping the code equally clear.  A variable 
called "tmp16S" is a meaningless name over a span of 100 lines - a 
variable called "t" is perfectly clear over a two-line lifespan.

> Well that's just my view as a beginner.. maybe with time I will
> progressively start to see things the way you do ! ;-)
>
> > Also avoid local automatic constant arrays (like "Yes" in your example)
>
> Yes I don't like them, but since they just that, "local", I didn't want
> to declare them global, to avoid throwing bloat and confusion among all
> the variables declaration which are truly/genuinely global.

That's what "static" within a function is for (well, one of its uses). 
Being "static" it exists for the lifetime of the program, being locally 
declared within a function limits its scope to that function.  So 
although it *exists* globally, it is only *visible* locally.

> > - they must be built on the stack each time the function is called, 
> > whether they are used or not.
>
> Yes, I was conscious of this but it was trade-off between wasting time
> initialising them each time I enter the function.. and degrading code
> clarity by defining local variables among truly global ones. Doesn't
> make me happy.. like most compromises I suppose ! ;-)

A good compromise makes everyone equally unhappy.

> >  By making these "static", they will have 
> > local names but be built once at the start of the program.
>
> He ? Wow, so I can get the best of both worlds, just be qualifying them
> "static" ?! Yahoo !!!!! :o)
> Thanks David, you just made me and my code that little bit better ! :-)
>
> >  If you are short on ram space, you might also want to make them PROGMEM.
>
> Yeah I do do this at times, even though I have ample available RAM..
> just because I don't like wasting things, be it MCU resources or money
> or anything, it's just a general state of mind ;-)

That's a good attitude in general, but for small strings it's often 
easier to be a little wasteful.  On of the weak points of the AVR is 
that you can't mix pointers to ram and pointers to flash, so using data 
in flash is a bit of a pain.

> The "Yes" and "No" strings I accepted to waste RAM on because they are
> very short. But other strings in the program, which are 20 characters
> long and used in several times, I declared them global to save memory.

Make sure they are PROGMEM - that's what saves ram space, not the global 
or local declarations.

> Thanks for you comment, made my program better and I, a little bit
> smarter/skilled ;-)
>
>
> --
> Vince