[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 2.0.0.19 (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) {
foo();
}
or
if (xxx) {
foo();
} else {
bar();
}
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.
Re: [avr-gcc-list] Newbie question, Vincent Trouilliez, 2009/02/25
Re: [avr-gcc-list] Newbie question, David Kelly, 2009/02/25
Re: [avr-gcc-list] Newbie question, Georg-Johann Lay, 2009/02/25