why code in empty interrupt routine?

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

Hi there,

I have got the following interrupt routine:

ISR(TWI_vect)
{     
/* DO NOTHING, INTERRUPT FLAG IS NOT CLEARED VIA HARDWARE, SOFTWARE IS MONITORING INTERRUPT FLAG  */
}

which generates the following code:

ISR(TWI_vect)
{     
     fd2:	1f 92       	push	r1
     fd4:	0f 92       	push	r0
     fd6:	0f b6       	in	r0, 0x3f	; 63
     fd8:	0f 92       	push	r0
     fda:	11 24       	eor	r1, r1
/* DO NOTHING, INTERRUPT FLAG IS NOT CLEARED VIA HARDWARE, SOFTWARE IS MONITORING INTERRUPT FLAG  */
}
     fdc:	0f 90       	pop	r0
     fde:	0f be       	out	0x3f, r0	; 63
     fe0:	0f 90       	pop	r0
     fe2:	1f 90       	pop	r1
     fe4:	18 95       	reti

Isn't this unnecessary code? Is there a way to tell gcc to not generate the code?

Any help will be appreciated?

Thanks
Jacques

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

If you do not want the ISR() to do anything, you could just use a single "reti" instruction.

In practice, your ISR() will want to do something, least of which is to preserve the SREG.

I note that your example cites TWI. It is inevitable that you will be at least acknowledging the IRQ even if you do not react to each state.

You can create a whole load of non-portable avr-gcc "naked" code if you wish. But life is simpler if you just write regular C and let the Compiler use its own common sense.

David.

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

Declare it naked. But don't forget to explicitly put a "reti" in it then.

But I am confused by the comment in it. What exactly is the purpose of this empty ISR?

Stefan Ernst

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

There's a macro named EMPTY_INTERRUPT() which serves exactly that
purpose. I guess its main application is situations where you
technically need an (enabled) interrupt but don't care when the
ISR is actually called. In particular, waking up from sleep
comes to mind: if your code continues processing, you *know* the
interrupt must have happened, but often you don't really care,
as proceeding is the only thing you want.

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

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

Hi,

Jorg, yes the EMPTY_INTERRUPT() macro is exactly what I need, like you said the interrupt is just there to wake-up from sleep mode.

Thanks to all.

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

It is nevertheless slightly tedious that GCC produces far more code than strictly necessary for certain very small (but non-empty) interrupt service routines, eg:

volatile unsigned char WD_Flag;

ISR(WDT_vect) {
   WD_Flag = 1;
}

produces 14 instructions thus:

34:       extern "C" ISR(WDT_vect) {
+00000025:   921F        PUSH      R1             Push register on stack
+00000026:   920F        PUSH      R0             Push register on stack
+00000027:   B60F        IN        R0,0x3F        In from I/O location
+00000028:   920F        PUSH      R0             Push register on stack
+00000029:   2411        CLR       R1             Clear Register
+0000002A:   938F        PUSH      R24            Push register on stack
35:       	WD_Flag = 1;
+0000002B:   E081        LDI       R24,0x01       Load immediate
+0000002C:   9380006B    STS       0x006B,R24     Store direct to data space
36:       }
+0000002E:   918F        POP       R24            Pop register from stack
+0000002F:   900F        POP       R0             Pop register from stack
+00000030:   BE0F        OUT       0x3F,R0        Out to I/O location
+00000031:   900F        POP       R0             Pop register from stack
+00000032:   901F        POP       R1             Pop register from stack
+00000033:   9518        RETI                     Interrupt return

when all that is necessary is the following 5 instructions:

