ISR with function arguments (in a modular driver approach)

Go To Last Post
61 posts / 0 new

Pages

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The code here was to separate the HAL (which contains very HW specific code) from a generic timer or serial application that works by using interrupts. This application is compatible with an AVR or a very different ARM (just to mention one architecture I used, was a CC3200). The "code architecture" was important because I was looking for making a HAL with the same interfaces regardless of the underneath MCU architecture. So for example, because the ISR() statement is very specific with avr-gcc and the AVR, the only solution to keep this abstraction, was to put it inside the interrupt hal, making available the interfaces of this hal which are independent from the architecture: is just boiled down to an interrupt init and a callback set which will be handled correctly inside the hal, according to the MCU architecture. But the interrupt init and the callback set are always the same on the application level.

 

So all the differences are hidden in the interrupt hal (which contains the ISR() if using an AVR or an IVT if using an ARM). I am not sure if that is what you asked. Did I answered?

 

And no, I haven't studied the Asm from this. But what you mean by register preservation? The one in stack? Why they should not be preserved? I see the function ending and turning back from the call of the callback array, which is in the ISR() statement.
I only verified the overhead of asm instructions which was mentioned here.

Last Edited: Sat. Feb 8, 2020 - 03:40 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

And I definitely need to understand better the assembly. It is not what happens only in the ISR which worries me (because this might be optimized on ARMs), but the overhead of instructions I get when abstracting the registers. I am not sure this can change if changing MCU architecture.

For example, when writing to TCNT0 (line 00000B2D), via the array of registers gives a lot more indexing instructions rather than the following direct assignment (at line 00000B37 )

    *tcnt[handle->channel] = val & 0xff;
00000B2D  LD R30,X        Load indirect
00000B2E  LDI R31,0x00        Load immediate
00000B2F  LSL R30        Logical Shift Left
00000B30  ROL R31        Rotate Left Through Carry
00000B31  SUBI R30,0x76        Subtract immediate
00000B32  SBCI R31,0xFE        Subtract immediate with carry
00000B33  LD R0,Z+        Load indirect and postincrement
00000B34  LDD R31,Z+0        Load indirect with displacement
00000B35  MOV R30,R0        Copy register
00000B36  STD Z+0,R22        Store indirect with displacement
    TCNT0 = val;
00000B37  OUT 0x26,R22        Out to I/O location 

I wonder if there is a way to let the compiler natively optimize or writing some inline code to reduce the instructions while keeping the abstraction.
Or probably I am asking for something impossible...
 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm talking about the ISR prologue/epilogue.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Regarding the fact register preservation gets lost, I am not sure I understood what you meant.

To try understand, I made the experiment with the task tick increment, which is a variable in the scope of the "scheduler" application.
The ISR with no callback is:
 

ISR(TIMER1_COMPA_vect)
{
  task_systick_increment_ISR();
}

And brings this disassembly:
 

