[avr-gcc-list] Re: Newbie question

[David Brown]

Weddington, Eric wrote:
> > -----Original Message----- From: 
> > address@hidden 
> > [mailto:address@hidden org]
> > On Behalf Of David VanHorn Sent: Wednesday, February 25, 2009 9:05
> > AM To: address@hidden Subject: [avr-gcc-list] Newbie
> > question
<snip>
> > 467:         if ((TIFR & (1<< TOV1))==1) // If we rolled T0 
> > +00000253:   B788        IN        R24,0x38       In from I/O
> > location +00000254:   2799        CLR       R25            Clear
> > Register +00000255:   7084        ANDI      R24,0x04       Logical
> > AND with immediate +00000256:   7090        ANDI      R25,0x00
> > Logical AND with immediate +00000257:   9701        SBIW
> > R24,0x01       Subtract immediate from word +00000258:   F421
> > BRNE      PC+0x05        Branch if not equal
>
> Hmm. It seems the compiler is treating this expression as a 16-bit
> value. It could be because of the comparison with the constant '1',
> which gets automatically treated like an int. Can you see if
> typecasting will help?:
>
> if ((TIFR & (uint8_t)(1<<TOV1)) == (uint8_t)1)
>
> If it works, it should save 2 instructions.

(This reply is really to David VanHorn, rather than Eric.)

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)) {

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.  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.


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.

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.


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) {
                foo();
        }

or
        if (xxx) {
                foo();
        } else {
                bar();
        }

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();

But until you consider yourself an expert, using the brackets will make 
your code clearer, your debugging easier, and give you fewer headaches 
during code maintenance.


If you are still looking for suggestions to improve the code, post the C 
code (including definitions of any globals involved), and I will happily 
give more suggestions.

mvh.,

David