[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: Wed, 25 Feb 2009 22:08:08 +0100
User-agent: Thunderbird (Windows/20081209)

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

        if (xxx) {
        } else {

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.



reply via email to

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