View Issue Details

IDProjectCategoryView StatusLast Update
0035899FPCCompilerpublic2019-07-28 21:30
ReporterChristo CrauseAssigned ToJeppe Johansen 
PrioritynormalSeverityminorReproducibilityN/A
Status closedResolutionfixed 
Product Version3.3.1Product Build42516 
Target VersionFixed in Version3.3.1 
Summary0035899: AVR [patch] Enable nostackframe directive for interrupt routines
DescriptionCurrently the compiler ignores the nostackframe directive for interrupt routines. This results in stack frame prologue & epilogue generation, even if not required. Consider the simple example in steps to reproduce below. The code generated for this interrupt is 12 instructions (excluding reti). Since the sbi instruction doesn't modify any status flags nor involve any registers it is a good candidate for the nostackframe directive.

The interrupt below can be rewritten in assembler as follows:
procedure Timer0Overflow; public name 'TIMER0_OVF_ISR'; interrupt; nostackframe; assembler;
asm
  sbi PORTB+(-32), 5
end;

Currently the compiler ignores the nostackframe directive, resulting in 9 extra instructions for stack frame prologue/epilogue.

Attached a patch which enables the nostackframe directive for interrupt routines. The code generated by this patch only contains the sbi instruction followed by reti, a substantial reduction in code size for this simple example.
Steps To Reproduceprocedure Timer0Overflow; public name 'TIMER0_OVF_ISR'; interrupt;
begin
  PORTB := PORTB or (1 shl 5);
end;

Resulting code:
procedure Timer0Overflow; public name 'TIMER0_OVF_ISR'; interrupt;
begin
  9e: 2f 93 push r18
  a0: 1f 92 push r1
  a2: 0f 92 push r0
  a4: 0f b6 in r0, 0x3f ; 63
  a6: 0f 92 push r0
  a8: 11 24 eor r1, r1
  PORTB := PORTB or (1 shl 5);
  aa: 2d 9a sbi 0x05, 5 ; 5
end;
  ac: 0f 90 pop r0
  ae: 0f be out 0x3f, r0 ; 63
  b0: 0f 90 pop r0
  b2: 1f 90 pop r1
  b4: 2f 91 pop r18
  b6: 18 95 reti
TagsNo tags attached.
Fixed in Revision42519
FPCOldBugId
FPCTarget-
Attached Files
  • nostackframe.patch (1,041 bytes)
    Index: compiler/avr/cgcpu.pas
    ===================================================================
    --- compiler/avr/cgcpu.pas	(revision 42516)
    +++ compiler/avr/cgcpu.pas	(working copy)
    @@ -2061,7 +2061,7 @@
           begin
             if current_procinfo.procdef.isempty then
               exit;
    -        if po_interrupt in current_procinfo.procdef.procoptions then
    +        if (po_interrupt in current_procinfo.procdef.procoptions) and not(nostackframe) then
               begin
                 { check if the framepointer is actually used, this is done here because
                   we have to know the size of the locals (must be 0), avr does not know
    @@ -2160,7 +2160,7 @@
               exit;
             if po_interrupt in current_procinfo.procdef.procoptions then
               begin
    -            if not(current_procinfo.procdef.isempty) then
    +            if not(current_procinfo.procdef.isempty) and not(nostackframe) then
                   begin
                     regs:=rg[R_INTREGISTER].used_in_proc;
                     if current_procinfo.framepointer<>NR_NO then
    
    nostackframe.patch (1,041 bytes)

Activities

Christo Crause

2019-07-28 16:10

reporter  

nostackframe.patch (1,041 bytes)
Index: compiler/avr/cgcpu.pas
===================================================================
--- compiler/avr/cgcpu.pas	(revision 42516)
+++ compiler/avr/cgcpu.pas	(working copy)
@@ -2061,7 +2061,7 @@
       begin
         if current_procinfo.procdef.isempty then
           exit;
-        if po_interrupt in current_procinfo.procdef.procoptions then
+        if (po_interrupt in current_procinfo.procdef.procoptions) and not(nostackframe) then
           begin
             { check if the framepointer is actually used, this is done here because
               we have to know the size of the locals (must be 0), avr does not know
@@ -2160,7 +2160,7 @@
           exit;
         if po_interrupt in current_procinfo.procdef.procoptions then
           begin
-            if not(current_procinfo.procdef.isempty) then
+            if not(current_procinfo.procdef.isempty) and not(nostackframe) then
               begin
                 regs:=rg[R_INTREGISTER].used_in_proc;
                 if current_procinfo.framepointer<>NR_NO then
nostackframe.patch (1,041 bytes)

Christo Crause

2019-07-28 21:30

reporter   ~0117466

That was quick! Thanks Jeppe.

Issue History

Date Modified Username Field Change
2019-07-28 16:10 Christo Crause New Issue
2019-07-28 16:10 Christo Crause File Added: nostackframe.patch
2019-07-28 18:46 Jeppe Johansen Assigned To => Jeppe Johansen
2019-07-28 18:46 Jeppe Johansen Status new => resolved
2019-07-28 18:46 Jeppe Johansen Resolution open => fixed
2019-07-28 18:46 Jeppe Johansen Fixed in Version => 3.3.1
2019-07-28 18:46 Jeppe Johansen Fixed in Revision => 42519
2019-07-28 18:46 Jeppe Johansen FPCTarget => -
2019-07-28 21:30 Christo Crause Status resolved => closed
2019-07-28 21:30 Christo Crause Note Added: 0117466