25:       extern "C" ISR(WDT_vect, ISR_NAKED) {
+00000025:   938F        PUSH      R24            Push register on stack
27:       	asm volatile (" LDI       R24, 1      ");
+00000026:   E081        LDI       R24,0x01       Load immediate
28:       	asm volatile (" STS       WD_Flag,R24 ");
+00000027:   9380006B    STS       0x006B,R24     Store direct to data space
29:       	asm volatile (" POP       R24         ");
+00000029:   918F        POP       R24            Pop register from stack
30:       	asm volatile (" RETI                  ");
+0000002A:   9518        RETI                     Interrupt return

I have not found an easy fix for this; use of ISR_NAKED like this:

ISR(WDT_vect, ISR_NAKED) {
    asm volatile (" push r??"); // save a register here, but which?
    WD_Flag = 1;
    asm volatile (" pop r??"); // matching restore 
    reti();
}

is no solution because the C programmer has no control over which register the compiler will use for the temporary. Therefore you can't know which register needs to be saved and restored manually. It looks like R24 would be a good guess and it may well work, but a compiler update could break that.

Caveat: there may be a way of doing this with GCC's inline assembler, but the full syntax is so complex that I have never had the time/inclination to work out how to use it fully.

I resign myself to the fact that this problem generally represents a significant waste of time and/or space only for the very simplest and shortest ISR functions, and those I can easily write in assembler from the outset.

Christopher Hicks
==

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
ISR(WDT_vect, ISR_NAKED)
{
        uint8_t temp;
        __asm__ volatile("push %[temp]" "\n\t"
                         "ldi %[temp], 1" "\n\t"
                         "sts WD_flag, %[temp]" "\n\t"
                         "pop %[temp]" "\n\t"
                         "reti":
                         [temp] "=d" (temp));
}

But of course, it would ultimately make more sense to write a separate
assembly source file rather than obfuscating that in inline assembly code.

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

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

I see, thanks. The syntax isn't too bad in this case. With this code, is the compiler guaranteed to allocate the temporary in a register? Even, for example, with -O0? Otherwise it clearly won't work.

dl8dtl wrote:
But of course, it would ultimately make more sense to write a separate
assembly source file rather than obfuscating that in inline assembly code.
Of course.

CH
==

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

A couple side points:

Quote:

/* DO NOTHING, INTERRUPT FLAG IS NOT CLEARED VIA HARDWARE, SOFTWARE IS MONITORING INTERRUPT FLAG */

I'm not a TWI person much, and never interrupt-driven TWIE. Is it correct that TWINT is kind of a different AVR interrupt flag? There are the "normal" ones that are cleared by entering the ISR. There are the persistent ones like UDRE and low-level where the ISR is repeatedly invoked. And this is different: no repetitive ISR invocation if the flag isn't cleared?

Quote:

the interrupt is just there to wake-up from sleep mode.

I've always put a sleep-disable in those ISRs, in case there is the race with sleep-enable in the main() and then the wakeup condition occurs before the sleep. There is a way around the race that I was taught a couple months ago by enabling interrupts immediately before the sleep--references upon request.

Quote:

It is nevertheless slightly tedious that GCC produces far more code than strictly necessary for certain very small (but non-empty) interrupt service routines, eg:

I won't disagree. For flag setting use GPIOR0 or other low I/O register and SBI--no temp register needed and no SREG flags affected. If you dedicate a GP register similar can be done.

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

> With this code, is the compiler guaranteed to allocate the
> temporary in a register?

Yes, it does. In a non-naked function, it would also ensure
it is properly saved before if needed.

Yes, I know this is far from being optimal. As I understand it,
the ISR prologue/epilogue is currently hard-coded, as nobody has
made the AVR backend smart enough to generate them automatically
to the degree that is really needed.

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

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

Lee wrote:

> I'm not a TWI person much, and never interrupt-driven TWIE. Is it correct
> that TWINT is kind of a different AVR interrupt flag?

Yes, it is. As I understand it (don't use much TWI myself), clearing TWINT
causes the next bus transaction to start, so it's crucial the user code always
has control over that.

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

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

I know it's an 8 year old conversation, still worth clarifying for folks like me who bump into it later.  theusch tried to say it, but I'm not sure how well it came through.  In naked interrupts, if you're going to write it entirely in asm, I see no need or meaning to having the compiler "allocate" any registers.  In normal subroutines the compiler can arrange so that the calling function "allocates" certain registers for the called function, but that's of course impossible in an interrupt since there is no way to know when it will happen.  The only thing the compiler can and does do is push the register values before use within the function itself, and restore them at the end of the function.  But that's exactly what the naked function disables. 

 

That's mostly fine so long as you do it yourself, and the examples above did that.  But I see no need for the compiler declaration of the temp variable.  Use any register you like, just put it back the way you found it.    However probably nobody should do this if they don't know what they're doing.  It seems there's a good reason R0, R1, and SREG are saved, and that R1 is cleared, just not a good reason in a truly empty ISR.  if an ISR is only nearly empty it's probably good to understand why R0, R1, ans SREG are dealt with as they are, before attempting this.  I just wrote a significantly longer ISR (in C mostly, not asm) that resets at the end.  I didn't care what it clobbered, so this naked thing was just the trick (and this old thread was useful), but I did clear R1 first. 

 

I guess if mixing C and asm wthin the ISR then declaring the variable in C first is probably the right thing to do so that C knows about it. 

 

You can also just use a global register variable, but most of the time, that's probably not a great solution.

 

 

There is a useful document here:

http://www.atmel.com/webdoc/AVRL...

 

about what registers normal assembly subtroutines are allowed to use without worry and which ones they must restore.  But that doesn't really apply to interrupts (unless the interrupt calls a subroutine).

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

Doesn't the reti instruction clear the interrupt flag?

(From memory)

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

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

No it SET it

 

 

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

Paulvdh wrote:

Doesn't the reti instruction clear the interrupt flag?

(From memory)

The RETI instruction causes the CPU to re-enable interrupts. For classic 8-bit AVRs, that means writing a '1' to the I bit in SREG. (There's more to it than that in devices such as XMEGAs with hardware support for nested interrupts.)

 

With respect to the peripheral's interrupt request bit (which is what I think you're actually talking about):

For many peripherals, the hardware's act of jumping INTO the interrupt service routine in the first place automatically clears the corresponding interrupt request bit. However, for some peripherals that behavior would actually be undesirable, so in those cases it is left up to the application software to clear the bit at the appropriate time. The AVR's TWI peripheral is an example of such a case: you have to make a point of clearing the interrupt request bit yourself once you're satisfied that everything is ready to proceed.

Last Edited: Mon. Jan 30, 2017 - 12:32 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

darm site deletes your post if it asks you to re-login in!  Grr..  I had quite a detailed answer, probably shorter this time.

 

SREG is not only for the interrupt flag.  It contains overflow bits etc.  Several assembler commands modify SREG and a few modify R0 and R1 (particularly multiply and program memmory operations).  A few even modify adjacent registers.  You have to check the instruction set manual.  I just wrote a simple interrupt to increment a global timer overflow variable.  It needs lds, inc and sts.  None of that touches R0 but inc changes the SREG.  I have no idea where in the code the interrupt will happen so how can I predict if something is about to check the value of the overflow flag when the interrupt reti's?  I can't.  maybe nothing in this code does now, but sensible programming doesn't lay down land mines for the future.  I'm likely to even step on it myself if I do that, never mind other people.  So I at least need to store SREG.

 

I'm not sure about the interrupt flag. The data sheet says very clearly, I just don't remember, but it's essentially a separate issue to me.

 

So here is the short version I wrote (from inline asm, 0x3F is the status register and 0x007E is the variable to increment):

 

 PUSH R2        Push register on stack 
 IN R2,0x3F        In from I/O location 
 PUSH R2        Push register on stack 
 LDS R2,0x007E        Load direct from data space 
 INC R2        Increment 
 STS 0x007E,R2        Store direct to data space 
 POP R2        Pop register from stack 
 OUT 0x3F,R2        Out to I/O location 
 POP R2        Pop register from stack 
 RETI         Interrupt return 

 

and the non-naked compiler's version of simply  { counter++;}

 

PUSH R1        Push register on stack 
 PUSH R0        Push register on stack 
 IN R0,0x3F        In from I/O location 
 PUSH R0        Push register on stack 
 CLR R1        Clear Register 
 PUSH R24        Push register on stack 
 LDS R24,0x007E        Load direct from data space 
 SUBI R24,0xFF        Subtract immediate 
 STS 0x007E,R24        Store direct to data space 
 POP R24        Pop register from stack 
 POP R0        Pop register from stack 
 OUT 0x3F,R0        Out to I/O location 
 POP R0        Pop register from stack 
 POP R1        Pop register from stack 
 RETI         Interrupt return 

 

Well I saved 10 bytes I think. Amusing that one byte came from subtracting 255 instead of just "inc" ing. Weird, maybe it knows something I don't yet.  If that's worth it to you to worry about it ok (it is for me, today anyway).  

 

It's disappointing that the compiler doesn't even re-use R0.  Apparently it considers it completely off limits.  If it at least did that it would be about 5 bytes difference and even less worth it.

 

I'm pretty new to this so if I got anything wrong let me know, but that's how I see it all.

Last Edited: Tue. Jan 31, 2017 - 02:04 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Oh yes, forgot the second time around.  Using the clobbers section in extended asm doesn't work.  It is only aware of interferences of other C code within the section.  It doesn't deal with the interrupt clobbering what it interrupted.  I tried.  That's what the pre/post amble code is for I suppose and naked disables it.  So you have to write the push/pop yourself.

Last Edited: Tue. Jan 31, 2017 - 02:07 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

There are many ways to skin a cat!

 

The GCC for AVR is far from optimal (I would guess that it's better for a ARM).

 

From the old AVR's that only could do LPM to R0 they(the AVR back end people) decided  to have the zero register in R1.

 

Now the mul instructions came and changed R0 and R1 .

 

It has to clear R1 every time mul is used (should be an other register but to late to move!).

And R0 has the same problem.

 

The compiler use R25:R24 for basic int and R24 for bytes as a kind ACC, (the reason for that is that it's only register pair that can add and sub a const (0..63) to a word, that's not a pointer).

And for high registers r16..r31 there is no penalty for using add and sub with a constant on a AVR so inc and dec will just be handled as +1 -1 .  

 

If you want to get the optimal code generated I guess you have to write in ASM.

 

or try an other compiler I think that CV make the smallest/best AVR code for small direct things, and it can make good use of registers (it's build around the AVR), but as a C compiler in general I don't think it's to "clever".

 

 

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

Should I be using r24 then for that inc?  I just chose 2 at random, although I was aware there are limitations, hadn't delved into it yet. It compiled though.  

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

No that is just what the AVR GCC compiler do, CV use Z (R31:R30) for every thing.

 

It's just if you use a high register you can can add any constant with the same code and therefor make the same base code if you add 1 or add 5.

(remember that the AVR don't have a ADDI you simply SUBI -n).

 

Have in mind that INC/DEC don't change carry, add and sub do, so if you write optimal ASM code where the flags survive through code there could be a difference.

it's like there is a big difference in different ways to clear R24 :

 

CLR R24  (instruction don't exist it make a EOR R24,R24) so S,V,N =0 Z=1 and C is unchanged.

SUB R24,R24 all logic flags change importand C change!

LDI R24,0x00  no flags are changed

 

Whats best depends :) have fun

 

 

 

 

 

 

 

Last Edited: Tue. Jan 31, 2017 - 04:05 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Flintrock wrote:
Well I saved 10 bytes I think.

sparrow2 wrote:
The GCC for AVR is far from optimal ...

 

Here we go again. ;) 

 

GCC does many "good" optimizations".  "Far from optimal"?  Perhaps; "far" is a bit nebulous.

 

But anyway, Flintrock -- apparently >>you<< have chosen to use the infinite-value toolchain.  As I've often mentioned in these threads, GCC ain't that good at skinny ISRs.

 

With another toolchain:

                 ;unsigned char counter;
                 ;// Timer 0 overflow interrupt service routine
                 ;interrupt [TIM0_OVF] void timer0_ovf_isr(void)
                 ;0000 0005 {

                 	.CSEG
                 _timer0_ovf_isr:
                 ;.FSTART _timer0_ovf_isr
000054 93ea      	ST   -Y,R30
000055 b7ef      	IN   R30,SREG
000056 93ea      	ST   -Y,R30
                 ;0000 0006 counter++;
000057 91e0 0108 	LDS  R30,_counter
000059 5fef      	SUBI R30,-LOW(1)
00005a 93e0 0108 	STS  _counter,R30
                 ;0000 0007 }
00005c 91e9      	LD   R30,Y+
00005d bfef      	OUT  SREG,R30
00005e 91e9      	LD   R30,Y+
00005f 9518      	RETI
                 ;.FEND

There are also semi-naked facilities to dedicate a register for ISR use for the SREG saving, which saves a few cycles.  In practice, I end up tuning an ISR in such a manner once every few years; like to get best edge handling rate for quadrature encoding.

 

[The above is CodeVision.  But the "far from optimal" GCC does better optimization than CV in certain other instances.  Peephole is always dangerous. ;) ]

 

[[and in e.g. that quadrature handling, I'll keep my "counter" in a register variable...]]

                 ;register unsigned char counter;
                 ;// Timer 0 overflow interrupt service routine
                 ;interrupt [TIM0_OVF] void timer0_ovf_isr(void)
                 ;0000 0005 {
                 
                 	.CSEG
                 _timer0_ovf_isr:
                 ;.FSTART _timer0_ovf_isr
000054 93ea      	ST   -Y,R30
000055 b7ef      	IN   R30,SREG
                 ;0000 0006 counter++;
000056 9423      	INC  R2
                 ;0000 0007 }
000057 bfef      	OUT  SREG,R30
000058 91e9      	LD   R30,Y+
000059 9518      	RETI
                 ;.FEND

 

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.

Last Edited: Tue. Jan 31, 2017 - 05:04 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Theusch I find your reply a little cryptic (intentionally?).  The only differences I see in your first version and mine is the use of Y as a stack pointer and, the difference I already pointed out of  using subi instead of inc.   Inifinite value?   Update: ok, I thought you were referring to the carry flag but I think your point is just that some other compiler can produce the same result as my hand written one.  Ok.

 

In your second example it appears (hopefully) that R2 is a reserved global register, so it's not stored, so sure, that saves instructions.

 

But my real point was not that one should micro-optimize these or that I did it very well even, or that this is the best you can get with any compiler, rather that if one is coming from a background of C, with enough assembler knowledge to be dangerous, one should be a cautious doing this with anything other than an empy ISR, and shouldn't assume that declaring the variable in C first does much to make things safer (and it wasn't assumed above either really).  ISR_naked already disabled C's safety mechanism. 

 

I suppose getting C to provide the variable may be one method to avoid some reserved global register (like Y in your example).

 

 

 

Last Edited: Wed. Feb 1, 2017 - 02:25 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

https://gcc.gnu.org/onlinedocs/g... wrote:

naked

This attribute allows the compiler to construct the requisite function declaration, while allowing the body of the function to be assembly code. The specified function will not have prologue/epilogue sequences generated by the compiler. Only basic asm statements can safely be included in naked functions (see Basic Asm). While using extended asm or a mixture of basic asm and C code may appear to work, they cannot be depended upon to work reliably and are not supported.

Moderation in all things. -- ancient proverb

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

This thread necromancy intrigues me. Yes, the compiler has a missed optimization. Yes you can hand code the ISR in Asm with only preservation of what it actually touches. I don't see very much point in fannying about with ISK_NAKED and then coding the body in the tortuous inline Asm syntax. Joerg's code in #7 produces this:

C:\SysGCC\avr\bin>type avr.c
#include <avr/io.h>
#include <avr/interrupt.h>

volatile unsigned char WD_Flag;

ISR(WDT_vect, ISR_NAKED)
{
        uint8_t temp;
        __asm__ volatile("push %[temp]" "\n\t"
                         "ldi %[temp], 1" "\n\t"
                         "sts WD_Flag, %[temp]" "\n\t"
                         "pop %[temp]" "\n\t"
                         "reti":
                         [temp] "=d" (temp));
}
int main(void) {
        while(1) {
                if (WD_Flag) {
                        PORTB = 0x55;
                        WD_Flag = 0;
                }
        }
}

C:\SysGCC\avr\bin>avr-gcc -Os -mmcu=atmega328p -save-temps avr.c -o avr.elf

C:\SysGCC\avr\bin>avr-objdump -S avr.elf

avr.elf:     file format elf32-avr


Disassembly of section .text:

00000000 <__vectors>:
   0:   0c 94 34 00     jmp     0x68    ; 0x68 <__ctors_end>
   4:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
   8:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
   c:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  10:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  14:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  18:   0c 94 48 00     jmp     0x90    ; 0x90 <__vector_6>
  1c:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  20:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  24:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  28:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  2c:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  30:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  34:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  38:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  3c:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  40:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  44:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  48:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  4c:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  50:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  54:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  58:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  5c:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  60:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  64:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>

00000068 <__ctors_end>:
  68:   11 24           eor     r1, r1
  6a:   1f be           out     0x3f, r1        ; 63
  6c:   cf ef           ldi     r28, 0xFF       ; 255
  6e:   d8 e0           ldi     r29, 0x08       ; 8
  70:   de bf           out     0x3e, r29       ; 62
  72:   cd bf           out     0x3d, r28       ; 61

00000074 <__do_clear_bss>:
  74:   21 e0           ldi     r18, 0x01       ; 1
  76:   a0 e0           ldi     r26, 0x00       ; 0
  78:   b1 e0           ldi     r27, 0x01       ; 1
  7a:   01 c0           rjmp    .+2             ; 0x7e <.do_clear_bss_start>

0000007c <.do_clear_bss_loop>:
  7c:   1d 92           st      X+, r1

0000007e <.do_clear_bss_start>:
  7e:   a1 30           cpi     r26, 0x01       ; 1
  80:   b2 07           cpc     r27, r18
  82:   e1 f7           brne    .-8             ; 0x7c <.do_clear_bss_loop>
  84:   0e 94 4e 00     call    0x9c    ; 0x9c <main>
  88:   0c 94 57 00     jmp     0xae    ; 0xae <_exit>

0000008c <__bad_interrupt>:
  8c:   0c 94 00 00     jmp     0       ; 0x0 <__vectors>

00000090 <__vector_6>:
  90:   8f 93           push    r24
  92:   81 e0           ldi     r24, 0x01       ; 1
  94:   80 93 00 01     sts     0x0100, r24
  98:   8f 91           pop     r24
  9a:   18 95           reti

0000009c <main>:
  9c:   85 e5           ldi     r24, 0x55       ; 85
  9e:   90 91 00 01     lds     r25, 0x0100
  a2:   99 23           and     r25, r25
  a4:   e1 f3           breq    .-8             ; 0x9e <main+0x2>
  a6:   85 b9           out     0x05, r24       ; 5
  a8:   10 92 00 01     sts     0x0100, r1
  ac:   f8 cf           rjmp    .-16            ; 0x9e <main+0x2>

000000ae <_exit>:
  ae:   f8 94           cli

000000b0 <__stop_program>:
  b0:   ff cf           rjmp    .-2             ; 0xb0 <__stop_program>

or to put it another way:

C:\SysGCC\avr\bin>type avr.s
        .file   "avr.c"
__SP_H__ = 0x3e
__SP_L__ = 0x3d
__SREG__ = 0x3f
__tmp_reg__ = 0
__zero_reg__ = 1
        .text
.global __vector_6
        .type   __vector_6, @function
__vector_6:
/* prologue: naked */
/* frame size = 0 */
/* stack size = 0 */
.L__stack_usage = 0
/* #APP */
 ;  9 "avr.c" 1
        push r24
        ldi r24, 1
        sts WD_Flag, r24
        pop r24
        reti
 ;  0 "" 2
/* epilogue start */
/* #NOAPP */
        .size   __vector_6, .-__vector_6
        .section        .text.startup,"ax",@progbits
.global main
        .type   main, @function
main:
/* prologue: function */
/* frame size = 0 */
/* stack size = 0 */
.L__stack_usage = 0
        ldi r24,lo8(85)
.L3:
        lds r25,WD_Flag
        tst r25
        breq .L3
        out 0x5,r24
        sts WD_Flag,__zero_reg__
        rjmp .L3
        .size   main, .-main
        .comm   WD_Flag,1,1
        .ident  "GCC: (GNU) 5.3.0"
.global __do_clear_bss

Surely, then it's just cleaner to simply write:

C:\SysGCC\avr\bin>type avr.c
#include <avr/io.h>
#include <avr/interrupt.h>

volatile unsigned char WD_Flag;

int main(void) {
        while(1) {
                if (WD_Flag) {
                        PORTB = 0x55;
                        WD_Flag = 0;
                }
        }
}

C:\SysGCC\avr\bin>type isr.S
#include <avr/io.h>

.global WDT_vect
WDT_vect:
        push r24                ; save
        ldi r24, 1
        sts WD_Flag, r24        ; change
        pop r24                 ; restore
        reti

C:\SysGCC\avr\bin>avr-gcc -Os -mmcu=atmega328p -save-temps avr.c isr.S -o avr.elf

C:\SysGCC\avr\bin>avr-objdump -S avr.elf

avr.elf:     file format elf32-avr


Disassembly of section .text:

00000000 <__vectors>:
   0:   0c 94 34 00     jmp     0x68    ; 0x68 <__ctors_end>
   4:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
   8:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
   c:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  10:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  14:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  18:   0c 94 48 00     jmp     0x90    ; 0x90 <__vector_6>
  1c:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  20:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  24:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  28:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  2c:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  30:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  34:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  38:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  3c:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  40:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  44:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  48:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  4c:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  50:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  54:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  58:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  5c:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  60:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>
  64:   0c 94 46 00     jmp     0x8c    ; 0x8c <__bad_interrupt>

00000068 <__ctors_end>:
  68:   11 24           eor     r1, r1
  6a:   1f be           out     0x3f, r1        ; 63
  6c:   cf ef           ldi     r28, 0xFF       ; 255
  6e:   d8 e0           ldi     r29, 0x08       ; 8
  70:   de bf           out     0x3e, r29       ; 62
  72:   cd bf           out     0x3d, r28       ; 61

00000074 <__do_clear_bss>:
  74:   21 e0           ldi     r18, 0x01       ; 1
  76:   a0 e0           ldi     r26, 0x00       ; 0
  78:   b1 e0           ldi     r27, 0x01       ; 1
  7a:   01 c0           rjmp    .+2             ; 0x7e <.do_clear_bss_start>

0000007c <.do_clear_bss_loop>:
  7c:   1d 92           st      X+, r1

0000007e <.do_clear_bss_start>:
  7e:   a1 30           cpi     r26, 0x01       ; 1
  80:   b2 07           cpc     r27, r18
  82:   e1 f7           brne    .-8             ; 0x7c <.do_clear_bss_loop>
  84:   0e 94 4e 00     call    0x9c    ; 0x9c <main>
  88:   0c 94 57 00     jmp     0xae    ; 0xae <_exit>

0000008c <__bad_interrupt>:
  8c:   0c 94 00 00     jmp     0       ; 0x0 <__vectors>

00000090 <__vector_6>:
  90:   8f 93           push    r24
  92:   81 e0           ldi     r24, 0x01       ; 1
  94:   80 93 00 01     sts     0x0100, r24
  98:   8f 91           pop     r24
  9a:   18 95           reti

0000009c <main>:
  9c:   85 e5           ldi     r24, 0x55       ; 85
  9e:   90 91 00 01     lds     r25, 0x0100
  a2:   99 23           and     r25, r25
  a4:   e1 f3           breq    .-8             ; 0x9e <main+0x2>
  a6:   85 b9           out     0x05, r24       ; 5
  a8:   10 92 00 01     sts     0x0100, r1
  ac:   f8 cf           rjmp    .-16            ; 0x9e <main+0x2>

000000ae <_exit>:
  ae:   f8 94           cli

000000b0 <__stop_program>:
  b0:   ff cf           rjmp    .-2             ; 0xb0 <__stop_program>

C:\SysGCC\avr\bin>

I got the same result and managed to do it typing something most human beings can actually read:

#include <avr/io.h>

.global WDT_vect
WDT_vect:
        push r24                ; save
        ldi r24, 1
        sts WD_Flag, r24        ; change
        pop r24                 ; restore
        reti

No sign of "naked", no sign of "[%temp]" or anything else. Just plain, clear code.

 

Even Joerg said:

dl8dtl wrote:
But of course, it would ultimately make more sense to write a separate assembly source file rather than obfuscating that in inline assembly code.

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

PS I was toying with whether my code should also have included:

#include <avr/io.h>

.extern WD_Flag

.global WDT_vect
WDT_vect:
        push r24                ; save
        ldi r24, 1
        sts WD_Flag, r24        ; change
        pop r24                 ; restore
        reti

but the user manual reminds us...

 

http://web.mit.edu/gnu/doc/html/...

for compatibility with other assemblers--but it is ignored. as treats all undefined symbols as external.

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

And I guess the next step is to have a (low)register like R2 reserved and always have the value 1 :)

 

Then the code will be :

 

 

WDT_vect:
        sts WD_Flag, r2
        reti

 

Then it only can be better if the code are placed in the vector table so we save the (r)jmp .

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

sparrow2 wrote:
Then it only can be better if the code are placed in the vector table so we save the (r)jmp .
That one is slightly trickier to arrange ;-)

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

Yes for a compiler devil

 

Here I guess that I need a disclaimer so theusch don't attack :

 

We are there here talking about GCC!

I'm sure that your CV can do a better job.

 

 

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

Yeah, what's that about?  Is he grumbling about the -fwrapv fiasco?  If so I'm sympathetic.  That's idiocy, but confusing newcomers with somewhat random and cryptic grumblings of disgruntled veterans probably isn't too useful. Theusch if you want to educate folks on the pitfalls of open source development(or whatever that was), maybe you should provide a link or some parentheses with your micro-rants instead of just confusing folks.  

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

Flintrock wrote:
Theusch if you want to educate folks on the pitfalls of open source development(or whatever that was), maybe you should provide a link or some parentheses with your micro-rants instead of just confusing folks.

???  What kind of links are you looking for?  Apparently, >>you<< chose a toolchain.  Apparently, the generated code offends you.  If skinny ISR handling is the topmost criterion for this app, then there are alternatives.  I posted compiler-produced code which I think is the same words and cycles than your hand-crafted ideal.

 

I said nothing about open-source.  Value=Worth/Cost, right?  GCC toolchain certainly has Worth.  Cost is 0.  Value is then infinite.

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.

Last Edited: Thu. Feb 2, 2017 - 03:00 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You said nothing about gcc either (well maybe you did in a language only >>you<< chose to speak).  Also my comment about was about this  thread.  Yes, I chose a toolchain.  Ok, the one we're talking about in this  thread.  I'm supposed to udnerstand what it means when you jump on me talking about >>you<< chose the "infinite value" tool chain... What the what?   The only place that expression even turns  up anywhere on the whole internet is some other thread with you in it.  It makes sense to you, not to anyone else. The links would have been to whaveter cryptic point you were referencing with the cryptic and somewhat confrontational seeming language.  

 

I'm I supposed to switch compilers now?  No thanks. 

Last Edited: Fri. Feb 3, 2017 - 03:52 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Flintrock wrote:
The only place that expression even turns up anywhere on the whole internet is some other thread with you in it.
Yeah but(*) all the regular readers know what Lee means by the phrase. He's quite right. While I choose avr-gcc not Codevision (though I recognise the latter is in many sense a "better" compiler) I agree that GCC can be called "infinite value" because it costs nothing and delivers lots and anything divided by 0 is infinite. I choose GCC simply because I've been using GCC on all kinds of targets and know a lot about its operation.

 

It is undeniable that it is really a very sad missed optimistation in that it generates the SREG preservation and R1 zeroing code in all ISR() code even when sometimes the body of the ISR neither changes SREG nor requires the 0x00 in R1.

 

If you read the Bugzilla for avr-gcc you will see why. I forget but I think the earliest mention of the issue was back in the very early 2000's (perhaps 2003..2006 time frame). The arv-gcc maintainers at the time gave their reasons for why it is the way it is and those conditions have not changed.

 

Obviously someone could perhaps engineer some kind of "post processor" that analyzed the generated code and if it determined ISR code had nothing touching SREG it might remove the SREG preservation and if it determined hat a 0x00 in R1 was not required it could remove that too. But for the very odd occasion where the few additional cycles of this missed optimization actually matter the programmer can go NAKED or implement in .s (my preferred solution) or try something else to mitigate this (-nostartfiles and a completely different IVT strategy perhaps being the ultimate?)

 

If one is writing really time sensitive code all the time then perhaps the investment in a copy of CodeVision is justified to save you have to make such workarounds? But I'm guessing the majority of ISRs do affect SREG (apart from setting a flag there's little you can do on the CPU that does not affect SREG). Whether they contain code that makes reliance on R1 holding 0x00 is perhaps another matter. BTW if you have ISR()s that do nothing but set a flag then just poll the state of the interrupts own xxIF flag and forget about setting a separate variable anyway.

 

(*) "yeah but" is officially registered as another "Lee Theusch'ism" too ;-)

 

