[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[avr-gcc-list] Re: Newbie question

From: David Brown
Subject: [avr-gcc-list] Re: Newbie question
Date: Thu, 26 Feb 2009 01:01:03 +0100
User-agent: Thunderbird (Windows/20081209)

David VanHorn wrote:

    I'd be surprised if either the original code or your modified code
    actually works - since TOV1 is 2, the conditional can never be true.
    The code you *really* want is:

           if (TIFRx & (1 << TOV1)) {

This section I'd not tested yet, and it may be pretty much irrelevant since the pulses are pretty short, and very unlikely to ever roll the counter.
However, I had intended to trap that condition, so thanks for the catch.

It's up to you to determine the relevance (overflow certainly can happen, and it may give you the wrong measurements if it does - whether such glitches are an issue or not is dependent on the rest of your program).

    You also seem to have forgotten to enable optimisation.  There is no
    point trying to hand-optimise your code if you don't let the
    compiler do its job properly!  Use "-Os" optimisation unless you
have particular reason to do anything else. I had -Os selected in the makefile I was using. Which is another reason that I was surprised to see the pushing and popping of unused registers. It's possible that the optimization was somehow not on, or not working as I noticed that when I commented out major routines in main, so that they never were executed, the compiled code size wasn't changing by more than a few bytes. Apparently it wasn't "seeing" that those blocks were never used. Another alligator that I was going to look into at some point once I drained the swamp a bit.

It's possible that optimisation was on, but it did not appear that way from the generated code. However, that was out of context - occasionally other parts of the code will affect the optimisation of conditionals and other constructs.

    Also enable compiler warnings - at a minimum, use these flags:

           -Wall -Wextra -Wunreachable-code

    Then the compiler would have told you of your error here.

I had -Wall on, but not the others. I'm VERY new to this, and makefiles are yet another thing I have to learn.

I generally have a lot more warnings enabled than just these. It's not quite as good as lint, but gcc's warnings can find a lot of potential errors or stylistic mistakes.

-Wall -Wextra -Wbad-function-cast -Wcast-qual -Wcast-align -Wpointer-arith -Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn -Wredundant-decls -Wnested-externs -Wno-multichar -Winline -Wunreachable-code -Wpadded -Wc++-compat

This covers most of the possible warnings, although there are a few that I omit (like -Wshadow, because it's not a mistake to shadow another name, and it gives far too many false positives). Sometimes I have to omit others to reduce the number of minor warnings, especially if I'm working with code from other sources.

One thing I like about the -Wmissing-prototypes, -Wmissing-declarations and -Wredundant-decls is that it enforces strict policies about extern and static identifiers - a function or variable is *either* global and has an "extern" declaration (in an appropriate header, although the compiler can't check that), *or* it is declared "static". There is no middle ground with functions that are uselessly defined globally (i.e., no "static") but inaccessible (no "extern") - such global code pollutes your global name space and hinders optimisation.

I also just switched today, to using Studio as the IDE in front of GCC, rather than Programmers Notepad.
I'll have to figure out how to set that up in Studio.

My preference is to have a decent Makefile, and run make from a command box - I'm not much into IDEs (perhaps I'm old fashioned). Programmer's Notepad is a better editor than AVR Studio, so I'd stick to that for editing, command-line make for building (and burning to the avr), and AVR Studio for debugging if I can't avoid it.

I do try to create code (asm or C) that produces no warnings. Warnings make me nervous. At the moment, I'm trying to figure out how to get rid of the warning generated by my main not having a return type of Int. There was a "standalone" flag in my previous makefile that took care of this, but again, I need to learn more about makefiles. Cool, I got that figured out. I wonder why it's not the default, but oh well.. Even more amusing.. -Wunreachable-code generated 16 warnings, and none in what I wrote.
A bunch in the LCD library, and one in the delay library.

The disadvantage of setting yourself high standards is that other people fail to live up to them...

    Also, when posting to the mailing list, it is much more helpful if
    you include your actual C code (preferably a minimal compilable
    snippet) and a note of your compiler version and command-line flags.
     That will let others test your code and give you more help.  Some
    people, such as me, *like* the challenge of finding ways to get
    smaller and faster code for a given function.  But I don't like
    trying to dig C code out of the comments of an assembling listing.

I try to post as much relevant detail as needed, and no more.
I can always send the full package to anyone interested.

That's a good aim.

Given my level of experience in C, I expect that my mistakes will be pretty simple and obvious to a skilled practitioner. :)

Since it's the C code that's relevant, post the C code. Don't post a complete program - that's more than we want or need. But the interrupt function source code is highly relevant, together with the definitions of any global variables used.

    It's also not very polite to say that the compiler "irritates you no
    end" - Eric and many others have spend a great deal of time and
    effort writing the compiler and tools so that people like you have
    high quality development tools that are free (as in "free speech")
    and zero cost. The developers are always happy to listen to
    constructive criticism, but your complaints should be well-grounded
    and politely worded.

It may not be what you want to hear, but it's factual. I appreciate the work that has been done, but when I see ISRs compile like this, I'm just left speechless.. The whole point of ISRs is to get in, and get out, as fast as possible. Pushing registers that aren't even used is just pointlessly wasting cycles. If I wrote ASM like that, I'd get laughed at.

This may not be what *you* want to hear, but this is factual - C is not assembly. There are things that can be done in assembly that cannot be done efficiently in C. This is partly due to the way the C language works, and partly due to the way compilers work.

The C language has no concept of jumping between sections of code as one does freely in assembly - it is all within a function. When generating code for a function, the compiler will collect the pre-amble and post-amble for all branches of the function, and will attempt to optimise the re-use of registers and stack frame space. Thus if any branch of the function requires a register to be preserved, it will be stacked at the entry to the function. It is *conceivable* that a compiler could be smarter than this, and only stack such registers before they are used. In practice, I think this would be very hard to achieve and have very little usage outside a few situations like this one.

The C language does not have any concept of interrupt functions - this is an extension to the language that the compiler supports by using a special preamble and postamble. avr-gcc establishes a bare minimum of safety by preserving R0, R1 and the flags, and clearing R1, since many of the instruction generation sequences assume that R1 is always 0, and that R0 is always available as a scratch register. Again, it is possible to write a compiler that does not have these assumptions - and again, the work would be great and the rewards small (it would arguably have been better to have different registers - say R2 and R3 - rather than R0 and R1, but that's easier to see in hindsight). The compiler does a pretty good job of preserving the minimum of registers - I've seen plenty of compilers for various architectures (including some very expensive compilers) that preserve all the compiler's "volatile" register set on entry to an interrupt, whether they are needed or not.

    As a newbie to C, here's a couple of tips in writing your code - use
    plenty of spaces *consistently* in your code, it makes it easier to
    read, and easier to search.  And *always* write your if's as:

           if (xxx) {

           if (xxx) {
           } else {

Sigh.. This is my second program in C.. On the first one, I got beat up for NOT doing the brackets the way I am now.. The nice part about standards is that there are so many to choose from. The forms that you're suggesting were how I learned it from K+R, and they use the "expert" form pretty much interchangably with the one above.

The way you space and indent the brackets is up to you, as long as it is readable and consistent. My point is just that you should have the brackets - omitting them makes it easy to get mixed up as to what is within the conditional's scope, and what is not.

    Do the same with for and while loops.  Once you are a C expert,
    you'll occasionally find the short form more appropriate :

           if (xxx) foo();

And I'm using that form where I consider it to be appropriate, when the contitional is relatively short.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]