|
|
View previous topic :: View next topic |
Author |
Message |
2xhp
Joined: 14 Jan 2019 Posts: 39
|
dsPIC33 and weirdness on multiplication |
Posted: Tue Mar 02, 2021 12:14 pm |
|
|
I'm sure there's a good explanation for this, but I'm really baffled. I'm migrating code from an 18F device to a dsPIC33 (specifically the dsPIC33EV128GM104). I know that the dsPIC33 has variables defaulted to signed, so I have been careful to be explicit about signed vs. unsigned.
Bottom line is the following works on a 18F and doesn't on the dsPIC33:
Code: | first_byte = 0x28;
second_byte = 0x65;
return (unsigned int16)(((unsigned int32)(make16(first_byte,second_byte)) * 6250) >> 14); |
Here's the stripped down function with some notes of various things I tried and the results:
Code: | unsigned int16 test_function() {
unsigned int8 first_byte, second_byte;
float temp1, temp2;
unsigned int32 temp_calculation;
first_byte = 0x28;
second_byte = 0x65;
/*
// Both of the following return garbage (but repeatable garbage) in all calls (and something different in each of 6 calls in a row)
return (unsigned int16)(((unsigned int32)(make16(first_byte,second_byte)) * (unsigned int32)6250) >> 14);
return (unsigned int16)(((unsigned int32)(make16(first_byte,second_byte)) * 6250) >> 14);
*/
/*
// returns (repeatable) garbage the first call, and the correct 0x0F68 in 5 subsequent calls to the function
temp_calculation = (unsigned int32)make16(first_byte,second_byte);
temp_calculation *= 6250;
temp_calculation >>= 14;
return (unsigned int16)temp_calculation;
*/
/*
// Returns the correct 0x0F68 in all calls
temp1 = (float)make16(first_byte,second_byte);
temp1*=(float)6250;
temp1/=(float)16383;
return (unsigned int16)temp1;
*/
// returns the correct 0x0F68 in all calls to the function
temp_calculation = 0;
temp_calculation = (unsigned int32)make16(first_byte,second_byte);
temp_calculation *= 6250;
temp_calculation >>= 14;
return (unsigned int16)temp_calculation;
} |
PCD compiler version is 5.103.
Thanks in advance for any thoughts. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19504
|
|
Posted: Tue Mar 02, 2021 1:13 pm |
|
|
I've no idea why it is behaving so badly. Will try to have a look at it
tomorrow.
However you have to realise that casting costs time, and in what you
show is really unwanted.
Use:
Code: |
unsigned int16 test_function() {
unsigned int8 first_byte, second_byte;
first_byte = 0x28;
second_byte = 0x65;
return ((make16(first_byte,second_byte)) * 6250UL) >> 14);
}
|
Using UL for the declaration of 6250, forces this to be used as an unsigned
int32. Using this then forces the make16 result to be implicitly cast to this
type. The function return does a similar implicit cast down to int16.
There is a 'known' issue that sometimes lines that us several
'scratch' variables internally, can result in the temporary scratch values
being overwritten. My suspicion is that by combining the explicit casts
with the other maths operations, you are overflowing this area. Using
implicit casts avoids the use of extra space, and will be quicker. |
|
|
2xhp
Joined: 14 Jan 2019 Posts: 39
|
|
Posted: Tue Mar 02, 2021 3:16 pm |
|
|
Ttelmah wrote: |
Code: |
unsigned int16 test_function() {
unsigned int8 first_byte, second_byte;
first_byte = 0x28;
second_byte = 0x65;
return ((make16(first_byte,second_byte)) * 6250UL) >> 14);
}
|
Using UL for the declaration of 6250, forces this to be used as an unsigned
int32. Using this then forces the make16 result to be implicitly cast to this
type. The function return does a similar implicit cast down to int16.
|
Thanks so much for the quick response.
Unfortunately though, attempting this code results in zero being returned. Very odd. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19504
|
|
Posted: Wed Mar 03, 2021 2:24 am |
|
|
It I try with my function (one bracket missing as originally posted), and use
6250ULL, I get the correct answer *2!... So it looks as if it is rotating by
13 positions, not 14.
Now your first 'Both of the following', does exactly the same. So not
'garbage', but a missed rotation. It looks as if when the maths is performed
inside the return like this, it is doing one less rotation than it is being asked
to do.
I then went back a few compiler versions (had to switch to a different PIC33
since this chip is not supported this far back), and it does the same.
Oddly, a couple of the ones you claim work, also give the same error for me.
The results also don't seem to change with different numbers of calls.
Possibly you have something 'else' happening that is screwing the
diagnostics a little.
The ones that do work are where int32 arithmetic is avoided (so using
float). It seems to be the multiplication by 6250, giving twice the value
it should!...
The most code efficient version I could generate, was to use make32
instead of make16. Avoids the cost of copying the 16bit variable:
Code: |
unsigned int16 test_function() {
unsigned int8 first_byte, second_byte;
first_byte = 0x28;
second_byte = 0x65;
return (((make32(0,0,first_byte,second_byte)) * 6250U) >> 14);
}
|
Definitely a 'talk to CCS' problem. |
|
|
|
|
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
|