|
|
View previous topic :: View next topic |
Author |
Message |
kda406
Joined: 17 Sep 2003 Posts: 97 Location: Atlanta, GA, USA
|
[Solved] Math Issue Int Rounding GCC vs PCH |
Posted: Thu Oct 17, 2019 2:36 pm |
|
|
I have a piece of code that works. Now I need it in two places, so I put it into a function and it no longer works in PCH. I checked the code using GCC and it works as a function in GCC. I'm using a PIC18F67K40 and have tried PCH 5.078 - 5.090.
I have an unsigned long int with a value like 5076, which means 50.76%. Eventually, I need the rounded whole number, which would be 51%. The CCS compiler returns 5001. Here is the code to evaluate the rounding:
Code: | //--------------------------------------------------------------------------
// This function divides a UINT32 by 100 and returns a rounded answer.
//
UINT32 RoundDiv100_UINT32(UINT32 lRawE2) {
UINT32 lWholeNum;
UINT32 lMantissa;
lWholeNum = lRawE2 / 100; // Store off the whole number for rounding and post-rounding use
lMantissa = lRawE2 - (lWholeNum * 100); // Store off the mantissa for rounding
if(lMantissa > 49) lWholeNum++; // Round it up if necessary
return(lWholeNum); // Return the rounded, divided by 100 answer
} |
In CCS, with this code inline (truly inline, I do not mean with #inline), the CCS compiler returns 51. With the code as a function, it returns 5001.
I coded up a test program and compiled it with GCC, but it works in GCC:
Code: | #include <stdio.h>
#define UINT32 unsigned long
#define TERM stdout
UINT32 RoundDiv100_UINT32(UINT32);
int main() {
UINT32 lNum = 5076;
fprintf(TERM,"lNum = %lu\n\r",lNum);
lNum = RoundDiv100_UINT32(lNum);
fprintf(TERM,"Rounded = %lu\n\r",lNum);
}
UINT32 RoundDiv100_UINT32(UINT32 lRawE2) {
UINT32 lWholeNum;
UINT32 lMantissa;
lWholeNum = lRawE2 / 100; // Store off the whole number for rounding and post-rounding use
lMantissa = lRawE2 - (lWholeNum * 100); // Store off the mantissa for rounding
if(lMantissa > 49) lWholeNum++; // Round it up if necessary
return(lWholeNum); // Return the rounded, divided by 100 answer
} |
Quote: | $ ./rounddiv100
lNum = 5076
Rounded = 51
$ |
Can anyone suggest why the code works inline with CCS and as a function in GCC, but not in a function in CCS?
Thanks,
Kyle |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19546
|
|
Posted: Fri Oct 18, 2019 2:02 am |
|
|
Dangerous thing. _don't use identifiers like 'long' for variable sizes_.
It's a thing I and others have warned about again and again. 'long' has
different meanings in different compilers. Always use explicit identifiers
like int16, int32 etc., or use the 'ANSI' specifier, which forces the compiler
to select the ANSI default size behaviour.
What is happening is that 'long' in CCS C on a PIC16 or 18, is an int16
not an int32. Your #defined identifier that you think refers to an int32,
doesn't. It is generating an int16. Result when the call is made to the
function, it arrives as an int16 variable, and int16 maths is used.
Built as:
Code: |
#include <18F67K40.h>
#USE DELAY(INTERNAL=16MHz)
#PIN_SELECT U1TX=PIN_C0
#PIN_SELECT U1RX=PIN_C1
#use RS232(UART1, baud=9600, ERRORS)
#include <stdio.h>
#define UINT32 unsigned int32
#define TERM stdout
UINT32 RoundDiv100_UINT32(UINT32);
void main(void) {
UINT32 lNum = 5076;
fprintf(TERM,"lNum = %lu\n\r",lNum);
lNum = RoundDiv100_UINT32(lNum);
fprintf(TERM,"Rounded = %lu\n\r",lNum);
while(TRUE)
delay_cycles(1);
}
UINT32 RoundDiv100_UINT32(UINT32 lRawE2) {
UINT32 lWholeNum;
UINT32 lMantissa;
lWholeNum = lRawE2 / 100; // Store off the whole number for rounding and post-rounding use
lMantissa = lRawE2 - (lWholeNum * 100); // Store off the mantissa for rounding
if(lMantissa > 49) lWholeNum++; // Round it up if necessary
return(lWholeNum); // Return the rounded, divided by 100 answer
}
|
Built like this, it merrily gives:
lNum = 5076
Rounded = 51
on the terminal. |
|
|
kda406
Joined: 17 Sep 2003 Posts: 97 Location: Atlanta, GA, USA
|
|
Posted: Fri Oct 18, 2019 6:54 am |
|
|
Thanks, but in the version I compile for use on the target board, this is how UINT32 is defined: Code: | #define UINT32 unsigned int32 |
This project uses 48 c files, and even more headers. On processors like the K40s, with things like mappable IO, required weak pullups, and the like, the target boards go crazy with "simplified" software. Relays buzzing, etc. So if my code is suspect, I drop it into GCC to test. Unfortunately, GCC does not recognize unsigned int32, so I had to define it otherwise for GCC. You are correct, and I never use "long" in CCS code either.
Perhaps it was a mistake, but I showed the GCC code to demonstrate it works in GCC. My problem is it does not work when compiled with PCH.
I do not fully understand the 18F assembly, but it appears to me that PCH has compiled it to use 32 bit integers in the non-working code: Code: | .................... lWholeNum = lRawE2 / 100; // Store off the whole number for rounding and post-rounding use
*
012AE: BCF FD8.1
012B0: MOVFF 376,493
012B4: MOVFF 375,492
012B8: MOVFF 374,491
012BC: MOVFF 373,490
012C0: MOVLB 4
012C2: CLRF x97
012C4: CLRF x96
012C6: CLRF x95
012C8: MOVLW 64
012CA: MOVWF x94
012CC: MOVLB 0
012CE: RCALL 0DB4
.................... lMantissa = lRawE2 - (lWholeNum * 100); // Store off the mantissa for rounding
012D0: MOVFF 03,38B
012D4: MOVFF 02,38A
012D8: MOVFF 01,389
012DC: MOVFF 00,388
012E0: MOVLB 3
012E2: CLRF x8F
012E4: CLRF x8E
012E6: CLRF x8D
012E8: MOVLW 64
012EA: MOVWF x8C
012EC: MOVLB 0
012EE: RCALL 0D58
012F0: MOVF 00,W
012F2: MOVLB 3
012F4: SUBWF x73,W
012F6: MOVWF x77
012F8: MOVF 01,W
012FA: SUBWFB x74,W
012FC: MOVWF x78
012FE: MOVF 02,W
01300: SUBWFB x75,W
01302: MOVWF x79
01304: MOVF 03,W
01306: SUBWFB x76,W
01308: MOVWF x7A
.................... if(lMantissa > 49) lWholeNum++; // Round it up if necessary
0130A: MOVF x7A,F
0130C: BNZ 131C
0130E: MOVF x79,F
01310: BNZ 131C
01312: MOVF x78,F
01314: BNZ 131C
01316: MOVF x77,W
01318: SUBLW 31
0131A: BC 132C
0131C: MOVLW 01
0131E: ADDWF 00,F
01320: BTFSC FD8.0
01322: INCF 01,F
01324: BTFSC FD8.2
01326: INCF 02,F
01328: BTFSC FD8.2
0132A: INCF 03,F
.................... return(lWholeNum); // Return the rounded, divided by 100 answer |
Again, this function works in CCS if I paste it inline where I need to divide by 100 and round. It just does not work when called as a function.
Thanks,
Kyle |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19546
|
|
Posted: Fri Oct 18, 2019 7:14 am |
|
|
As I show it runs fine for me. Have tried with two compiler versions just
to be sure.
Add some diagnostics. When the value arrives in the function, print it
there. Then print the /100 version, and the result of multiplying this
by 100.
If you are loading stdint, then use uint32_t which is the unsigned int32
type defined in this, and both CCS and GCC should accept this. |
|
|
kda406
Joined: 17 Sep 2003 Posts: 97 Location: Atlanta, GA, USA
|
|
Posted: Fri Oct 18, 2019 8:55 am |
|
|
Some really weird stuff is going on with this.
In my GenericDefsForCCS.h file I changed
#define UINT32 unsigned int32
to
#define UINT32 uint32_t
and it compiles fine. When I put it into the target, all heck breaks loose, and the target reboots over and over. I've never seen a PIC do that before. So I switched back to the original method, and it no longer reboots.
On the checking values, that also gives odd behavior. In the following:
Code: | lWholeNum = lRawE2 / 100; // Store off the whole number for rounding and post-rounding use
//fprintf(TERM,"lWholeNum=%lu ",lWholeNum);
lMantissa = lRawE2 - (lWholeNum * 100); // Store off the mantissa for rounding
if(lMantissa > 49) lWholeNum++; // Round it up if necessary
return(lWholeNum); // Return the rounded, divided by 100 answer |
Again, assuming I pass 5076, the function returns 5001. If I unremark the print line, the function correctly returns 51. Remark out the print line, and it again returns 5001.
In fact, I found that if I move the print line to after the lMantissa = line, the print gives me 5000. So printing the variable on one C line versus the other yields different results.
This prints 50: Code: | lWholeNum = lRawE2 / 100; // Store off the whole number for rounding and post-rounding use
fprintf(TERM,"lWholeNum=%lu ",lWholeNum);
lMantissa = lRawE2 - (lWholeNum * 100); // Store off the mantissa for rounding
|
This prints 5000: Code: | lWholeNum = lRawE2 / 100; // Store off the whole number for rounding and post-rounding use
lMantissa = lRawE2 - (lWholeNum * 100); // Store off the mantissa for rounding
fprintf(TERM,"lWholeNum=%lu ",lWholeNum);
|
I'm going to start dissecting the .lst file as my next step. Thanks for trying it on your target. |
|
|
newguy
Joined: 24 Jun 2004 Posts: 1909
|
|
Posted: Fri Oct 18, 2019 9:19 am |
|
|
I suspect you might be seeing an issue with printf itself. Instead of printing the decimal equivalent, can you just print the raw hex value of the variable instead and then convert that yourself to decimal? |
|
|
kda406
Joined: 17 Sep 2003 Posts: 97 Location: Atlanta, GA, USA
|
|
Posted: Fri Oct 18, 2019 9:45 am |
|
|
Thanks, but we are sure the function is returning the wrong answer. |
|
|
kda406
Joined: 17 Sep 2003 Posts: 97 Location: Atlanta, GA, USA
|
|
Posted: Fri Oct 18, 2019 11:15 am |
|
|
It appears to be a bug in the code optimization level.
With optimization set to 9, the function does NOT work.
With optimization set 0-8, the function WORKS as expected.
Comparing the optimization 8 vs 9 in WinMerge so I can see the assembler side by side, the optimization 9 is missing 12 lines comparatively. The 12 lines are 3 sets of 4 regarding 32 bit data. It appears to me that the optimization 9 level code is doing incomplete the math (8 bits only?).
It's a lot to post, but here are two snippets that demonstrate the kinds of differences I'm seeing. Optimization 8 (working): Code: | .................... return(lWholeNum); // Return the rounded, divided by 100 answer
0137E: MOVFF 377,00
01382: MOVFF 378,01
01386: MOVFF 379,02
0138A: MOVFF 37A,03
0138E: MOVLB 0
01390: GOTO 14DE (RETURN) |
Optimization 9 (not working): Code: | .................... return(lWholeNum); // Return the rounded, divided by 100 answer
011F2: MOVLB 0
011F4: GOTO 1336 (RETURN) |
That's just a sample. Each C line of this function seems to missing the 32 bit "mechanisms" in the assembler.
Thanks,
Kyle |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19546
|
|
Posted: Sat Oct 19, 2019 12:44 am |
|
|
Are you sure you have actually recompiled with the multiple CCS versions
you described in your first post?. It's not possible that the recompile
was actually using the old compiler DLL.
There was an issue reported here with OPT back in the 5.078 era, giving
results like you describe, but this was fixed a couple of versions later.
As already said, it is not doing this for me, and this with the default OPT. |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Sat Oct 19, 2019 12:49 am |
|
|
My gut feeling for a while now, is that he should re-install his compiler. |
|
|
kda406
Joined: 17 Sep 2003 Posts: 97 Location: Atlanta, GA, USA
|
|
Posted: Tue Oct 22, 2019 9:23 am |
|
|
Solved!
I removed all versions of CCS. After a clean boot, I reinstalled 5.090 and now the function works regardless of optimization level setting. Thanks very much for attempting to recreate the issue and for the excellent advice.
-Kyle |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19546
|
|
Posted: Tue Oct 22, 2019 1:54 pm |
|
|
That's good.
I think what was happening was a faulty DLL was being used for the build
with all the versions. This is why I always install as completely separate
compilers, rather than using the compilers 'historical version' feature. |
|
|
|
|
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
|