On Fri, 24 Sep 2010 17:17:30 +0100, OneStone
wrote:

> Strange place to keep your wine, I used to wear mine way back when in
> the Army to keep warm.

Wow, you used to wear your wine to keep it warm? Did the army know you
were doing that? Was it an especially good vintage? ;-)

-- Paul.
Start a New Thread

Absolutely, chateau plonk '74, we used to sit and contemplate it for a
while until it was well mulled over, then pour it down our demijohns to
keep warm.

Al

On 25/09/2010 2:07 AM, Paul Curtis wrote:
> On Fri, 24 Sep 2010 17:17:30 +0100, OneStone
> wrote:
>
>> Strange place to keep your wine, I used to wear mine way back when in
>> the Army to keep warm.
>
> Wow, you used to wear your wine to keep it warm? Did the army know you
> were doing that? Was it an especially good vintage? ;-)
>
> -- Paul.
> Start a New Thread

On 2010-09-23 23:15, ratishpunnoose wrote:
> I've come across a bug in IAR (lastest version, latest patches).
> I have reduced it down to a simple one-line function.
>
> It appears at every optimization level except "None".
>
> I have emailed John Regehr (see his papers on C compiler correctness)
> and Nigel Jones (http://embeddedgurus.com/stack-overflow/) who concurred
> that this is a compiler bug and not some C language ambiguity. I have
> contacted IAR support/development but they don't acknowledge it is a bug.
>
> I hope that the IAR gentleman (Anders) who posts in this forum can help
> to get this resolved. This is a very common construct and I don't know
> if it affects other IAR target platforms.
>
> I found this since I tend to compile my applications with multiple
> compilers to ensure that they always behave identically.

Hi!

Over the past week we've had this discussion privately, via our support
department, but I'm more that willing to discuss is here as well.

I would base my argumentation on a basic principle (formulate by me):

Any variable that is accessed (read from or written to) as a side
effect outside the rules of the language must be declared "volatile".

Why must this be true? Let's, for the sake of this discussion, assume
the opposite. Let's look at a typical function, below. If the basic
principle would not hold, the access to "my_uart" could trigger any
variable in the system to change, including the non-volatile variable
"value".

// ------------------------
#include
volatile int my_uart;
int value;
int alpha, beta, gamma;
void test()
{
alpha = value;
if (my_uart & 1)
{
beta = value;
}
gamma = value;
}
// ------------------------
However, if the compiler can rely on the principle, the generated code
is allowed to cache the value of "value". When looking at the generated
code of our compiler, you clearly see that it does this:

// ------------------------
test:
MOV.W &value, R15 // Caching
MOV.W R15, &alpha
BIT.W #0x1, &my_uart
JNC ??test_0
MOV.W R15, &beta
??test_0:
MOV.W R15, &gamma
RET
// ------------------------

Clearly, if we could not rely on the basic principle, the generated code
would be far worse, which would affect every single embedded application
out there, with little or no gain.

Lets look at your example, or rather a cut-down version of it. In this
example, we assume that writing to "v" will trigger to hardware to
perform the side-effect of reading the value that it points to.

// ------------------------
unsigned char volatile * volatile v;
void test2(unsigned char ch)
{
v = &ch;
}
// ------------------------
What is going on here? From the language point of view, we have one
assignment to volatile-declared variable "v".

The hardware, however, triggers an extra read from the variable "ch".

The generated code, looks like the following. Note that "ch" is placed
on the stack and the initial value is passed in R12 (according to the
calling-convention used). Clearly, the compiler does not generate a MOV
to store the value of R12 to "ch":s location on the stack.

// ------------------------
test2:
SUB.W #0x2, SP
MOV.W SP, &v
ADD.W #0x2, SP
RET
// ------------------------

The big question here: is this code correct?

My answer is "YES". The variable "ch" is not declared volatile, and it
is not accessed by the program according to the rules of the language,
so the compiler is free to optimize away the store.

So, how should this code be written, if it should work as intended? One
way to do this is:

// ------------------------
unsigned char volatile * volatile v;
void test3(unsigned char ch)
{
volatile unsigned char ch2 = ch;
v = &ch2;
}
// ------------------------

Clearly, the resulting code contains the MOV instruction.

// ------------------------
test3:
SUB.W #0x2, SP
MOV.B R12, 0(SP)
MOV.W SP, &v
ADD.W #0x2, SP
RET
// ------------------------
To conclude, I have in this post presented the basic principle for code
generation regarding volatile and non-volatile variables. I have
demonstrated that the basic principle is sound by explaining that the
consequences would be catastrophic under the assumption that the
principle does not hold. By using the basic principle, I have explained
the code generated by our compiler is correct.

My conclusion is that the IAR compiler generates correct code for all
code examples above.
I have the highest regard for Nigel Jones and John Regeher -- in fact
that I read Nigels blog regularly an have studied John and Eric Eide:s
paper on volatile correctness in great detail. Please forward this reply
to them -- I'm would like to hear their opinions when they are faced
with my side of the story.

-- Anders Lindgren, IAR Systems
--
Disclaimer: Opinions expressed in this posting are strictly my own and
not necessarily those of my employer.

Start a New Thread

I'm not a C expert by a long shot, but IMO Anders has nailed it extremely
well with his code examples.
It also highlights my personal belief in general wrt the whole 'volatile'
issue.
Although not in Linux context per se, the priciple still applies - so I'd
like to excerpt Linus' famous stament here
about volatiles :

{ Also, more importantly, "volatile" is on the wrong _part_ of the whole
system. In C, it's "data" that is volatile, but that is insane. Data isn't
volatile - _accesses_ are volatile. So it may make sense to say "make this
particular _access_ be careful", but not "make all accesses to this data
use some random strategy"..... }

While I personally don't really agree with this statement, I don't really
disagree with it either ;-)
Not sure, but I think what Linus meant was more that - as a concept - it's
(ideally) wrong to mark data as volatile, the _access_ should be marked,
not the data itself.
The posted code example of a volatile pointer referencing a non-volatile
and then a volatile variable speaks for itself IMO.........

Anyhoo, I always assert that volatile doesn't necessarily have anything to
do with *optimisation*.
I was asked this very question about volatile at a job interview a little
while ago, and it got me in hot water (the interviewer asserting that
volatile just means "don't optimise" - which is rubbish IMO, it completely
misses the point that Anders made so well........:-)
The optimisation is a secondary and consequential issue, not a *causal* or
primary issue. That's how I see it.

I'm sure this can/willl lead to re-endless (healthy) debate, but I was
tempted by Anders' poetic post :-)

Best regards,
Kris

On Mon, 27 Sep 2010 10:58:54 +0200, Anders Lindgren
wrote:
> On 2010-09-23 23:15, ratishpunnoose wrote:
>> I've come across a bug in IAR (lastest version, latest patches).
>> I have reduced it down to a simple one-line function.
>>
>> It appears at every optimization level except "None".
>>
>> I have emailed John Regehr (see his papers on C compiler correctness)
>> and Nigel Jones (http://embeddedgurus.com/stack-overflow/) who
concurred
>> that this is a compiler bug and not some C language ambiguity. I have
>> contacted IAR support/development but they don't acknowledge it is a
bug.
>>
>> I hope that the IAR gentleman (Anders) who posts in this forum can help
>> to get this resolved. This is a very common construct and I don't know
>> if it affects other IAR target platforms.
>>
>> I found this since I tend to compile my applications with multiple
>> compilers to ensure that they always behave identically.
>
> Hi!
>
> Over the past week we've had this discussion privately, via our support
> department, but I'm more that willing to discuss is here as well.
>
> I would base my argumentation on a basic principle (formulate by me):
>
> Any variable that is accessed (read from or written to) as a side
> effect outside the rules of the language must be declared "volatile".
>
> Why must this be true? Let's, for the sake of this discussion, assume
> the opposite. Let's look at a typical function, below. If the basic
> principle would not hold, the access to "my_uart" could trigger any
> variable in the system to change, including the non-volatile variable
> "value".
>
> // ------------------------
> #include
> volatile int my_uart;
> int value;
> int alpha, beta, gamma;
> void test()
> {
> alpha = value;
> if (my_uart & 1)
> {
> beta = value;
> }
> gamma = value;
> }
> // ------------------------
> However, if the compiler can rely on the principle, the generated code
> is allowed to cache the value of "value". When looking at the generated
> code of our compiler, you clearly see that it does this:
>
> // ------------------------
> test:
> MOV.W &value, R15 // Caching
> MOV.W R15, &alpha
> BIT.W #0x1, &my_uart
> JNC ??test_0
> MOV.W R15, &beta
> ??test_0:
> MOV.W R15, &gamma
> RET
> // ------------------------
>
> Clearly, if we could not rely on the basic principle, the generated code

> would be far worse, which would affect every single embedded application

> out there, with little or no gain.
>
> Lets look at your example, or rather a cut-down version of it. In this
> example, we assume that writing to "v" will trigger to hardware to
> perform the side-effect of reading the value that it points to.
>
> // ------------------------
> unsigned char volatile * volatile v;
> void test2(unsigned char ch)
> {
> v = &ch;
> }
> // ------------------------
> What is going on here? From the language point of view, we have one
> assignment to volatile-declared variable "v".
>
> The hardware, however, triggers an extra read from the variable "ch".
>
> The generated code, looks like the following. Note that "ch" is placed
> on the stack and the initial value is passed in R12 (according to the
> calling-convention used). Clearly, the compiler does not generate a MOV
> to store the value of R12 to "ch":s location on the stack.
>
> // ------------------------
> test2:
> SUB.W #0x2, SP
> MOV.W SP, &v
> ADD.W #0x2, SP
> RET
> // ------------------------
>
> The big question here: is this code correct?
>
> My answer is "YES". The variable "ch" is not declared volatile, and it
> is not accessed by the program according to the rules of the language,
> so the compiler is free to optimize away the store.
>
> So, how should this code be written, if it should work as intended? One
> way to do this is:
>
> // ------------------------
> unsigned char volatile * volatile v;
> void test3(unsigned char ch)
> {
> volatile unsigned char ch2 = ch;
> v = &ch2;
> }
> // ------------------------
>
> Clearly, the resulting code contains the MOV instruction.
>
> // ------------------------
> test3:
> SUB.W #0x2, SP
> MOV.B R12, 0(SP)
> MOV.W SP, &v
> ADD.W #0x2, SP
> RET
> // ------------------------
> To conclude, I have in this post presented the basic principle for code
> generation regarding volatile and non-volatile variables. I have
> demonstrated that the basic principle is sound by explaining that the
> consequences would be catastrophic under the assumption that the
> principle does not hold. By using the basic principle, I have explained
> the code generated by our compiler is correct.
>
> My conclusion is that the IAR compiler generates correct code for all
> code examples above.
> I have the highest regard for Nigel Jones and John Regeher -- in fact
> that I read Nigels blog regularly an have studied John and Eric Eide:s
> paper on volatile correctness in great detail. Please forward this reply

> to them -- I'm would like to hear their opinions when they are faced
> with my side of the story.
>
> -- Anders Lindgren, IAR Systems
Start a New Thread

Anders, Thank you very much for the detailed reply.
I had some mixed messages from initial support which made me think that they did not quite understand what I was asking and that I wasn't actually in communication with the developers.

Paul, Thanks very much as well.

This has been a learning experience about
what the C language enforces about volatiles and compiler optimization.
For the benefit of all, I'll also paste in a response by Paul Curtis that was offline.

Ratish
-------------------
> ---------------------------
> The function is logically flawed, and so is the reasoning I'm afraid.
> 'volatile' has a very specific meaning in the ISO standard and that is
> to do with ordering of accesses to volatile objects, and what must be
> done at certain points in the abstract machine. Be relieved that you
> are not developing on a processor where there are out-of-order reads
> and writes where you need to include barrier instructions.
>
> You need a different view of 'volatile'. The compiler does not know,
> and cannot know, hardware side-effects. So, for many reasons, your
> example is invalid.
>
> A converse view is the principle of least astonishment--that is, the
> compiler does what the user expects in many cases. This just leads to
> pain when porting code. I've had my fill of "this compiles and runs
> correctly on compiler x but it doesn't on yours" whining. Even when
> you point out to customers their code is in error, they still cling to
> the notion that they know better than the engineers that wrote the
> compiler and eat the ISO C standard with sugar for breakfast each morning.
> ----------------------------
> I think you should write this correctly. I think this should work for
> you:
>
> void really_send_this(unsigned char ch) {
> volatile unsigned char tx = ch; // Note: the assignment MUST
> happen before the ';'
> DMA1SA = (int)&tx; // I'm lazy here
> DMA1SZ = 1;
> start-dma-transfer;
> wait-for-DMA-complete;
> }
>
> Because there is an explicit assignment to tx, that must happen.
> Because it's address is taken, then tx must be stored in memory
> because DMA1SA is a pointer to it and it is volatile and the
> assignment must happen (before the
> ;)
>
> That is, tx must be assigned before its address is taken and the
> address must be stored in DMA1SA.
>
> So, like I said, no bug in the compiler, bug in user code.
>
> The above *must* work. Really. Time to start believing that IAR, when
> they told you there was no bug in the compiler were right. Pretty
> much. Now, if it doesn't, come back and we'll try to reason why it
> doesn't.
>
-----------------
--- In m..., Anders Lindgren wrote:
> // ------------------------
> However, if the compiler can rely on the principle, the generated code
> is allowed to cache the value of "value". When looking at the generated
> code of our compiler, you clearly see that it does this:
>
> // ------------------------
> test:
> MOV.W &value, R15 // Caching
> MOV.W R15, &alpha
> BIT.W #0x1, &my_uart
> JNC ??test_0
> MOV.W R15, &beta
> ??test_0:
> MOV.W R15, &gamma
> RET
> // ------------------------
>
> Clearly, if we could not rely on the basic principle, the generated code
> would be far worse, which would affect every single embedded application
> out there, with little or no gain.
>
> Lets look at your example, or rather a cut-down version of it. In this
> example, we assume that writing to "v" will trigger to hardware to
> perform the side-effect of reading the value that it points to.
>
> // ------------------------
> unsigned char volatile * volatile v;
> void test2(unsigned char ch)
> {
> v = &ch;
> }
> // ------------------------
> What is going on here? From the language point of view, we have one
> assignment to volatile-declared variable "v".
>
> The hardware, however, triggers an extra read from the variable "ch".
>
> The generated code, looks like the following. Note that "ch" is placed
> on the stack and the initial value is passed in R12 (according to the
> calling-convention used). Clearly, the compiler does not generate a MOV
> to store the value of R12 to "ch":s location on the stack.
>
> // ------------------------
> test2:
> SUB.W #0x2, SP
> MOV.W SP, &v
> ADD.W #0x2, SP
> RET
> // ------------------------
>
> The big question here: is this code correct?
>
> My answer is "YES". The variable "ch" is not declared volatile, and it
> is not accessed by the program according to the rules of the language,
> so the compiler is free to optimize away the store.
>
> So, how should this code be written, if it should work as intended? One
> way to do this is:
>
> // ------------------------
> unsigned char volatile * volatile v;
> void test3(unsigned char ch)
> {
> volatile unsigned char ch2 = ch;
> v = &ch2;
> }
> // ------------------------
>
> Clearly, the resulting code contains the MOV instruction.
>
> // ------------------------
> test3:
> SUB.W #0x2, SP
> MOV.B R12, 0(SP)
> MOV.W SP, &v
> ADD.W #0x2, SP
> RET
> // ------------------------
> To conclude, I have in this post presented the basic principle for code
> generation regarding volatile and non-volatile variables. I have
> demonstrated that the basic principle is sound by explaining that the
> consequences would be catastrophic under the assumption that the
> principle does not hold. By using the basic principle, I have explained
> the code generated by our compiler is correct.
>
> My conclusion is that the IAR compiler generates correct code for all
> code examples above.
> I have the highest regard for Nigel Jones and John Regeher -- in fact
> that I read Nigels blog regularly an have studied John and Eric Eide:s
> paper on volatile correctness in great detail. Please forward this reply
> to them -- I'm would like to hear their opinions when they are faced
> with my side of the story.
>
> -- Anders Lindgren, IAR Systems
> --
> Disclaimer: Opinions expressed in this posting are strictly my own and
> not necessarily those of my employer.
>

Start a New Thread

On Mon, 27 Sep 2010 17:24:31 +0100, ratishpunnoose
wrote:

> Anders, Thank you very much for the detailed reply.
> I had some mixed messages from initial support which made me think that
> they did not quite understand what I was asking and that I wasn't
> actually in communication with the developers.
>
> Paul, Thanks very much as well.
>
> This has been a learning experience about
> what the C language enforces about volatiles and compiler optimization.
> For the benefit of all, I'll also paste in a response by Paul Curtis
> that was offline.

Whilst we're slapping each other on the back, I might as well add that the
original post by Ratish is well-argued and coherent. This is so much more
enjoyable than "I problem hardware not working. Wiget must interface
grommet. U send code 2 me now." that you typically find littered about
the MSP430 and LPC2000 Y! groups.

Rgds,

-- Paul.

Start a New Thread

Hi Ratish!

> Anders, Thank you very much for the detailed reply.

You're welcome!
> I had some mixed messages from initial support which made me think that
> they did not quite understand what I was asking and that I wasn't
> actually in communication with the developers.

I typically don't get into the loop until after a few mails have been
exchanged, so this might have been the case. However, I will talk to the
guys at support to see if we can make it clear that a reply has
originated from a developer.

This case is kind of an odd animal -- in all the texts I've ever read
about "volatile", I've never seen a discussion of this scenario, where
an access to a volatile variable results in another access to a variable
that wasn't declared volatile.

-- Anders Lindgren, IAR Systems
--
Disclaimer: Opinions expressed in this posting are strictly my own and
not necessarily those of my employer.

Start a New Thread

Anders,

Thank you for your illuminating discussion.

I can see that "not doing this" would add to the grief of the compiler
writer.... but can I take it a little further forward?

There is a "reasonable user gripe" here - if the optimiser decides it
doesn't need to store ch to memory, and that is a reasonable decision in
this case, then you might reasonably expect it to un-think it's
association of the hole on the stack with ch. At which point ptr=&ch
would become a problem for it to compile, and it would have to do
something - probably wind the optimisation level down one and re-process
the current procedure from the top. I don't know if I call it a compiler
bug, but it does manage to create a pointer to a location which does not,
and will not, contain the value it's told to point to.
so you ask
// ------------------------
test2:
SUB.W #0x2, SP
MOV.W SP, &v
ADD.W #0x2, SP
RET
// ------------------------

The big question here: is this code correct?

Well it clearly doesn't accomplish anything useful, which makes it smell
a little fishy.

You say yes. I'm not so sure. My feeling is that you've compiled the
mov.w on an assumption that the store exists.... then you've keyhole
optimised out the store, as it is clearly never followed by a read, and
is thus unneeded.

Maybe the optimiser should say "the setting of a volatile pointer to
access this location should be considered ( for optimisation purposes )
to be equivalent to a read"

David
Start a New Thread

--- In m..., "david collier" wrote:
>
> Anders,
>
> Thank you for your illuminating discussion.
>
> I can see that "not doing this" would add to the grief of the compiler
> writer.... but can I take it a little further forward?
>
> There is a "reasonable user gripe" here - if the optimiser decides it
> doesn't need to store ch to memory, and that is a reasonable decision in
> this case, then you might reasonably expect it to un-think it's
> association of the hole on the stack with ch. At which point ptr=&ch
> would become a problem for it to compile, and it would have to do
> something - probably wind the optimisation level down one and re-process
> the current procedure from the top. I don't know if I call it a compiler
> bug, but it does manage to create a pointer to a location which does not,
> and will not, contain the value it's told to point to.
> so you ask
> // ------------------------
> test2:
> SUB.W #0x2, SP
> MOV.W SP, &v
> ADD.W #0x2, SP
> RET
> // ------------------------
>
> The big question here: is this code correct?
>
> Well it clearly doesn't accomplish anything useful, which makes it smell
> a little fishy.
>
> You say yes. I'm not so sure. My feeling is that you've compiled the
> mov.w on an assumption that the store exists.... then you've keyhole
> optimised out the store, as it is clearly never followed by a read, and
> is thus unneeded.
>
> Maybe the optimiser should say "the setting of a volatile pointer to
> access this location should be considered ( for optimisation purposes )
> to be equivalent to a read"
>
> David
>

C is a language with a standard (actually, several standards...). The standards are not perfect, but they are not bad, and they form a contract between the programmer and the compiler writers. IAR do their best to follow these standards, but their compiler can only generate code for programs that also follow the standards. The compiler should not be trying to "guess" what the programmer intended to write - it generates code based on what they /did/ write.

The original poster wrote code that did not do what he thought it would - that's the programmer's error, not the compiler's.

It's also worth noting that the poster wrote code that was "on the edge" - it was not as clear or simple as it could be, and at high risk for misinterpretation about the meaning of the code. In this case it was the poster that got it wrong, but it could have been the compiler writer.

Code that is written like that is bad code even if it works.

In this case, the issue is that clear code seldom modifies parameters. It is certainly never good programming to mess around taking addresses of a parameter, and adding "volatile" to the mix is even worse. If you want to muck around with a parameter, make a new local variable and use that. The compiler will happily generate near-optimal code anyway - adding new local variables seldom costs in code space or time.

Start a New Thread

Hi David!

> Thank you for your illuminating discussion.

You're welcome!
> I can see that "not doing this" would add to the grief of the compiler
> writer.... but can I take it a little further forward?
>
> There is a "reasonable user gripe" here - if the optimiser decides it
> doesn't need to store ch to memory, and that is a reasonable decision in
> this case, then you might reasonably expect it to un-think it's
> association of the hole on the stack with ch. At which point ptr=&ch
> would become a problem for it to compile, and it would have to do
> something - probably wind the optimisation level down one and re-process
> the current procedure from the top. I don't know if I call it a compiler
> bug, but it does manage to create a pointer to a location which does not,
> and will not, contain the value it's told to point to.

When it comes to the hole on the stack, it might be possible to optimize
it away as well. The IAR compiler typically doesn't do stack frame
trimming, as we commit to the stack frame relative early in the code
generation phase (which has other advantages). However, even if it
would, I'm not 100% it would be legal to optimize away the location on
the stack.

For example, the volatile register I'm accessing might compare the value
with the address of other variables. If the locations of ch would be
optimized away, then the program might behave differently.

To conclude, the location and the value of a variable are two different
things that are governed by different set of rules. It might be legal to
optimize one but not the other.

-- Anders Lindgren, IAR Systems
--
Disclaimer: Opinions expressed in this posting are strictly my own and
not necessarily those of my employer.

Start a New Thread