I am using an 8051 (C51/BL51) with no off-chip memory. I have two functions with parameters:
void Detect( U8 iLed )
and
static U8 INHSampleHandler( U16 u16Sample )
Now I understand that Keil will allocate a variable (in DATA) for these. The problem seems to be that the locator is using the same memory location for both. I cannot understand why.
Below are excerpts from the scratchpad showing 2 "D:0026H". These are the only places these symbols are declared. Any ideas what I'm doing wrong?
Thanks, Jeff
BL51 BANKED LINKER/LOCATER V5.12 07/14/2011 09:36:23 PAGE 1 BL51 BANKED LINKER/LOCATER V5.12, INVOKED BY: Z:\TOOLS\SOFTWARE\KEIL\BL51.EXE Z:\Software\FB_CPU_Init.obj, >> Z:\Software\Settings.obj, Z:\Software\Glo >> bals.obj, Z:\Software\Devices\Clock.obj, Z:\ >> Software\Devices\Flash.obj, Z:\Software\Devices\HMI.obj >> , Z:\Software\Devices\INH.obj, Z:\ >> Software\Devices\ADC.obj, Z:\Software\Devices\Timer.obj, Z >> :\Software\Builds\TestINH - 06-00039-21-09\Main.obj >> , Z:\Software\Test\Test_Button.obj, Z:\So >> ftware\Builds\TestINH - 06-00039-21-09\Version.obj TO Z:\ >> Software\Builds\TestINH - 06-00039-21-09\06-00039-21-09-xx.wsp >> RS (256) PL (68) PW (78) XDATA (?XD?SETTINGS (0X0)) CODE (?CO?VERSION (0X7 >> FC0)) MEMORY MODEL: SMALL Deleted for brevity ------- PROC _INHSAMPLEHANDLER D:0026H SYMBOL u16Sample C:0BF1H LINE# 150 C:0BF5H LINE# 151 C:0BF5H LINE# 207 C:0BF7H LINE# 208 ------- ENDPROC _INHSAMPLEHANDLER ------- ENDMOD INH Deleted for brevity C:09FEH PUBLIC _Detect C:074EH PUBLIC main ------- PROC _DETECT D:0026H SYMBOL iLed
both of you are correct - with IB being more correct.
it is true that calling a function from within the isr should be avoided as much as possible. but that's not always possible, and I would further state that it is unlikely to be possible if you are relying pre-made code modules.
in that sense, IB is more correct that you can do the undesirable / risk thing of calling a function from within an isr, as long as you know what you are doing.
sometimes, it is optimal to do the sub-optimal thing.
Disagree.
Because Keil C51 functions are inherently not reentrant (due to the Overlaying) there are potential problems with calling Keil C51 functions from ISRs - specifically, when those functions are also called from non-ISR code.
I think the Linker will warn of this: http://www.keil.com/support/man/docs/bl51/bl51_l15.htm
So, as a blanket statement, I think "There's NO problem with calling a function from an '51 ISR" is less helpful than, "If at all possible, avoid calling any functios from a '51 ISR, period"
There is also the general issue (not specific to Keil or the 8051) that ISRs should generally be "short and sweet" - and calling functions generally indicates that the programmer has forgotten (or not understood) this...
If at all possible, avoid calling any functios from a '51 ISR, period.
There's NO problem with calling a function from an '51 ISR.
However, like most tasks, the programmer needs to understand the mechanism(s) involved and take appropriate steps to ensure the cogs fit correctly.
FWIW, we have plenty of code that calls functions from ISRs. It's all written in assembler and there have been no reported problems. Quite a few of those functions are inherently re-entrant. They are called from both ISR and mainline code. Most importantly, parameters and local variables are passed in registers.
No problem, so long as you know what you're doing, period.
Hello,
You are focusing in on the wrong section of the memory map. You need to view the "overlay analysis" section
If the overlay analysis is incorrect, you can use the OVERLAY command to correct it:
http://www.keil.com/support/man/docs/bl51/bl51_overlay.htm
For example look at page 5 of this application note.
http://www.keil.com/appnotes/docs/apnt_129.asp
It explains exactly the issue you describe, a function called by a function pointer.
Page 5 -6 show how to use the OVERLAY command to tell the linker that these functions are not independent. Once the linker is made aware of this, it will not overlay those variables.
The linker builds its call tree and don't think the Sub2 and Sub1 overlap. But obviously they can.
The problem is precisely that that is not at all obvious to the linker. Static call tree analysis pretty much always fails as soon as function pointers get involved, and deep down, it really has no chance --- there's an impossible task hiding in there, equivalent to the Halting Problem. Keil is no exception to that rule.
So what you should do is: do not call functions via pointer from inside an ISR. Not on a '51, anyway. If at all possible, avoid calling any functios from a '51 ISR, period.
If you really can't see any way of accomplishing that, see the app notes already referenced in this thread about ways to fudge the linker call tree data. And brace yourself, because this will hurt.
"I'll see if I can work up an example that actually fails."
I have one that follows your basic structure but uses a tmr isr to trigger one of the subroutines.
as andrew pointed out, you need to make sure that both subroutines actually operate on those two variables.
"please not the word basically"
is it too much to ask that you write complete and comprehensible sentences, erik?
I've tried to make my example more "complicated" but that darn optimizer is too smart
try optimize = 2. this is basically variable overlay and nothing else.
to the homewrecker and the sardine: please not the word basically
Erik
Make sure your functions make use of the parameter, and have them write the parameter value to a volatile variable. Volatile variables are normally a good way to get the optimizer to stop removing code - you want to convince the compiler that there is an important side effect that must not be optimized away.
I've tried to make my example more "complicated" but that darn optimizer is too smart. On the other hand my actual code is more than one swallow. I'll see if I can work up an example that actually fails.
No, I don't think that you have demonstrated it here.
That may or may not mean that there is no problem - it might just be because the example presented here is over-simplified so that the problem doesn't arise.
Probably worth a discussion with Keil support - likely to be easier to share a more substantial example with them...
"I think I've demonstrated the problem here"
I am not as confident.
I have tried to replicate your code, using a timer isr instead of your adc isr. Both variables are allocated at the same address.
and I never observed them having the same value.
so unless shown otherwise, I am going to agree with others that you are trying to solve a problem that doesn't exist.
Another thing is to disable passing parameters in registers, and make sure that the functions do something:
#pragma NOREGPARMS #include "c8051f9xx.h" typedef unsigned char U8; typedef unsigned int U16; U8 Sub1( U8 SubVar1 ) // No 'volatile' { SubVar1 = SubVar1 + 1; return SubVar1; } U8 Sub2( U8 SubVar2 ) // No 'volatile' { SubVar2 = SubVar2 + 1; return SubVar2; } void main(void) { while(1) { Sub1( 13 ); } } static void ADC_ISR(void) interrupt 10 { U16 u16Result = 12; Sub2( u16Result ); ADC0CN &= ~0x20; }
Which then gives:
; FUNCTION Sub1 (BEGIN) ; SOURCE LINE # 7 ; SOURCE LINE # 8 ; SOURCE LINE # 9 0000 0500 R INC SubVar1 ; SOURCE LINE # 10 0002 AF00 R MOV R7,SubVar1 ; SOURCE LINE # 11 0004 ?C0001: 0004 22 RET ; FUNCTION Sub1 (END) ; FUNCTION Sub2 (BEGIN) ; SOURCE LINE # 13 ; SOURCE LINE # 14 ; SOURCE LINE # 15 0000 0500 R INC SubVar2 ; SOURCE LINE # 16 0002 AF00 R MOV R7,SubVar2 ; SOURCE LINE # 17 0004 ?C0002: 0004 22 RET ; FUNCTION Sub2 (END)
------- MODULE OVERLAY C:0000H SYMBOL _ICE_DUMMY_ D:00E8H PUBLIC ADC0CN D:00A8H PUBLIC IE C:0003H SYMBOL ADC_ISR D:00B8H PUBLIC IP C:0024H PUBLIC main D:0088H PUBLIC TCON C:002CH PUBLIC Sub1 C:0031H PUBLIC Sub2 D:0098H PUBLIC SCON0 D:00D0H PUBLIC PSW ------- PROC SUB1 D:0008H SYMBOL SubVar1 C:002CH LINE# 7 C:002CH LINE# 8 C:002CH LINE# 9 C:002EH LINE# 10 C:0030H LINE# 11 ------- ENDPROC SUB1 ------- PROC SUB2 D:0009H SYMBOL SubVar2 C:0031H LINE# 13 C:0031H LINE# 14 C:0031H LINE# 15 C:0033H LINE# 16 C:0035H LINE# 17 ------- ENDPROC SUB2
To use the keyword "volatile" was my first thought, and it worked for a while in my program. In this simplistic program it works as well. As I continued working with my code the problem got "unfixed".
I think I've demonstrated the problem here and I'm not confident that this solution with "volatile" is at all guaranteed. I'd guess that we a bit afield of the ANSI standard it it's up to how Keil designed their optimization.
I'll probably send this over to Keil technical support.
If I change the source code to:
#include "c8051f9xx.h" typedef unsigned char U8; typedef unsigned int U16; void Sub1( volatile U8 SubVar1 ) { SubVar1 = SubVar1 + 1; } void Sub2( volatile U8 SubVar2 ) { SubVar2 = SubVar2 + 1; } void main(void) { while(1) { Sub1( 13 ); } } static void ADC_ISR(void) interrupt 10 { U16 u16Result = 12; Sub2( u16Result ); ADC0CN &= ~0x20; }
The result is:
; FUNCTION _Sub1 (BEGIN) ; SOURCE LINE # 6 0000 8F00 R MOV SubVar1,R7 ; SOURCE LINE # 7 ; SOURCE LINE # 8 0002 0500 R INC SubVar1 ; SOURCE LINE # 9 0004 22 RET ; FUNCTION _Sub1 (END) ; FUNCTION _Sub2 (BEGIN) ; SOURCE LINE # 11 0000 8F00 R MOV SubVar2,R7 ; SOURCE LINE # 12 ; SOURCE LINE # 13 0002 0500 R INC SubVar2 ; SOURCE LINE # 14 0004 22 RET ; FUNCTION _Sub2 (END)
And the map file says:
------- MODULE OVERLAY C:0000H SYMBOL _ICE_DUMMY_ D:00E8H PUBLIC ADC0CN D:00A8H PUBLIC IE C:0003H SYMBOL ADC_ISR D:00B8H PUBLIC IP C:0023H PUBLIC main D:0088H PUBLIC TCON C:002AH PUBLIC _Sub1 C:002FH PUBLIC _Sub2 D:0098H PUBLIC SCON0 D:00D0H PUBLIC PSW ------- PROC _SUB1 D:0008H SYMBOL SubVar1 C:002AH LINE# 6 C:002CH LINE# 7 C:002CH LINE# 8 C:002EH LINE# 9 ------- ENDPROC _SUB1 ------- PROC _SUB2 D:0009H SYMBOL SubVar2 C:002FH LINE# 11 C:0031H LINE# 12 C:0031H LINE# 13 C:0033H LINE# 14 ------- ENDPROC _SUB2
View all questions in Keil forum