Hi David,
criticism is fine....I'll explain my view.
2006/7/3, David Brown <[email protected]>:
I don't mean to sound patronising, but I don't think you are aware of what
compilers do, how they work, how "volatile" works, or how to write good C
code for embedded systems. I'm afraid your post is almost completely
incorrect - I hope you take my reply here as constructive critisism to aid
your better understanding. Thinking about these things is good, and
discussing them will refine that thinking.
> knowing how a compiler works in the most unusual situations is always
> interesting...and this discussion is no different.
That's not only true, but essential to embedded programming. Knowing your
compiler is vital, including knowing its shortcomings.
up to now, we agreee....
> however, a compiler is just a machine with a lot of rules, so you
> cannot expect it to be "clever" anytime, even if correct code is a
> necessity and efficiency is only a plus;
You can certainly expect the compiler to be "clever" - the people who wrote
it were clever people, and I have often looked at generated assembly code
and admired neat tricks I hadn't thought of myself. The aim of the
compiler's optimiser is to give you the best generated assembly that has the
same effect as the code you wrote, regardless of the style in which you
wrote it.
[omissis...]
in that sense, compilers are nowadays definitely clever, especially gcc.
What I meant is that you can always find a way to complicate the input
to any compiler, so that you'll need still another rule to have it
take the right decision....do you get the difference? English is not
my mother language, I hope I'm writing what I'm thinking.
> but in this case, reading the C source code, I honestly can't
> understand for sure what it meant....if I were to translate manually
> that code into asm, I would have asked a clarification to the
> programmer: did you want to sample twice the port and then add these
> two samples ? (say for making an average) or did you want two sample
> the port once and double that reading ?
> IMHO the answer wasn't clear, even if I considered all the volatile
qualifiers;
In that case, you simply don't have the experiance to understand volatiles
properly. The code was perfectly clear - the port should be sampled twice,
and it's sum taken. If Grant had wanted to sample it once and double it, he
would have writen "two = 2 * P6IN;". Of course, in real code the function
and variable names would have given an indication of *why* he was doing
this, but the code itself is perfectly clear.
I do know what volatile means and my first thought was that he wanted
to sample twice; nevertheless, my experience is that way of writing
code saves 1 line in the source file, but may potentially cause
headaches in debugging...it's not a good practice to access two
devices in one line...there's no drawback in splitting it a little
more and sometimes compilers have more chances to apply the
optimization rules
Oh, except when you're taking part in the IOCCC :-)
> so I wouldn't try to add another complicated rule to gcc, but I would
> rather write a better C code...an unambiguous one that the "simple"
> rules of any compiler can understand.
>
A C compiler is required to generate correct code for all legal input - it
would be a poor compiler indeed if it required specific simplistic styles to
generate correct code. What has been uncovered here are two issues - one is
inefficient code (which will be fixed if it can easily be done), and the
other is incorrect code (which people are already working on to find a fix).
It may be that in the meantime, people will need to use a workaround to
avoid the bug - but that's an exceptional circumstance, not a solution.
I agree
> for sampling twice: x = P6IN; x += P6IN;
Why do you thing that is better than "x = P6IN + P6IN" ? To the compiler,
this is exactly the same (assuming "x" is a local variable), and it
generates exactly the same output. To the writer (and reader) of the C
code, it is more verbose without adding any useful information - and
therefore worse code.
in the first post, x was not a local variable
> By the way, I think it's also better to change your initial example:
>
> unsigned two;
> void foo(void)
> {
> two = P6IN + P6IN;
> }
>
> into:
>
> unsigned two;
>
> void foo(void)
> unsigned int tmp;
> {
> tmp = P6IN + P6IN; // <----- tmp = 2 * P6IN; // <----- tmp =
> P6IN; tmp+= P6IN;
> two = tmp;
> }
>
You may think that, but no one will agree with you. There is no benifit in
introducing an extra temporary variable here - it simply makes the code more
verbose and thus less readable. You are writing in C, using a compiler -
you are not writing everything out explicitly in assembler. It is up to the
compiler to figure out what extra temporary variables it needs - in fact,
the compiler will probably internally use exactly the code you have written
out explicitly. And unless you are specifically compiling with no
optimisation, the code generated is identical with or without the "tmp"
variable.
Apart from the fact that the compiler generates two completely
different codes if you use the temp variable or not (try it!),
I don't know what others would say...but I don't think that is
"verbose"; on the contrary, it's more readable because you may have
that memory variable defined far away (in terms of code lines) and you
may forget the type....and....and......it's error prone.
And it's no more efficient then using explicitely the temp var.
> or even better a function. it's more efficient (and easier to compile)
> if you make the calculations through local variables.
>
It is hard to split up a single-line function into smaller functions.
first of all, if in that function there was really just that
instruction (and I don't believe so), you would be wasting stack,
making calls, and so on.....which is the opposite of you should do
with embedded systems. In PC your CPU has the pipelines doing nops for
the most part of the time, so you wouldn't affect the performances
significantly, but with an MSP430 you should keep an eye on not
wasting anything.
Note also that every extra instruction means extra current that you're
drawing from the battery....
if you really want that single instruction as a function by itself and
you want it to write directly in the global variable, then you should
at least declare it naked.
And as for being easier to compile - I try to be nice to my computer, but I
refuse to structure my code differently just to save the computer a couple
of microsecond's effort during compilation.
I didn't care about the compilation time....if you got there, it means
my English is really bad...sorry everyone!
"easier" meant that you help the compiler to understand when to use a
register instead of a direct memory access
Local variables are, in general, more efficient than global variables - but
they are not more efficient than calculations carried out internally by the
compiler using registers. Add variables when they make sense in your code,
and make the logic of your code clearer - don't do it for the compiler's
sake.
when you are writing firmware, using explicitely a local variable
means that you _prefer_ it to be a register !!! And if I happen to see
that I used too many local variables (which is quite difficult with 16
registers), so that the compiler is forced to allocate some of them in
memory, then I try to rearrange my code.
Efficiency comes also at the cost of coding time, if efficiency is your aim.
Are you sure you know everything about embedded systems ?
R.
[omissis]
