Hi all, thanks for your various responses.

First I must admit I'm not exactly the target audience for the manual - I have 
many years' experience in embedded
systems, and I don't use mspgcc (yet :-). I was looking for guidelines on 
efficient programming for the MSP430
specifically, since the icc430 compiler I'm using doesn't have anything to say 
on this subject. I will use the "try it
both ways and see which way generates the best code" method.

>>> 5. Avoid using global variables of small size - it is a waste of RAM.
>> I don't understand this at all. It's normal for an embedded program to have
>> lots of global variables, and many of these will be bytes. If you mean what
>> you wrote, I disagree and I don't see your point. If you mean something else,
>> perhaps you could make the wording clearer. By "small size" do you mean
>> smaller than a byte?
> Have you looked at the typical embedded programmer's work? 90% of all
> their variable are global. It derives from poor assembly language
> programming, I guess, and the style is carried over to poor C programming.

I see what you mean. I guess programmers who come from an assembly background
would normally make all variables global, since this is common in assembly 
language,
to avoid setting up a stack frame and dealing with messy equates. Assembly 
language
programmers are also likely to use registers for locally calculated results, 
instead of
named variables, when writing in assembler; these temporary variables would be
obvious candidates for 'automatic' storage allocation. Programmers who come from
a high-level language background would be likely to use automatic variables.

Steve has changed this point to:
> Use int instead of char or unsigned char if you want a small integer within
> a function. The code produced will be more efficient, and in most cases
> storage isn't actually wasted.

This no longer addresses the original recommendation, which I would summarise as
"use automatic (local) variables for variables which don't need to be global,
since globals always occupy RAM, but automatic variables only use stack space
while the function is executing".

>>> 6. Avoid using volatiles, unless they are really necessary.
>> No one would ever declare a variable volatile unless there was a good
>> reason to. Are you saying that the compiler can optimise better with
>> variables which aren't volatile? That's normal for any compiler AFAIK.
> Do you have any idea how small a percentage of embedded C programmers
> understand what volatile really means, and where it is needed?

Apparently not! I see you've added two notes explaining volatile very well.

> Lots of people who have used the IAR tools complain about GCC being
> broken, because changes to a global are not sensed. The IAR compiler is
> too dumb to optimise away most things, and you can be sloppy and leave
> out all the volatiles. Most programmers do. At the other extreme, some
> people put volatile in front of everything. That looses you a lot of
> space and speed optimisation with GCC.
> const is another important keyword, especially for embedded systems,
> that people can't really get the hang of. static is even worse.

Point taken! As I said I'm probably not in the target audience for your 
document.

>>> 7. Use int instead of char or unsigned char if you want an 8 bit integer.
>> What? An unsigned char _is_ 8 bits wide. Do you mean the case where you need
>> eight bits _plus sign_? That would be a 9-bit integer.
> If you use a char or unsigned char as a simple small integer, like a
> loop counter, the code will be bigger and slower than using the natural
> integer of the machine - an int. Perhaps I use expand the wording there.

In general I think it's worth _explaining_ the _reasons_ for your
recommendations, as you just did in your response above, instead of just
stating the recommendations.

>>> 18. If you execute
>>>    while ((long) a & 0x80000l);
>>>    the program will hang, unless 'a' is declared volatile. So, do it!
>> That would be a really weird and pointless thing to write, unless you _know_ 
>> that
>> 'a' is volatile and _will_ be changed elsewhere, e.g. by an interrupt 
>> handler. And
>> I think you mean "So, _don't_ do it!"
> Maybe the wording could be better. My intention by "do it" is do declare
> the thing volatile. I should change the order too, to group this with
> other volatile related points.

Yes, and AFAIK the "(long)" is not relevant and just clouds the issue - a
byte or word variable would be clearer.

>>>19. Delay loops are very sophisticated routines.
>>>[...]
>> I don't think delay loops are necessarily "poor programming style" in the
>> context of an embedded system where the MCU clock frequency is known and
>> the device has a (fairly) predictable number of cycles per instruction.
>> If you want to delay by a few microseconds, a loop is the obvious way -
>> you could use a timer (if you have one spare), with or without an
>> interrupt, but this would add lots of overhead, and a very short tight
>> delay would not be achievable. True the maximum execution time is not
>> defined if interrupts are used and interrupts are enabled during the
>> loop, but this is often not the case - it depends on the context in
>> which the code is used. In any case, it's not always necessary to have
>> a maximum limit on the delay time - for a safety timeout, for example,
>> it's just necessary to avoid getting stuck forever in a loop somewhere
>> due to some hardware failure. Instead of saying words to the effect of
>> "don't do it", you could suggest how to trick the compiler into not
>> optimising the loop away into the ether. For example, adding an
>> "asm("nop")" (or mspgcc's equivalent) in the body of the loop might be
>> enough; if there's no recommendable way to do this, perhaps you should
>> add a feature to the compiler to specifically support short delays, e.g.
>> it could generate the loop itself, given a number of CPU cycles for the
>> delay. Short delay loops are common in embedded systems software; some
>> conventions that apply to other programming situations apply less or not
>> at all to an embedded system. Just a suggestion.
> Delay loops are often the only appropriate way to get a small
> predictable delay. There's nothing wrong with that. There is
> something wrong with
>    for (i = 0;  i < 10;  i++);
> GCC will optimise it clean away, unless i is a global volatile. Even
> if i a global volatile, the speed of the loop is totally uncertain.
> The brief_pause routine I show in the manual is what I would call a
> good pause routine - compact and basically predictable. Sure, and
> interrupt could extend it, but these things are usually there to
> impose a well defined minimum delay. Adding _NOP() is wasteful -
> maybe only a little, but still poor style when you are programming
> a 1k chip.

I still disagree with your blanket statement that delay loops are
"poor programming style"; you just said "delay loops are often the
only appropriate way to get a small predictable delay. There's nothing
wrong with that." I would remove the word "predictable" from that
statement though. Delay loops should be avoided if possible, but
avoiding them is _not always possible_ in an embedded environment.

OTOH, I agree with your suggestion to write the loop in assembler.
If you use a for() loop, a future change to the compiler's code
generator could affect how long the loop takes, so writing it in
C is risky. Also, if a programmer is working at the level where he
has to use a delay loop, he should be comfortable writing that
loop in assembler.

Yes adding a nop is wasteful, but it may force the compiler to
leave the loop intact, if the compiler doesn't provide any other
way for the programmer to tell it to leave the loop alone. IMO if
you're so worried about space that one nop is a problem, you
shouldn't be writing your code in C in the first place :-)

Regards

Kris
--
k...@abbey.co.nz
Kris Heidenstrom  Embedded systems design engineer / programmer
Abbey Systems Ltd  Telemetry specialists  Wellington New Zealand
Voice +64 -4 -385-6611  Fax +64-4-385-6848


Reply via email to