View previous topic :: View next topic |
Author |
Message |
EdWaugh
Joined: 07 Dec 2004 Posts: 127 Location: Southampton, UK
|
Change in compiler behaviour between v4.114 and v4.116 |
Posted: Tue Jan 04, 2011 10:12 am |
|
|
Hi all,
I’m having a problem with v4.116, it is behaving differently (and giving the wrong result) for some code that worked fine under v4.114 (and previous versions). I’m not sure if the C I have written is strictly legal tho (and there are certainly other ways to achieve the same result).
What I am doing is referencing a byte inside a 32 bit value (data), I wanted something portable and I prefer to avoid doing math on pointers so that’s why I use the [] referencing to increment through the values.
Code: | v4.114
.................... uint8_t a = ((uint8_t*)&data)[0];
05F30: MOVLW 0B
05F32: MOVWF FEA
05F34: MOVLW 8C
05F36: MOVWF FE9
05F38: MOVFF FEF,B94
05F3C: MOVLB B
.................... uint8_t a = make8(data, 0);
05F30: MOVLB B
05F32: MOVFF B8C,B94
v4.116
.................... uint8_t a = ((uint8_t*)&data)[0];
05F30: MOVLW 0B
05F32: MOVLB B
05F34: MOVWF x96
05F36: MOVLW 8C
05F38: MOVWF FE9
05F3A: MOVFF B96,FEA
05F3E: MOVFF FEF,B94
.................... uint8_t a = make8(data, 0);
05F30: MOVLB B
05F32: MOVFF B8C,B94 |
The make8() function is much more efficient either way though so I should just change to that.
Any suggestions, especially on the validity of my C statement would be most welcome.
Thanks
ed |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Tue Jan 04, 2011 12:50 pm |
|
|
Post a compilable test program that displays the results with a printf
statement. Then we can drop it into MPLAB and run it in MPSIM
and see if it works.
The test program needs to have the #include for the PIC, #fuses,
#use delay, typedef statement(s), variable declarations, main(), etc. |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Tue Jan 04, 2011 1:17 pm |
|
|
Your code is valid, but ineffective under CCS C, as you already mentioned.
The V4.116 code is mainly introducing an unneccesary step. But the result should be correct, as least it is in my test with MPLAB SIM. If you also check other indices than 0, you realize that V4.116 is getting completely confused, blowing up the code size by 120%. But the result is still correct.
Code: | V4.112:
.................... a = ((unsigned int8*)&data)[1];
0870: MOVLW 02
0872: MOVWF FEA
0874: MOVLW 1B
0876: MOVWF FE9
0878: MOVFF FEF,219
V4.116:
.................... a = ((unsigned int8*)&data)[1];
0874: MOVLW 02
0876: MOVWF x24
0878: MOVLW 1A
087A: MOVWF x23
087C: MOVLW 01
087E: ADDWF x23,W
0880: MOVWF FE9
0882: MOVLW 00
0884: ADDWFC x24,W
0886: MOVWF FEA
0888: MOVFF FEF,219 |
Which surprizes will be brought by V4.117? |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19539
|
|
Posted: Tue Jan 04, 2011 4:05 pm |
|
|
Or just use a union, which is portable to non CCS compilers, and as efficient as make8...
Best Wishes |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Wed Jan 05, 2011 3:40 am |
|
|
A union will be only efficient, if the source data type is already defined as a union, otherwise either a copy operation or a typecast by pointer, similar to the shown example would be necessary.
It would be interesting to know, if the shown long-winded interpretation of the typecast by pointer is a particluar CCS C problem, or if it can be found with other PIC compilers as well? |
|
|
EdWaugh
Joined: 07 Dec 2004 Posts: 127 Location: Southampton, UK
|
|
Posted: Wed Jan 05, 2011 5:36 am |
|
|
Hi guys,
Thanks for the responses, it's nice to know I'm not completely crazy.
Shouldn't the compiler identify this operation for what it is? Why should the ASM be any longer than for the make8() function? All the inputs are constant.
I also emailed this to CCS as a bug, I'll report back if I get a response.
Cheers
Ed |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Wed Jan 05, 2011 6:49 am |
|
|
Quote: | Shouldn't the compiler identify this operation for what it is? |
It would be fine, indeed. Apparently, the ability of CCS C to understand the meaning of some popular C constructs instead of decoding them literally, step-by-step, is somewhat limited. I guess, you'll find a similar behaviour with other embedded C compilers, too. Their big brothers, compilers like Microsoft or Borland C++ however don't have it. |
|
|
EdWaugh
Joined: 07 Dec 2004 Posts: 127 Location: Southampton, UK
|
|
Posted: Wed Jan 05, 2011 7:31 am |
|
|
Hi,
I was working a bit more with my code and realised that this change is making it not work in various other places as well. I don't really want to go around fixing legal C code that worked on previous versions. What are the chances CCS will fix this?
ed |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Wed Jan 05, 2011 10:08 am |
|
|
I'm presently keeping V4.112, because PCD has an arithmetic bug in the newer versions up to V4.116 and I'm waiting for a fix. I reported it about two month ago, so hopefully it will be fixed in the next release. There are probably some changes related to the bug in V4.115, but it's still reproducible with the test code, I sent to CCS. Because the bug report hasn't been answered explicitely (except for the automated acknowledge mail), I can't be sure if it has been actually processed.
Most previously detected bugs are handled by workarounds in my code. You surely can imagine, that I'm not motivated to check which each new CCS release, if they have probably been fixed silently. I know, that many (or even most) actually have been fixed, but a few still exist, possibly modified.
Only if bug reports would be treated exactly and all changes to the compiler listed in detail, you could expect more. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19539
|
|
Posted: Wed Jan 05, 2011 10:20 am |
|
|
FvM wrote: | A union will be only efficient, if the source data type is already defined as a union, otherwise either a copy operation or a typecast by pointer, similar to the shown example would be necessary.
It would be interesting to know, if the shown long-winded interpretation of the typecast by pointer is a particluar CCS C problem, or if it can be found with other PIC compilers as well? |
You don't need to typecast by pointer. You can do a simple typecast. No data movement is needed.
Code: |
union combine {
int32 word32;
int16 word16[2];
int8 b[4];
};
void main()
{
int32 value = 0x01020304;
int8 small;
small=((union combine)value).b[0];
printf("%2x\n\r",small);
((union combine)value).word16[1]=0x0405;
printf("%08Lx\n\r",value);
while(TRUE);
}
|
Shows reading the first byte from an int32, and writing an int16, into the high half of the int32.
It really is efficient. For example, on the example above, with 'value' at 0x11, the byte move codes as:
Code: |
.................... small=((union combine)value).b[0];
0056: MOVF 11,W
0057: MOVWF 15
//and the 16bit move, as:
.................... ((union combine)value).word16[1]=0x0405;
0063: MOVLW 04
0064: MOVWF 14
0065: MOVLW 05
0066: MOVWF 13
....................
|
It is also portable (with the caveats about byte order on different processors), with the same code happily running under C on Unix, or on VC from MS....
Best Wishes |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Wed Jan 05, 2011 3:25 pm |
|
|
Yes, you're right. I've been using this construct before. Thanks for remembering.
Code: | typedef union
{
signed int32 l;
signed int8 b[4];
}lr;
a = ((lr)data).b[1];
.................... a = ((lw)data).b[1];
08AA: MOVFF 21B,219 |
|
|
|
EdWaugh
Joined: 07 Dec 2004 Posts: 127 Location: Southampton, UK
|
|
Posted: Mon Jan 17, 2011 9:49 am |
|
|
Hi all,
I got this response from CCS today:
Code: | This message is a reply to CCS e-mail id: 1AE002
I am not able to recreate with 4.117. Resumbit this with your source code if you still have the problem with 4.117. |
Not super helpful really but once 4.111 is available for download I'll let you know if it works.
cheers
ed |
|
|
gpsmikey
Joined: 16 Nov 2010 Posts: 588 Location: Kirkland, WA
|
|
Posted: Mon Jan 17, 2011 11:30 am |
|
|
At least you got a response from them. When I submitted a problem with the fact that they don't have the database correct for the 18f14k22, I got a response indicating it had been received and assigned to a tech. Last I heard from them. Hopefully 117 comes out before my upgrade period runs out end of this week :-) Have to see if they fixed mine too.
mikey _________________ mikey
-- you can't have too many gadgets or too much disk space !
old engineering saying: 1+1 = 3 for sufficiently large values of 1 or small values of 3 |
|
|
EdWaugh
Joined: 07 Dec 2004 Posts: 127 Location: Southampton, UK
|
Fixed |
Posted: Sun Aug 14, 2011 5:24 am |
|
|
Sorry about the late follow up to this but the error was fixed by v4.121 and should now compile correctly.
I have just posted a new but similar bug in v4.124 here:
http://www.ccsinfo.com/forum/viewtopic.php?t=46082
Cheers
Ed |
|
|
|