EDIT: here's the GCC bug about this: https://gcc.gnu.org/bugzilla/sho...

Last Edited: Fri. Feb 3, 2017 - 04:21 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 It makes sense to you, not to anyone else.

It makes sense to anyone who has spent more than a week on these fora.  Folks which, I suppose, you'd call 'old-timers'.

 

You're on a roll, Flintrock.  Eight posts so far and fully 75% of them have been combative, sardonic, and peppered with thinly veiled disrespect, with the remainder seemingly intended to convey to the rest of us how smart you are.  This approach will not serve you well here, nor will it serve those you seek to enlighten.  Please, start from the assumption that no-one is attacking you, but that they are expressing their opinions, just as you are.  Perhaps then your rebuttals will seem less defensive.

 

I'll look forward to your contributions in the future.

 

 

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

Last Edited: Fri. Feb 3, 2017 - 04:25 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Flintrock wrote:
I'm I supposed to switch compilers now? No thanks.

Fair enough.  In the end, I was just pointing out that the code generation model of that particular toolchain doesn't lend itself well to the skinny-ISR aim with C constructs and toolchain extensions only.  And I also mentioned that IME "tuning" an ISR for the last microsecond (e.g. 8 cycles at 8MHz) only comes up occasionally.

 

Now, if you look at other peepholes then the GCC does better than e.g. CV.  For example, CV's code generation model uses R30/R31/Z first for nearly all operations:

