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