Reply by Ratish Punnoose July 29, 20112011-07-29
--- In m..., Harri Haataja wrote:
>>On 14 July 2011 20:33, Ratish Punnoose wrote:
>> UPDATE:
>> I had reported a compiler bug to IAR after this thread conversation in
>> March
>> (http://tech.groups.yahoo.com/group/msp430/message/47777
>> )
>> IAR's update this week to version 5.30.1 of the compiler fixed it.
>

> So, is there any idea what was wrong and how was it fixed?
> ------------------

This specific bug does not appear anymore. I am not sure what "general class of bugs" was eliminated. The Release Notes don't even mention anything about any bugs being fixed. I had received a private email from IAR support about a month ago that my bug report (front-line support called it a glitch) would be fixed in this release.

It would be helpful if the IAR release notes contain details of previous bugs in the compiler that now have been fixed. But they don't.

Thanks again All.
Ratish

Beginning Microcontrollers with the MSP430

Reply by Harri Haataja July 20, 20112011-07-20
On 14 July 2011 20:33, Ratish Punnoose wrote:
> UPDATE:
> I had reported a compiler bug to IAR after this thread conversation in
> March
> (http://tech.groups.yahoo.com/group/msp430/message/47777
> )
> IAR's update this week to version 5.30.1 of the compiler fixed it.

So, is there any idea what was wrong and how was it fixed?

--
I appear to be temporarily using gmail's horrible interface. I
apologise for any failure in my part in trying to make it do the right
thing with post formatting.
Reply by Ratish Punnoose July 20, 20112011-07-20
UPDATE:
I had reported a compiler bug to IAR after this thread conversation in
March
(http://tech.groups.yahoo.com/group/msp430/message/47777
)
IAR's update this week to version 5.30.1 of the compiler fixed it.

Thanks to all who responded.
Ratish

--- In m..., "Paul Curtis" wrote:
>
> Hi,
>
> > People from IAR are reading here too. They should be able to verify
> > your problem. I do not use IAR, so I can not...
> >
> > I still in doubt that this is a compiler fault - because the fault
you
> > are reporting would hurt anybody else using the IAR. "while (P1IN);"
or
> > similar is a very common use of sfr's. Maybe it is a problem with
the
> > compiler setup.
>
> I think this is a bug, and I believe there is a release note for the
IAR
> compiler that shows that this is fixed.
>
> Using data flow the compiler is able to prove that the while is not
iterated
> if P1IN is zero:
>
> Original:
>
> static int log_calls(int num_iters)
> {
> puts("\nlogging");
> return (num_iters-1);
> }
>
> void myfunc(void)
> {
> signed int remaining_calls = 1;
>
> while (P1IN == 0 && (remaining_calls = log_calls(remaining_calls)) >
0)
> ;
> }
>
> We know that the first call is log_calls(1) by data flow analysis and
that
> log_calls will return zero by data flow analysis and as a side-effect
will
> print something.
>
> Hence, this can be transformed from what we know to:
>
> void myfunc(void)
> {
> if (P1IN == 0)
> puts("\nlogging");
> (void)P1IN; // as P1IN volatile and requires testing in the while
> }
>
> That is the effect of this function. No more, no less.
>
> The question is now what "puts" does. If it's
>
> void puts(const char *) {}
>
> Then the IAR compiler is correct. However, from the description of
this and
> the analysis the OP has done and provided, it smells like a compiler
issue.
>
> -- Paul.
>



Reply by Ratish Punnoose March 7, 20112011-03-07
Hi,
I'm using version 5.20.2 (5.20.2.50216) which is the latest version with all patches.
Here are the other settings:

Device: MSP430F1611, normal DLIB,
Optimization: High (for speed), all optimizations enabled.

I also found that it is the loop unrolling optimization
that may be causing the problem. Disabling that fixes it.

Ratish

--- In m..., "david_mcminn" wrote:
> [snip]
>
> I tried your original code and the refactored code using IAR (compiler version 5.10.1.50144) and I could never get it to generate the assembly with the call to puts() removed.
>
> This was using a realease build, fully optimised for size with a normal DLIB, for an F149. What project settings did you have?
>

