 |
 |
| View previous topic :: View next topic |
| Author |
Message |
pcarew
Joined: 08 Oct 2014
Posts: 6
|
| ccs v5.030 const table lookup bug |
 Posted: Wed Oct 08, 2014 9:18 pm |
 |
|
I believe that I have found a strange, but potentially disastrous bug if you use const tables in the latest release (5.030) of CCS
I have sent an email to CCS support as well.
Essentially, the bug is in the asm that is generated for a const table lookup.
Under certain circumstances, the compiler generates incorrect table lookup code (I have attached c source and lst files).
The strange thing is, if I add an extraneous line of c code to the crc calculation function (to printout a progress indication), the compiler suddenly generates the correct lookup code!
The function in question is simple and contains:
| Code: |
int8u docrc8(int8u value)
{
crc8 = dscrc_table[crc8 ^ value];
sprintf(displayString,"CRC8 done ");PrintDebugString(displayString);
return(crc8);
}
|
crc8 is a global variable defined as ' volatile int8u crc8 '
dscrc_table is a 256 entry table defined as ' const int8u dscrc_table[] = {.... '
With the function compiled as above, the asm contains a missing instruction, namely: '00296: CLR.B 1' in the dscrc_table lookup assembly
However....With the function compiled with a field added to the sprintf format, the asm contains the correct instruction!
I.E> with the format field shown here, the compiler generates the correct table lookup code
| Code: |
sprintf(displayString,"CRC8 done %d",88);PrintDebugString(displayString); |
As strange as this seems, I'm including the snippets from the lst files for both scenarios.
| Code: |
volatile int8u crc8, isrCrc8;
//**************************************************************************
const int8u dscrc_table[] = {
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
157,195, 33,127,252,162, 64, 30, 95, 1,227,189, 62, 96,130,220,
......
116, 42,200,150, 21, 75,169,247,182,232, 10, 84,215,137,107, 53};
//**************************************************************************
// Calculate the CRC8 of the byte value provided with the current
// global 'crc8' value.
// Returns current global crc8 value
int8u docrc8(int8u value)
{
crc8 = dscrc_table[crc8 ^ value];
sprintf(displayString,"CRC8 done %d",88);PrintDebugString(displayString);
//sprintf(displayString,"CRC8 done");PrintDebugString(displayString);
return(crc8);
}
|
Lst file contents that is correct:
| Code: |
Works
00288: CLR 32
0028A: MOV #AC,W3
0028C: SUB W0,W3,W3
0028E: BRA C,29A
00290: MOV #2A4,W3
00292: ADD W3,W0,W0
00294: TBLRDL.B[W0],W0L
00296: CLR.B 1 <--------This is the correct line that is missing in the version following!
00298: RETURN
0029A: MOV #2A4,W0
0029C: ADD W3,W3,W3
0029E: ADD W3,W0,W3
002A0: TBLRDH [W3],W0
002A2: RETURN
002A4: DATA 00,5E,0C
002A6: DATA BC,E2,52
.......
00340: DATA B2,EC,0A
00342: DATA 0E,50,54
00344: DATA AF,F1,D7
00346: DATA 13,4D,89
00348: DATA CE,90,6B
0034A: DATA 72,2C,35
0034C: DATA 6D,33,00
0034E: DATA D1,8F,00
.................... const int8u dscrc_table[] = {
.................... 0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
......
.................... 116, 42,200,150, 21, 75,169,247,182,232, 10, 84,215,137,107, 53};
....................
.................... //**************************************************************************
.................... // Calculate the CRC8 of the byte value provided with the current
.................... // global 'crc8' value.
.................... // Returns current global crc8 value
....................
.................... int8u docrc8(int8u value)
*
05848: MOV W5,[W15++]
.................... {
.................... crc8 = dscrc_table[crc8 ^ value];
0584A: MOV 22EE,W0
0584C: LSR W0,#8,W0
0584E: MOV 3B5E,W4
05850: CLR.B 9
05852: XOR.B W0L,W4L,W0L
05854: CLR.B 1
05856: CALL 288
0585A: PUSH 22EE
0585C: MOV.B W0L,[W15-#1]
0585E: POP 22EE
.................... sprintf(displayString,"CRC8 done %d",88);PrintDebugString(displayString);
05860: MOV #166A,W4
05862: MOV W4,230E
05864: MOV #0,W1
05866: MOV W1,W0
05868: CLR.B 1
0586A: CALL 350
0586E: INC W1,W1
05870: MOV W1,[W15++]
05872: MOV W0,[W15++]
05874: MOV [--W15],W0
05876: CALL 4B34
0587A: MOV [--W15],W1
0587C: MOV #9,W0
0587E: CPSGT W1,W0
05880: BRA 5866
05882: MOV #58,W0
05884: MOV #0,W4
05886: CALL 5742
0588A: MOV #166A,W4
0588C: MOV W4,3B60
0588E: CALL 4C14
.................... return(crc8);
|
And now the broken version with the missing line:
| Code: |
Fails
00288: CLR 32
0028A: MOV #AC,W3
0028C: SUB W0,W3,W3
0028E: BRA C,298
00290: MOV #2A8,W3
00292: ADD W3,W0,W0
00294: TBLRDL [W0],W0
00296: RETURN
00298: MOV #2A8,W0
0029A: ADD W3,W3,W3
0029C: ADD W3,W0,W3
0029E: TBLRDH [W3++],W0
002A0: TBLRDH [W3],W3
002A2: SL W3,#8,W3
002A4: IOR W3, W0,W0
002A6: RETURN
002A8: DATA 00,5E,0C
002AA: DATA BC,E2,52
002AC: DATA 61,3F,B0
......
00352: DATA D1,8F,00
.................... const int8u dscrc_table[] = {
.................... 0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
...
.................... 116, 42,200,150, 21, 75,169,247,182,232, 10, 84,215,137,107, 53};
....................
.................... //**************************************************************************
.................... // Calculate the CRC8 of the byte value provided with the current
.................... // global 'crc8' value.
.................... // Returns current global crc8 value
....................
.................... int8u docrc8(int8u value)
*
05744: MOV W5,[W15++]
.................... {
.................... crc8 = dscrc_table[crc8 ^ value];
05746: MOV 22EE,W0
05748: LSR W0,#8,W0
0574A: MOV 3B5E,W4
0574C: CLR.B 9
0574E: XOR.B W0L,W4L,W0L
05750: CLR.B 1
05752: CALL 288
05756: PUSH 22EE
05758: MOV.B W0L,[W15-#1]
0575A: POP 22EE
.................... sprintf(displayString,"CRC8 done ");PrintDebugString(displayString);
0575C: MOV #166A,W4
0575E: MOV W4,230E
05760: MOV #0,W1
05762: MOV W1,W0
05764: CLR.B 1
05766: CALL 354
0576A: INC W1,W1
0576C: MOV W1,[W15++]
0576E: MOV W0,[W15++]
05770: MOV [--W15],W0
05772: CALL 4B36
05776: MOV [--W15],W1
05778: MOV #9,W0
0577A: CPSGT W1,W0
0577C: BRA 5762
0577E: MOV #166A,W4
05780: MOV W4,3B60
05782: CALL 4C16
.................... return(crc8);
|
|
|
 |
Ttelmah
Joined: 11 Mar 2010
Posts: 19520
|
|
| Posted: Thu Oct 09, 2014 2:49 am |
|
|
Thing is the function only returns an int8, So clearing the high byte should not be necessary for the return, since the code 'waiting' at the other end should only be using the single low byte. It is necessary to clear it, if the value is to be used internally by another function, accessing the whole W register, so the compiler does this.
If you are having a problem, it suggests the calling function, is accessing the whole register on the return, which is where the bug is.... |
|
|
pcarew
Joined: 08 Oct 2014
Posts: 6
|
|
| Posted: Thu Oct 09, 2014 1:02 pm |
|
|
Looking in more detail at the assemblly for the table lookup, there were several differences between the working case and the non working case.
But the bug seems to be specifically with the TBLRDL table load instruction.
In the working version, it uses the byte mode and in the broken version it uses the word mode.
The worst part about this, is that it seems to be a 'Double Bug'.
It is not just that the compiler generates the wrong lookup code, but that it is not consistant about it. Sometime it creates the right code. Without understanding what is controllling this decision, it is hard to create a work around'.
As for the lookup code specifically, I'm still getting to grips with PIC24 Assembly, but it looks like the CCS compiler packs the 256 table entries into 172 16bit program memory words (PMW). The odd PMWs are 16bits, but the top 8 bits are always zero.
The table is mapped into this PMW space as:
The 1st 172 table byte values are stored into the low and high bytes of the Even PMW and the remaining 84 entries are then placed into theLow bytes of the 86 odd words (last two odd words have their byte zero filled)
From the PIC24 manual:
[/code]
TBLRDL (Table Read Low): In Word mode, it
maps the lower word of the program space
location (P<15:0>) to a data address (D<15:0>).
In Byte mode, either the upper or lower byte of
the lower program word is mapped to the lower
byte of a data address. The upper byte is
selected when the byte select is ‘1’; the lower
byte is selected when it is ‘0’.
[quote]
The Working Case:
| Code: |
00288: CLR 32
0028A: MOV #AC,W3
0028C: SUB W0,W3,W3
0028E: BRA C,29A
00290: MOV #2A4,W3
00292: ADD W3,W0,W0
00294: TBLRDL.B[W0],W0L <--- This grabs the 'appropriate' byte out of the even PMW
00296: CLR.B 1
00298: RETURN
0029A: MOV #2A4,W0
0029C: ADD W3,W3,W3
0029E: ADD W3,W0,W3
002A0: TBLRDH [W3],W0
002A2: RETURN
002A4: DATA 00,5E,0C
002A6: DATA BC,E2,52
...
0034C: DATA 6D,33,00
0034E: DATA D1,8F,00
|
Broken Case:
| Code: |
00288: CLR 32
0028A: MOV #AC,W3
0028C: SUB W0,W3,W3
0028E: BRA C,298
00290: MOV #2A8,W3
00292: ADD W3,W0,W0
00294: TBLRDL [W0],W0 <--- This grabs the whole word where the low byte is not the one we want when we're asking for the high byte
00296: RETURN
00298: MOV #2A8,W0
0029A: ADD W3,W3,W3
0029C: ADD W3,W0,W3
0029E: TBLRDH [W3++],W0
002A0: TBLRDH [W3],W3
002A2: SL W3,#8,W3
002A4: IOR W3, W0,W0
002A6: RETURN
002A8: DATA 00,5E,0C
002AA: DATA BC,E2,52
|
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19520
|
|
| Posted: Fri Oct 10, 2014 2:21 am |
|
|
Can you generate a small compilable program, that can be posted complete, so we can see what is going on?.
The storage, is a slight 'misunderstanding' on how the memory is laid out.
On the PIC24 and up, the program memory, has a word length of 32bits, with only 24 implemented. So a complete instruction word, is:
nnnnnn00 (hex)
This is the basic structure of the memory. This is then treated as two 16bit addresses for 'convenience', but instructions always start on an even address.
The lower word can contain two bytes, while the upper can only hold one.
So CCS are storing the data 3bytes to the instruction, in 86 instructions.
I've just put the program 'together' as a minimum file on a basic PIC24, and it gives the right answer. Only difference is that I'm using the CCS supplied types, and have just filled the table with duplicated lines. So:
| Code: |
#include <24FJ128GA006.h>
#FUSES NOWDT //No Watch Dog Timer
#FUSES NOJTAG //JTAG disabled
#FUSES CKSFSM //Clock Switching is enabled, fail Safe clock monitor is enabled
#device ICSP=1
#use delay(crystal=20000000)
#include "stdint.h"
const uint8_t dscrc_table[] =
{
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
116, 42,200,150, 21, 75,169,247,182,232, 10, 84,215,137,107, 53,
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
116, 42,200,150, 21, 75,169,247,182,232, 10, 84,215,137,107, 53,
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
116, 42,200,150, 21, 75,169,247,182,232, 10, 84,215,137,107, 53,
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
0, 94,188,226, 97, 63,221,131,194,156,126, 32,163,253, 31, 65,
116, 42,200,150, 21, 75,169,247,182,232, 10, 84,215,137,107, 53
};
uint8_t crc8=0; //CCS does not handle volatile - you have to protect
//variables if they can update in other code, yourself......
uint8_t docrc8(uint8_t value)
{
crc8 = dscrc_table[crc8 ^ value];
return(crc8);
}
void main()
{
uint8_t loop, temp;
for (loop=0;loop<128;loop++) //just test 128 times
temp=docrc8(loop);
while(TRUE)
{
;
}
}
|
With every version I have tried like this, the correct form is used. |
|
|
pcarew
Joined: 08 Oct 2014
Posts: 6
|
|
| Posted: Mon Oct 13, 2014 12:55 pm |
|
|
I agree that your small example compiles and generates the correct code.
All I can say is that our project is huge and definitely does not generate the same code (see quoted included lst files)
I will try to issolate further, but this is definitely a big problem for us. |
|
|
pcarew
Joined: 08 Oct 2014
Posts: 6
|
|
| Posted: Mon Oct 13, 2014 1:00 pm |
|
|
We're currently compile to these stats:
Memory usage: ROM=70% RAM=78% - 92%
This is on a PIC24 256K GA106 device ( 24FJ256GA106.h ) |
|
|
pcarew
Joined: 08 Oct 2014
Posts: 6
|
|
| Posted: Mon Oct 13, 2014 4:09 pm |
|
|
V5.X I believe introduced new optimizations for ram and rom usage. I wonder if this is where the issue lies.
Our project has ~35,000 lines of code and comments |
|
|
pcarew
Joined: 08 Oct 2014
Posts: 6
|
| Resolution from CCS |
| Posted: Wed Oct 22, 2014 9:40 am |
|
|
CCS supplied an updated compiler dll (v5.031) that seems to have resolved the problem.
They have not described what they found yet, although I have asked. |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
