CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com

PIC24 math bug?

 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
gaugeguy



Joined: 05 Apr 2011
Posts: 301

View user's profile Send private message

PIC24 math bug?
PostPosted: Fri Apr 03, 2015 8:36 am     Reply with quote

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: 19451

View user's profile Send private message

PostPosted: Fri Apr 03, 2015 9:47 am     Reply with quote

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

View user's profile Send private message

PostPosted: Sat Apr 04, 2015 5:19 am     Reply with quote

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: 19451

View user's profile Send private message

PostPosted: Sat Apr 04, 2015 7:19 am     Reply with quote

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

View user's profile Send private message

PostPosted: Sat Apr 04, 2015 8:06 am     Reply with quote

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 Smile
_________________
Regards,
Laurent

-----------
Here's my first visual theme for the CCS C Compiler. Enjoy!
gaugeguy



Joined: 05 Apr 2011
Posts: 301

View user's profile Send private message

PostPosted: Mon Apr 06, 2015 6:59 am     Reply with quote

CCS has responded that this will be fixed in the next compiler release.
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
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