ATmega1280 register use summary:
r0 : 612 r1 :  88 r2 : 163 r3 : 162 r4 : 159 r5 :  82 r6 : 123 r7 :  60 
r8 :  41 r9 :  35 r10:  13 r11:  12 r12: 272 r13:  13 r14: 118 r15:  16 
r16: 759 r17:1198 r18: 364 r19: 450 r20: 301 r21: 165 r22: 778 r23: 688 
r24: 553 r25: 348 r26:6261 r27:2801 r28: 288 r29:   1 r30:11881 r31:5063 
x  :1203 y  :5247 z  :1020 
Registers used: 35 out of 35 (100.0%)

This often "gets in the way" when Z might be better used to maintain an index.

 

 

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

joeymorin wrote:

 It makes sense to you, not to anyone else.

It makes sense to anyone who has spent more than a week on these fora.  Folks which, I suppose, you'd call 'old-timers'.

 

You're on a roll, Flintrock.  Eight posts so far and fully 75% of them have been combative, sardonic, and peppered with thinly veiled disrespect, with the remainder seemingly intended to convey to the rest of us how smart you are.

I get 1-3/7 .

In at least one case, Flintrock was provoked: Lee used the word "infinite" in a post that seemed rather a lot like criticism.

"You made a really bad choice, now live with it" would seem a sensible and likely translation.