Reply by David Brown March 4, 20112011-03-04
On 04/03/2011 10:41, Matthias Weingart wrote:
> Thanks David for the indoctrination. ;-)
> I just want to give hints how to find the problem.
>

Yes, I know that. I've just had to work with too much ugly code
recently, so took the opportunity for a rant.

Of course, what I wrote is still true :-)

mvh.,

David
> M.
>
> David Brown:
>
>> On 03/03/2011 16:25, Matthias Weingart wrote:
>>> "ratishpunnoose":
>>>
>>>> while(P1IN==0)
>>>
>>> What will happen when you write it this way:
>>>
>>> while(0==P1IN)
>>>
>>> M.
>>> There should be no difference in the generated code. Of course, since
>> it looks like this is a bug in the compiler, it could be that the bug is
>> only triggered with one format.
>>
>>
>>
>> I think writing things like "while (0 == P1IN)" is bad coding - it reads
>> backwards, and is therefore less clear than the logical "while (P1IN =>> 0)" formulation.
>>
>> If your compiler doesn't warn you when you accidentally write "while
>> (P1IN = 0)", then you need to learn how to use your compiler properly
>> (for gcc, use the "-Wall" flag). If your compiler is incapable of
>> giving such useful warnings, then you need to either get a decent
>> compiler, or use some sort of lint program. Poor compiler warnings are
>> no excuse for writing illogical code.
>>
>> Remember, MISRA rules are 50% code, 50% meaningless, and 50% bad. This
>> convention is one of the bad ones.
>>
>>
>>
>>
>> Please learn to use spaces correctly - they make code /much/ easier to
>> read. Far and away the most commonly accepted spacing convention for
>> this expression is "while (P1IN == 0)".
>>
>>
>>
Reply by Paul Curtis March 4, 20112011-03-04
Hi,

> People from IAR are reading here too. They should be able to verify
> your problem. I do not use IAR, so I can not...
>
> I still in doubt that this is a compiler fault - because the fault you
> are reporting would hurt anybody else using the IAR. "while (P1IN);" or
> similar is a very common use of sfr's. Maybe it is a problem with the
> compiler setup.

I think this is a bug, and I believe there is a release note for the IAR
compiler that shows that this is fixed.

Using data flow the compiler is able to prove that the while is not iterated
if P1IN is zero:

Original:

static int log_calls(int num_iters)
{
puts("\nlogging");
return (num_iters-1);
}

void myfunc(void)
{
signed int remaining_calls = 1;

while (P1IN == 0 && (remaining_calls = log_calls(remaining_calls)) > 0)
;
}

We know that the first call is log_calls(1) by data flow analysis and that
log_calls will return zero by data flow analysis and as a side-effect will
print something.

Hence, this can be transformed from what we know to:

void myfunc(void)
{
if (P1IN == 0)
puts("\nlogging");
(void)P1IN; // as P1IN volatile and requires testing in the while
}

That is the effect of this function. No more, no less.

The question is now what "puts" does. If it's

void puts(const char *) {}

Then the IAR compiler is correct. However, from the description of this and
the analysis the OP has done and provided, it smells like a compiler issue.

-- Paul.

Reply by Matthias Weingart March 4, 20112011-03-04
Thanks David for the indoctrination. ;-)
I just want to give hints how to find the problem.

M.

David Brown :

