[Top][All Lists]

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

Re: [avr-gcc-list] Re: Newbie question

From: David VanHorn
Subject: Re: [avr-gcc-list] Re: Newbie question
Date: Wed, 25 Feb 2009 16:50:32 -0500

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.

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.
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 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.
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.
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.
Given my level of experience in C, I expect that my mistakes will be pretty simple and obvious to a skilled practitioner.  :)
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. 
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.

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]