I'll look forward to your contributions in the future.

Really?

Moderation in all things. -- ancient proverb

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

Not trying to start fights.  I understood that the post was something that likely made sense to insiders and that likely wouldn't be expected to make sense to the person it was directed at (hmmm).  It's fine.  I know he didn't mean any harm.  Thanks for the clarification about what is meant by infinite value.  I just didn't know and wasn't smart enough to figure it out.

 

Anyway, I'm not posting back about that, but a mostly useless but possibly interesting observation.  All the docs say not to use local C variables in a naked function because there is no stack frame.  Make sense, but the question is, what happens?  I mean is it going to use the  frame pointer from the interrupted function and relative locations that it would have setup if it had set them up and just trash whatever is there?  That's hard to imagine. Is it going to use the "heap"?  Is it going go all gcc like and conclude that since it's impossible the world no longer exists?  It seems like the world won't end and has to do something.

 

So I tested it.  In my test it forced all local variables to register variables (of course without clobber protection).  That's pretty fine and actually saved 25 bytes of space compared to an OS_task implementation (OS_task doesn't protect clobbers but does setup a local frame).  But what if you have too many locals?  So I made a volatile array of 30bytes, summed it and sent the result to PORTB (just to make sure it didn't try to optimize it all out).  OS_task put it on the stack frame.  naked... broke the compiler with some error about store_field() that doesn't even show up on google.   That's relatively safe actually (other than completely confusing some future maintainer with this error).  Of course there are no guarantees.  Why would anyone want this? Let's just say you wouldn't.  In this interrupt I'm making that resets at the end I don't care about normal clobbers, but I do care about over-writting some random memory in an undefined way that might even end up clobbering the present function. In the end though I went with OS_task and register keywords.   I'm not defending or encouraging use of anything like what I'm doing either (there are almost always better ways), just reporting some interesting observations about it.

 

Last Edited: Sat. Feb 18, 2017 - 12:27 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

But the point is that you cannot guarantee that locals will be optimized to machine registers. Depending on the variables and what they are used for the code generation model of the compiler may create stack frame variables. This is just as true for "normal" functions as naked ones. If you want to see it in action maybe fake it by creating a large local array or similar. The point is you cannot know when this may happen. You can try using "register" but it's only ever a hint not a directive.

 

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

If one really wants, one can use local variables in a naked function.

As I noted earlier, documentation says not to use anything but inline assembly.

If you do otherwise, you pretty much have to read the compiler output every time you recompile.

Otherwise you will not know whether the compiler functioned as expected.

 

Documentation goes even further: it says only use basic asm (no inputs, outputs or clobbers).

To me, that seems overkill.  Definitely omit outputs and inputs that tell the compiler to pick a general purpose register.

Other inputs should be ok.

Moderation in all things. -- ancient proverb