{
00000256  PUSH R1		Push register on stack
00000257  PUSH R0		Push register on stack
00000258  IN R0,0x3F		In from I/O location
00000259  PUSH R0		Push register on stack
0000025A  CLR R1		Clear Register
0000025B  PUSH R18		Push register on stack
0000025C  PUSH R19		Push register on stack
0000025D  PUSH R20		Push register on stack
0000025E  PUSH R21		Push register on stack
0000025F  PUSH R22		Push register on stack
00000260  PUSH R23		Push register on stack
00000261  PUSH R24		Push register on stack
00000262  PUSH R25		Push register on stack
00000263  PUSH R26		Push register on stack
00000264  PUSH R27		Push register on stack
00000265  PUSH R30		Push register on stack
00000266  PUSH R31		Push register on stack
task_systick_increment_ISR();
00000267  CALL 0x00000CD2		Call subroutine
}
00000269  POP R31		Pop register from stack
0000026A  POP R30		Pop register from stack
--- hal/atmega328/interrupt_hal.c
0000026B  POP R27		Pop register from stack
0000026C  POP R26		Pop register from stack
0000026D  POP R25		Pop register from stack
0000026E  POP R24		Pop register from stack
0000026F  POP R23		Pop register from stack
00000270  POP R22		Pop register from stack
00000271  POP R21		Pop register from stack
00000272  POP R20		Pop register from stack
00000273  POP R19		Pop register from stack
00000274  POP R18		Pop register from stack
00000275  POP R0		Pop register from stack
00000276  OUT 0x3F,R0		Out to I/O location
00000277  POP R0		Pop register from stack
00000278  POP R1		Pop register from stack
00000279  RETI 		Interrupt return
{

But with the callback:
 

ISR(TIMER1_COMPA_vect)
{
	isr_pt_array[TIMER1_COMPA_vect_num].isr_pt();
}

Generates:

{
00000256  PUSH R1		Push register on stack
00000257  PUSH R0		Push register on stack
00000258  IN R0,0x3F		In from I/O location
00000259  PUSH R0		Push register on stack
0000025A  CLR R1		Clear Register
0000025B  PUSH R18		Push register on stack
0000025C  PUSH R19		Push register on stack
0000025D  PUSH R20		Push register on stack
0000025E  PUSH R21		Push register on stack
0000025F  PUSH R22		Push register on stack
00000260  PUSH R23		Push register on stack
00000261  PUSH R24		Push register on stack
00000262  PUSH R25		Push register on stack
00000263  PUSH R26		Push register on stack
00000264  PUSH R27		Push register on stack
00000265  PUSH R30		Push register on stack
00000266  PUSH R31		Push register on stack
	isr_pt_array[TIMER1_COMPA_vect_num].isr_pt();
00000267  LDS R30,0x030F		Load direct from data space
00000269  LDS R31,0x0310		Load direct from data space
0000026B  ICALL 		Indirect call to (Z)
--- user/hal/atmega328/interrupt_hal.c
}
0000026C  POP R31		Pop register from stack
0000026D  POP R30		Pop register from stack
0000026E  POP R27		Pop register from stack
0000026F  POP R26		Pop register from stack
00000270  POP R25		Pop register from stack
00000271  POP R24		Pop register from stack
00000272  POP R23		Pop register from stack
00000273  POP R22		Pop register from stack
00000274  POP R21		Pop register from stack
00000275  POP R20		Pop register from stack
00000276  POP R19		Pop register from stack
00000277  POP R18		Pop register from stack
00000278  POP R0		Pop register from stack
00000279  OUT 0x3F,R0		Out to I/O location
0000027A  POP R0		Pop register from stack
0000027B  POP R1		Pop register from stack
0000027C  RETI 		Interrupt return 

In the first case, the direct ISR is called with

CALL 0x00000CD2	

While with the callback, is
 

00000267  LDS R30,0x030F		Load direct from data space
00000269  LDS R31,0x0310		Load direct from data space
0000026B  ICALL 		Indirect call to (Z) 

With the expected overhead discussed before because is a function call, it seems to save the stack exactly in both cases. It is even documented in FreeRTOS documentation this behaviour.

So what is the data which is not preserved you are referring to?

 

 

Just for reference, putting the increment directly in the ISR (which normally is not done in a RTOS):

00000256  PUSH R1		Push register on stack
00000257  PUSH R0		Push register on stack
00000258  IN R0,0x3F		In from I/O location
00000259  PUSH R0		Push register on stack
0000025A  CLR R1		Clear Register
0000025B  PUSH R24		Push register on stack
0000025C  PUSH R25		Push register on stack
0000025D  PUSH R26		Push register on stack
0000025E  PUSH R27		Push register on stack
task_system_tick+=1000;
0000025F  LDS R24,0x01BF		Load direct from data space
00000261  LDS R25,0x01C0		Load direct from data space
00000263  LDS R26,0x01C1		Load direct from data space
00000265  LDS R27,0x01C2		Load direct from data space
00000267  SUBI R24,0x18		Subtract immediate
00000268  SBCI R25,0xFC		Subtract immediate with carry
00000269  SBCI R26,0xFF		Subtract immediate with carry
0000026A  SBCI R27,0xFF		Subtract immediate with carry
0000026B  STS 0x01BF,R24		Store direct to data space
0000026D  STS 0x01C0,R25		Store direct to data space
0000026F  STS 0x01C1,R26		Store direct to data space
--- hal/atmega328/interrupt_hal.c
00000271  STS 0x01C2,R27		Store direct to data space
}
00000273  POP R27		Pop register from stack
00000274  POP R26		Pop register from stack
00000275  POP R25		Pop register from stack
00000276  POP R24		Pop register from stack
00000277  POP R0		Pop register from stack
00000278  OUT 0x3F,R0		Out to I/O location
00000279  POP R0		Pop register from stack
0000027A  POP R1		Pop register from stack
0000027B  RETI 		Interrupt return 

