Sanity check, compiler generated ISR

Go To Last Post
12 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am working with AVR-GCC and using Ob Dev's AVR-USB driver. It has a spec for maximum time that interrupts can be disabled. The recommended solution is to sei in your interrupt routines. I am just trying to keep my ISR as short as possible. I believe I am under the spec for my clock frequency but would like a little more cushion . Here is the assembler that was generated from my ISR code.

PUSH R1
PUSH R0
IN R0,0x3F
PUSH R0
CLR R1
PUSH R24
IN R24,0x05
STS 0x0091,R24
LDI R24,0x01
STS 0x0078,R24
POP R24
POP R0
OUT 0X3F,R0
POP R0
POP R1
RETI

I feel fairy certain that I can shorten this to:

PUSH R24
IN R24,0x05
STS 0x0091,R24
LDI R24,0x01
STS 0x0078,R24
POP R24
RETI

Am I right?
Thanks,
David

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

Thanks to whoever decided to move this but I am not asking specifically about GCC. I am asking if the assembly routine I wrote would work just as well as the one the compiler generated.

Anyway, I'm pretty sure it will work. I just wasn't sure if maybe the R0 and R1 registers were used in the core in some way I wasn't aware of. I already checked the effects of the instructions on the SREG and none of them effect it according to the instruction reference. Thanks.

David.

Last Edited: Fri. Jan 16, 2009 - 01:15 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I vote with you. Your code doesn't modify r0, r1 or the status register, so you don't need to save and restore any of those.

Mike

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

Yes, just what I thought. Thank you for the confirmation.

David.

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

Quote:

I feel fairy certain that I can ...

... wave my magic wand and ... ;)

Quote:

Thanks to whoever decided to move this but I am not asking specifically about GCC.

I'd just let my compiler do an appropriate skinny ISR:

unsigned char value;
unsigned char flag;
// External Interrupt 0 service routine
interrupt [EXT_INT0] void ext_int0_isr(void)
{
value = PIND;
flag = 1;
}
                 	.CSEG
                 _ext_int0_isr:
00005d 93ea      	ST   -Y,R30
                 ;      17 value = PIND;
00005e b1e9      	IN   R30,0x9
00005f 93e0 0200 	STS  _value,R30
                 ;      18 flag = 1;
000061 e0e1      	LDI  R30,LOW(1)
000062 93e0 0201 	STS  _flag,R30
                 ;      19 }
000064 91e9      	LD   R30,Y+
000065 9518      	RETI

or similar with register variables/flags.

The different compilers will have different models of code generation, but true enough there is no AVR "core" meddling here.

Lee

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

Thanks for the answer Lee. The first ISR is of course what GCC came up with. I tried different optimizations but it always generated the same code. I knew I could use an attribute to keep GCC from generating the push and pop and just do it myself. I just wanted to be sure that the stuff I thought was unnecessary really was.

The comment about it being moved was because I was hoping an assembler guru would see it and chime in.

So the down side is that every time I change something I will have to double check the assembly for my ISRs.

Thanks again for the attention guys.
David.

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

Yes, that should work.

You did not use any instructions that change status bits, so it should not be necessary to save SREG (0x3F), and you didn't touch R0 or R1.

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

The ASM ISR Lee showed looks flawed. It is chaning a more or less random place in RAM where the Y registers is pointing to. Its much better to save R30 to the stack.
If this code is compiler generated I would condsider this a compiler bug, unless Y register and the RAM it points to is expecitely reserved for saving R30 during the ISR. In this case it would be a stupid program because it would be easier to use YL or YH direktly instead of R30.

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

Quote:

The ASM ISR Lee showed looks flawed.

Perhaps to those unfamiliar with dual-stack code generation models for AVR processors.

Quote:
It is chaning a more or less random place in RAM where the Y registers is pointing to.

It is using the data stack.

Quote:
Its much better to save R30 to the stack.

It is saving it to the data stack.

"Much better"? What criteria determine that? Smaller code? One word, right? Faster code? Two cycles, right?

Lee

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

theusch wrote:
"Much better"? What criteria determine that? Smaller code? One word, right? Faster code? Two cycles, right?
Lee

"Much better" coming from the (misinformed) assumption that the data was being saved in a random location, potentially clobbering whatever other data lived there. Surely, operating from that assumption, the potential negative side-effects of saving to a random location would have made it a poor choice.

You and I know that CodeVision actually reserves the Y pointer to keep track of a data stack that is separate from the return address stack, and therefore the potential random clobbering problem is actually a non-issue; apparently Klienstien didn't know that.

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

Yeah, I should have ignored the thread being in the GCC forum. I thought that

Quote:
I am not asking specifically about GCC.
made it somewhat fair game.

-- Most of the AVR C compilers use a dual-stack model. Better or worse than single-stack? That has been thrashed out several times. For this thread it doesn't matter what the answer is. The fragment ISR that I posted is indeed CodeVision and the ISR register save/restore for a simple example could be done PUSH/POP in this case with the same size and speed of generated code.

-- Yes, I was indeed tweaking the reading audience with the CV "smart ISRs" which does a quite nice job of making skinny ISRs with straight C Code.

Lee

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

Actually yes, I wanted it to be fair game and I thank you Lee for your contribution. It is something I will think about if I ever decide to invest in a compiler.

David.