 |
 |
| View previous topic :: View next topic |
| Author |
Message |
gaugeguy
Joined: 05 Apr 2011
Posts: 303
|
| PIC24 math bug? |
 Posted: Fri Apr 03, 2015 8:36 am |
 |
|
I found what I believe is a bug in the math routine on a PIC24FJ64GA306 with 8 bit and 16 bit values.
| Code: |
#include <24FJ64GA306.h>
#device ADC=12
#build(STACK=512)
#FUSES NOWDT //No Watch Dog Timer
#FUSES WINDIS //Watch Dog Timer in non-Window mode
#FUSES NOLVR //Low Voltage Regulator Disabled
#FUSES NOWRT //Program memory not write protected
#FUSES NOPROTECT //Code not protected from reading
#FUSES NOJTAG //JTAG disabled
#FUSES NOIOL1WAY //Allows multiple reconfigurations of peripheral pins
#FUSES NOOSCIO //OSC2 is clock output
#FUSES CKSFSM //Clock Switching is enabled, fail Safe clock monitor is enabled
#FUSES VREFNORM_CVREFNORM //VREF and CVREF are mapped to their default pins
#FUSES IESO //Internal External Switch Over mode enabled
#FUSES WPFP //Write/Erase Protect Page Start/End Location, set to last page or use WPFP=x to set page
#FUSES VBATBOR //VBAT BOR Enabled
#FUSES SOSC_SEL //SOSC circuit selected
#FUSES WDTWIN_25% //Watchdog Window is 25% of WDT period
#FUSES BROWNOUT //Reset when brownout detected
#FUSES WPDIS //All Flash memory may be erased or written
#FUSES NOWPCFG //Configuration Words page is not erase/write-protected
#FUSES WPEND //Flash pages WPFP to Configuration Words page are write/erase protected
#FUSES DSWDT_25DAYS //DSWDT uses 1:68719476736 Postscale (25.7Days)
#FUSES DSWDTCK_LPRC //DSWDT uses LPRC as reference clock
#FUSES DSBOR //BOR enabled in Deep Sleep
#FUSES DSWDT //Deep Sleep Watchdog Timer enabled
#FUSES DS_SW //Deep Sleep is controlled by the register bit DSEN
#device ICSP=1
#use delay(clock=32Mhz,crystal=8MHz)
char c;
unsigned int16 LargeValue;
unsigned int8 SmallValue;
void main()
{
LargeValue = 400;
SmallValue = LargeValue / 4; // expect to get 100 here. Instead value is truncated to 144 and the result is 36
SmallValue = (LargeValue / 4); // expect to get 100 here. Instead value is truncated to 144 and the result is 36
SmallValue = (LargeValue >> 2); // expect to get 100 here. Instead value is truncated to 144 and the result is 36
while(TRUE);
}
|
| Code: |
....................
.................... void main()
*
0200: MOV #2600,W15
0202: MOV #27FF,W0
0204: MOV W0,SPLIM
0206: NOP
0208: BSET.B INTCON1.NSTDIS
020A: MOV #0,W0
020C: MOV W0,DMAL
020E: MOV #FFFF,W0
0210: MOV W0,DMAH
0212: CLR CLKDIV
0214: MOV.B #5,W0L
0216: MOV.B W0L,LCDSE0L
0218: CLR ANSB
021A: CLR ANSD
021C: CLR ANSE
021E: CLR ANSG
.................... {
.................... LargeValue = 400;
0220: MOV #190,W4
0222: MOV W4,802
.................... SmallValue = LargeValue / 4; // expect to get 100 here. Instead value is truncated to 144 and the result is 36
0224: MOV.B 802,W0L
0226: MOV.B W0L,801
0228: LSR.B 801
022A: LSR.B 801
....................
.................... SmallValue = (LargeValue / 4); // expect to get 100 here. Instead value is truncated to 144 and the result is 36
022C: MOV.B 802,W0L
022E: MOV.B W0L,801
0230: LSR.B 801
0232: LSR.B 801
....................
.................... SmallValue = (LargeValue >> 2); // expect to get 100 here. Instead value is truncated to 144 and the result is 36
0234: MOV.B 802,W0L
0236: MOV.B W0L,801
0238: LSR.B 801
023A: LSR.B 801
....................
.................... while(TRUE);
023C: BRA 23C
....................
.................... } |
The low byte of the 16 bit variable is first moved into the 8 bit result and then the 8 bit result is divided by 4. I expect the divide by 4 to be done with the 16 bit value and then the low byte of the result moved into the final variable.
Anyone see where I made a mistake here or is it a bug?
I have tried this on 5.028 and 5.042 |
|
 |