Essentially save fewer registers because there is no need to address any pointer or call.
In all the cases, with the proper declaration, task_system_tick does not lose its value on the next call.

Or did you refer to something else?

Last Edited: Sat. Feb 8, 2020 - 09:56 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
We see HAL threads here regularly but exactly how likely is it that you are going to reuse very hardware-specific AVR code on some other processor?

I also got what you mean by this. Well, abstracting or parameterizing if you plan to use the same or couple of device at most, is not worth. If the device follows a common architecture on variations which could include a lot more instantiations of the HW, like a soft-processor in an FPGA, or an arbitrary ARM-based device, I think it's at least worth to know how to parameterize properly. Or at least how to integrate a non-parameterized HW specific code with a generic HAL with standard, HW independent interfaces, at least for the common functionalities.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

thexeno wrote:
The ISR with no callback is:
surely you can see that things have already gone wrong at that stage? Build  an ISR that has NOTHING within the braces and compare.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am sorry, but what is the purpose of an empty ISR? If you mean an ISR that has no function calls, is in the last example.

 

But following to the letter what you suggest, here is the output of an empty one:

 

0000028D  PUSH R1		Push register on stack
0000028E  PUSH R0		Push register on stack
0000028F  IN R0,0x3F		In from I/O location
00000290  PUSH R0		Push register on stack
00000291  CLR R1		Clear Register
}
00000292  POP R0		Pop register from stack
00000293  OUT 0x3F,R0		Out to I/O location
00000294  POP R0		Pop register from stack
00000295  POP R1		Pop register from stack
00000296  RETI 		Interrupt return 

I think is normal that is shorter, there are no instructions, no calls, neither info to be pushed into the stack.

Also, I don't think is always the best never putting function calls, meaning that the first thing you do when setting up an RTOS (not on an AVR, but on equally simple 8 or 16 bit architectures), is to put the function call of the SysTick in the IVT.
(I say this because it seems that the general message is to never put function call in the ISR, no matter what. Even worse a callback: but I just have proven it does create an overhead of only 4 clock cycles (2x LDS). I really have trouble to see where is the devil.

But still, can you answer the important question I made? Why data preservation goes lost? Because seems it is not to my tests, and if it does, I am checking in a very wrong way. I really checked also examples on RTOS and superloops, other general MCU projects (AVR and not), and I saw some projects using short functions inside the ISR, especially the more complex. One of them was the FreeRTOS example of the Tick. They also keep the data when re-entering the ISR. Also in some Arduino libraries, and optimized or not, these are working.

Last Edited: Sun. Feb 9, 2020 - 05:17 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Also as soon as ISRs start calling through function pointers the register preservation goes through the roof

I read that as the register preservation gets lost (out of the roof, out of the window, in the trash bin). Reading again now I guess you meant that gets very big, ain't you?

LOL - my mistake then. In this case, indeed makes sense, when comparing the ISRs.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

thexeno wrote:
I guess you meant that gets very big

Yes - "through the roof" as in "way, waaaay up"; ie, very high

https://dictionary.cambridge.org/dictionary/english/go-through-the-roof

 

 

maybe it's a peculiarly British idiom ?

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

awneil wrote:
maybe it's a peculiarly British idiom ?

I'd say it's an English language idiom as it's also common here in the US.

Letting the smoke out since 1978

 

 

 

 

Pages