 |
 |
| View previous topic :: View next topic |
| Author |
Message |
Ttelmah
Joined: 11 Mar 2010
Posts: 19504
|
| Has anyone else noticed CCS have changed array maths. |
 Posted: Sun Oct 01, 2017 4:14 am |
 |
|
Hi,
Was re-compiling an old program that has always generated an 'interrupts disabled to prevent re-entrancy' warning, because it uses a structure array access inside an interrupt, and it uses the int16 multiply to calculate the location needed in this. Given this is a modern PIC with a hardware multiplier, it is only a very few instructions, but was always slightly 'annoying', since such accesses are very fundamental. Had last compiled it on 5.070, and switched to 5.074, and was surprised to see the warning disappear.
A quick change backwards and forwards confirmed the change. On .074, the compiler is generating two copies of some of the integer maths routines, without being asked.
Has anyone else noticed this?.
Will have to investigate to see if it also applies to chips without the hardware multiply.
Can't see anything about this in the change list, except a mention of an optimisation bug being fixed in interrupt handlers.
It is still giving the warning in 5.072, and not in 5.073.
It is using 100bytes more ROM in my case.
So has anyone else noticed?.
Any issues seen?. (If anything I'd expect things to be better!...).
Current tests are looking hopeful for me. |
|
 |
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Sun Oct 01, 2017 6:59 am |
|
|
It still gives me a warning for divide by 16. It may have been removed for
for structures, but not for general math. This was tested with vs. 5.074.
| Quote: |
Executing: "C:\Program files\Picc\CCSC.exe" +FH "PCH_Test.c" +DF
+LY -T -A +M -Z +Y=9 +EA #__buildcount__="0" #__18F46K22=TRUE
>>> Warning 216 "PCH_Test.c" Line 23(1,2): Interrupts disabled during
call to prevent re-entrancy: (@DIV1616)
Memory usage: ROM=1% RAM=1% - 1%
0 Errors, 1 Warnings.
Build Successful.
|
Test program:
| Code: |
#include <18F46K22.h>
#fuses INTRC_IO,NOWDT,PUT,BROWNOUT
#use delay(clock=4M)
int16 timer1_value;
#int_timer1
void ccp1_isr()
{
timer1_value = get_timer1() / 3;
}
//=====================================
void main(void)
{
int16 value;
int16 result;
result=value/3;
while(TRUE);
} |
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19504
|
|
| Posted: Sun Oct 01, 2017 7:40 am |
|
|
I think it is only the array index calculation that has been changed. Makes sense if you think about it, since it is always just a simple multiplication and addition, with the size determined by the RAM size of the chip. To add the whole library would be a lot bulkier (as you know we can trick it to do this if required), but it has always been a bit annoying, when you use anything other than a simple array, and end up with the warning. I was using a 2 dimensional array to hold lines of data and pointers to them. Far the easiest way to do what I wanted, but came with the warning, that I accepted. To suddenly have it disappear, was a surprise... 
So far been running since last night without any glitches, so it suggests the change hasn't brought any unwanted 'luggage', except a bit more code space used. |
|
|
newguy
Joined: 24 Jun 2004
Posts: 1907
|
|
| Posted: Sun Oct 01, 2017 8:49 am |
|
|
| It's nice that they changed it, but I didn't notice. Quite some time ago I did notice that same warning, so I dug a bit and found that a mul at the heart of an array access in an isr was to blame. To get round the warning, I encapsulated the multiplication in my own function, my_mult(), so as to force the compiler to use two different multiplications - one "virgin" for the isr, and the other, for everything else. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19504
|
|
| Posted: Sun Oct 01, 2017 9:16 am |
|
|
| That was neat. You can actually make the whole maths library 'duplicate' (just as with the delay functions), but it is quite cumbersome. Your approach was a neat way round. |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1345
|
|
| Posted: Mon Oct 02, 2017 6:58 am |
|
|
| newguy wrote: | | It's nice that they changed it, but I didn't notice. Quite some time ago I did notice that same warning, so I dug a bit and found that a mul at the heart of an array access in an isr was to blame. To get round the warning, I encapsulated the multiplication in my own function, my_mult(), so as to force the compiler to use two different multiplications - one "virgin" for the isr, and the other, for everything else. |
That's neat. A question on how this works:
Say one uses my_multi() which does a multiply and the main code does a multiply. What prevents the compiler from making one internal function for multiplying and simply just calling that both in the main and in my_multi?
Just wanting to make sure I understand so we don't get bugs down the line if the compiler decides to change how it does things. |
|
|
newguy
Joined: 24 Jun 2004
Posts: 1907
|
|
| Posted: Mon Oct 02, 2017 7:14 am |
|
|
This was quite some time ago, so bear with me... I believe that I wrapped it in a #separate, which forced the compiler to treat it as a completely separate function.
The point about accessing an array is that (apparently until recently), CCS was using a multiply to figure out the offset into the array. That sort of implementation, of course, is going to be inlined by the compiler. In other words, bypassing your new multiply function. No need to disable interrupts anymore with that sort of code structure.
I can't take the credit for the idea. PCM, on a number of occasions I believe, has recommended wrapping printf in your own wrapper, my_printf(), for similar reasons. I was just following his lead when I did this with the multiply. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19504
|
|
| Posted: Mon Oct 02, 2017 7:26 am |
|
|
| Yes, I too have wrappered printf here for quite a few things. For a program that has to do just one maths operation in a interrupt it makes an alternative to the #ORG solution (you can use an explicit #ORG before the interrupt handler so that the maths library routines used are loaded 'with' this, and then a separate one for the main code). |
|
|
newguy
Joined: 24 Jun 2004
Posts: 1907
|
|
| Posted: Mon Oct 02, 2017 8:13 am |
|
|
| Now I remember why I'd wrap printf in my own wrapper function: to reduce code size because CCS, by default, tends to inline printf. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19504
|
|
| Posted: Mon Oct 02, 2017 9:20 am |
|
|
| Yes. Generally the CCS optimisation tends to be for performance, rather than for space. Tricks like this are then a way of switching this type of thing to a form that is slightly more compact. Especially when repeated calls are made using the same format. |
|
|
Woody
Joined: 11 Sep 2003
Posts: 83
Location: Warmenhuizen - NL
|
|
| Posted: Tue Jun 29, 2021 9:41 am |
|
|
Excuses for reviving this old thread but I would like to ask: do I need to do something special to make the compiler solve the 'interrupts disabled to prevent re-entrancy [@MUL1616]' warning?
I'm using compiler version 5.104 on an 18F55Q43 (which has a hardware multiplier), but still get the warning for the same reason as Ttelmah decribes in this thread, using a two dimensional array in an (ADC) interrupt and outside that interrupt as well.
Just wondering.
Paul |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19504
|
|
| Posted: Wed Jun 30, 2021 12:01 am |
|
|
You have to be doing something else. Double check you are not doing
something like a multiplication in the interrupt.
I just built a deliberate test to check:
| Code: |
#include <18F55Q43.h>
#device ADC=12
#FUSES NOWDT //No Watch Dog Timer
#use delay(internal=32000000)
unsigned int16 array[3][10] = {
{1,2,3,4,5,6,7,8,9,10},
{1,2,3,4,5,6,7,8,9,10},
{1,2,3,4,5,6,7,8,9,10}
};
unsigned int16 rval=0; //return value
int index1=0,index2=0; //array indexes
#int_timer1
void tick(void)
{
rval=array[index1][index2]; //array access
if (++index2==10) //increment indexes through the array
{
index2=0;
if (++index1==3)
index1=0;
}
}
void main()
{
unsigned int16 testval=0, result, aval;
//basic tick for interrupt
setup_timer_1(T1_FOSC | T1_DIV_BY_8);
enable_interrupts(INT_TIMER1);
enable_interrupts(GLOBAL);
while(TRUE)
{
delay_ms(500);
//now perform a 16bit multiplication
result=testval*rval;
testval++;
//double check by forcing an array access here as well
aval=array[index1][index2];
}
}
|
And it compiles without giving the error. A look at the listing shows it
merrily using a MULLW in the interrupt to multiply the index by the
length of the array line, without calling the MUL1616 library.
Thinking about it, is possibly one of your array indexes larger than 255?.
I suspect that would force the mul library to be used again. |
|
|
Woody
Joined: 11 Sep 2003
Posts: 83
Location: Warmenhuizen - NL
|
|
| Posted: Wed Jun 30, 2021 12:47 am |
|
|
| Quote: | Thinking about it, is possibly one of your array indexes larger than 255?.
I suspect that would force the mul library to be used again. |
Well, the indexes are both int8. The 2 dimensional array itself however is 384 bytes long (12 sensors, and 16 16-bit values each). For a test I shortened it to 8 values per sensor and that indeed gets rid of the warning.
I need those 16 values (might even want 32 values later on) so I guess I'll have to live with the warning. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19504
|
|
| Posted: Wed Jun 30, 2021 2:01 am |
|
|
No, it should handle an array over 255 entries. The mul instruction, accepts
two int8's, but returns an int16. It sounds as if somebody at CCS writing
this decided not to handle the possibility of >255 return. However I'm sure
some of my arrays using this are larger than this, and aren't giving the
error. I really would talk to CCS about this. Possibly it is wrong on some
particular chips?. 
Point out that the multiply does give a int16 return. |
|
|
|
|
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