> On 03/03/2011 16:25, Matthias Weingart wrote:
>> "ratishpunnoose":
>>
>>> while(P1IN==0)
>>
>> What will happen when you write it this way:
>>
>> while(0==P1IN)
>>
>> M.
>> There should be no difference in the generated code. Of course, since
> it looks like this is a bug in the compiler, it could be that the bug is
> only triggered with one format.
>
> I think writing things like "while (0 == P1IN)" is bad coding - it reads
> backwards, and is therefore less clear than the logical "while (P1IN => 0)" formulation.
>
> If your compiler doesn't warn you when you accidentally write "while
> (P1IN = 0)", then you need to learn how to use your compiler properly
> (for gcc, use the "-Wall" flag). If your compiler is incapable of
> giving such useful warnings, then you need to either get a decent
> compiler, or use some sort of lint program. Poor compiler warnings are
> no excuse for writing illogical code.
>
> Remember, MISRA rules are 50% code, 50% meaningless, and 50% bad. This
> convention is one of the bad ones.
>
>
> Please learn to use spaces correctly - they make code /much/ easier to
> read. Far and away the most commonly accepted spacing convention for
> this expression is "while (P1IN == 0)".
>
>
>
Reply by Matthias Weingart March 4, 20112011-03-04
People from IAR are reading here too. They should be able to verify your
problem. I do not use IAR, so I can not...

I still in doubt that this is a compiler fault - because the fault you are
reporting would hurt anybody else using the IAR. "while (P1IN);" or similar
is a very common use of sfr's. Maybe it is a problem with the compiler
setup.
Are you really sure that P1IN is the P1IN-sfr and not just a macro?
Try to compile your code with the preprocessor only. The preprocessor will
only do a macro expansion and you get C code without macros as a result.
(If preprocessing is not possible with IAR then read here:
http://en.wikipedia.org/wiki/C_preprocessor and use the GNU CPP)

Are you sure you are using the include files from your "latest version IAR-
compiler"? Or do you use the include files from a different compiler?
(Make a error in the msp430xxx.h where P1IN is defined - if your project is
still compiling with no errors you are using the wrong include files).

M.

"ratishpunnoose" :

>
> There is no difference in the generated code as David said.
>
> This occurrence is very specific to checking a volatile
> as part of the while condition.
> It could be while(volatile_reg),
> while(volatile_reg & 0x03),
> while(volatile_reg == y) etc.
>
> However, if the while condition is based on an ordinary variable instead
> of a volatile, the sequence is compiled correctly.
>
> Ratish
>
> --- In m..., David Brown wrote:
>>
>> On 03/03/2011 16:25, Matthias Weingart wrote:
>> > "ratishpunnoose":
>> >
>> >> while(P1IN==0)
>> >
>> > What will happen when you write it this way:
>> >
>> > while(0==P1IN)
>> >
>> > M.
>> >
>>
>> There should be no difference in the generated code. Of course, since
>> it looks like this is a bug in the compiler, it could be that the bug
>> is only triggered with one format.
>>
>>
Reply by david_mcminn March 4, 20112011-03-04
Hi,

> > I don't think we have missed anything - I have tested your code
> > with several gcc compilers (new and old msp430 compilers, several
> > AVR compilers, and gcc for the ColdFire - I have a batch file
> > setup that I use for this sort of testing). They all agree with
> > our interpretation of the code.
> >
> > However, I suggest you check it with IAR. Either it /is/ a bug,
> > or they know more about the interpretation of horrible C code
> > than we do :-)
> > Thanks very much David, I'll do that. I refactored the code into
> the following easy to understand snippet and it exhibits the same
> problem.

[snip]

I tried your original code and the refactored code using IAR (compiler version 5.10.1.50144) and I could never get it to generate the assembly with the call to puts() removed.

This was using a realease build, fully optimised for size with a normal DLIB, for an F149. What project settings did you have?

Reply by ratishpunnoose March 3, 20112011-03-03
There is no difference in the generated code as David said.

This occurrence is very specific to checking a volatile
as part of the while condition.
It could be while(volatile_reg),
while(volatile_reg & 0x03),
while(volatile_reg == y) etc.

However, if the while condition is based on an ordinary variable instead of a volatile, the sequence is compiled correctly.

Ratish

--- In m..., David Brown wrote:
>
> On 03/03/2011 16:25, Matthias Weingart wrote:
> > "ratishpunnoose":
> >
> >> while(P1IN==0)
> >
> > What will happen when you write it this way:
> >
> > while(0==P1IN)
> >
> > M.
> > There should be no difference in the generated code. Of course, since
> it looks like this is a bug in the compiler, it could be that the bug is
> only triggered with one format.