Ttelmah
Joined: 11 Mar 2010
Posts: 19549
|
|
| Posted: Fri Apr 03, 2015 9:47 am |
|
|
You need to report this.
It's the use of unsigned that triggers it. I switched to using the type declarations in stdint.h, to avoid this.
If you include stdint.h, and then declare your variables using it's types, so:
| Code: |
uint_fast16_t LargeValue;
uint_fast8_t SmallValue;
|
The code then compiles correctly:
| Code: |
.................... LargeValue = 400;
0220: MOV #190,W4
0222: MOV W4,800
.................... SmallValue = LargeValue / 4L;
0224: PUSH 800
0226: POP 802
0228: LSR 802
022A: LSR 802
|
The compiler then uses a 16bit location for the int8, since this is easier and quicker.... |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
| Posted: Sat Apr 04, 2015 5:19 am |
|
|
| Ttelmah wrote: | You need to report this.
It's the use of unsigned that triggers it. I switched to using the type declarations in stdint.h, to avoid this.
If you include stdint.h, and then declare your variables using it's types, so:
| Code: |
uint_fast16_t LargeValue;
uint_fast8_t SmallValue;
|
The code then compiles correctly:
| Code: |
.................... LargeValue = 400;
0220: MOV #190,W4
0222: MOV W4,800
.................... SmallValue = LargeValue / 4L;
0224: PUSH 800
0226: POP 802
0228: LSR 802
022A: LSR 802
|
The compiler then uses a 16bit location for the int8, since this is easier and quicker.... |
Is it only this particular device or all the 24 bit opcode are touched by this flaw?
I have PCWHD 5.042.
_________________
Regards,
Laurent
-----------
Here's my first visual theme for the CCS C Compiler. Enjoy! |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19549
|
|
| Posted: Sat Apr 04, 2015 7:19 am |
|
|
The maths libraries are common to all chips.....
I had problems a while ago, when using unsigned int8, and stopped using them. For any integer counter that needed to go over 127, I switched to using int16, so the only place int8 was used was in variables inside data structures etc.., which I always cast up before using. I hadn't bothered to investigate what was wrong, but gaugeguy has managed to generate nice simple demo, which shows exactly what is happening. It's worth being aware that because of the 16bit nature of the chip, it's actually more efficient (except for storage space), to use int16 - hence the use of an int16 'container' for the fast int8 type. |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
| Posted: Sat Apr 04, 2015 8:06 am |
|
|
| Ttelmah wrote: | The maths libraries are common to all chips.....
I had problems a while ago, when using unsigned int8, and stopped using them. For any integer counter that needed to go over 127, I switched to using int16, so the only place int8 was used was in variables inside data structures etc.., which I always cast up before using. I hadn't bothered to investigate what was wrong, but gaugeguy has managed to generate nice simple demo, which shows exactly what is happening. It's worth being aware that because of the 16bit nature of the chip, it's actually more efficient (except for storage space), to use int16 - hence the use of an int16 'container' for the fast int8 type. |
Thanks Ttelmah for your advice 
_________________
Regards,
Laurent
-----------
Here's my first visual theme for the CCS C Compiler. Enjoy! |
|
|
gaugeguy
Joined: 05 Apr 2011
Posts: 303
|
|
| Posted: Mon Apr 06, 2015 6:59 am |
|
|
| CCS has responded that this will be fixed in the next compiler release. |
|
|
|
|
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
