diff mbox series

[avr] Implement PR83737

Message ID ea5ce349-0189-b7e3-0355-ed50b21d5cbb@gjlay.de
State New
Headers show
Series [avr] Implement PR83737 | expand

Commit Message

Georg-Johann Lay Jan. 8, 2018, 4:19 p.m. UTC
This PR skips saving of any registers in main.

Attribute OS_main can do this as well, however we can just drop
any saves / restores in all optimized compilation -- not even
the test suite needs these saves.

The feature can still be switched off by new -mno-OS_main

Ok for trunk?


gcc/
	Don't save registers in main().

	PR target/83737
	* doc/invoke.texi (AVR Options) [-mOS_main]: Document it.
	* config/avr/avr.opt (-mOS_main): New target option.
	* config/avr/avr.c (avr_in_main_p): New static function.
	(avr_regs_to_save) [avr_in_main_p]: Return 0.
	(avr_prologue_setup_frame): Don't save any regs if avr_in_main_p.
	(avr_expand_epilogue): Same.
	* common/config/avr/avr-common.c (avr_option_optimization_table):
	Switch on -mOS_main for optimizing compilations.

Comments

Denis Chertykov Jan. 8, 2018, 5:39 p.m. UTC | #1
2018-01-08 20:19 GMT+04:00 Georg-Johann Lay <avr@gjlay.de>:
> This PR skips saving of any registers in main.
>
> Attribute OS_main can do this as well, however we can just drop
> any saves / restores in all optimized compilation -- not even
> the test suite needs these saves.
>
> The feature can still be switched off by new -mno-OS_main
>
> Ok for trunk?

I like it.

Please commit.

>
>
> gcc/
>         Don't save registers in main().
>
>         PR target/83737
>         * doc/invoke.texi (AVR Options) [-mOS_main]: Document it.
>         * config/avr/avr.opt (-mOS_main): New target option.
>         * config/avr/avr.c (avr_in_main_p): New static function.
>         (avr_regs_to_save) [avr_in_main_p]: Return 0.
>         (avr_prologue_setup_frame): Don't save any regs if avr_in_main_p.
>         (avr_expand_epilogue): Same.
>         * common/config/avr/avr-common.c (avr_option_optimization_table):
>         Switch on -mOS_main for optimizing compilations.
Georg-Johann Lay Jan. 9, 2018, 11 a.m. UTC | #2
On 08.01.2018 18:39, Denis Chertykov wrote:
> 2018-01-08 20:19 GMT+04:00 Georg-Johann Lay <avr@gjlay.de>:
>> This PR skips saving of any registers in main.
>>
>> Attribute OS_main can do this as well, however we can just drop
>> any saves / restores in all optimized compilation -- not even
>> the test suite needs these saves.
>>
>> The feature can still be switched off by new -mno-OS_main
>>
>> Ok for trunk?
> 
> I like it.
> 
> Please commit.

Committed a different approach:

1) The effect is the same as OS_task, hence named the
    option -mmain-is-OS_task.  OS_main is too strict
    because that'd assume that IRQs are off when main is
    entered, hence due to that rare case (and when main
    needs a frame) OS_task is the right feature.

2) Instead of fiddling with prologue / epilogue directly,
    now just add OS_task to main.

3) Some restrictions have been removed re. diagnostics
    when naked, OS_task, OS_main are specified at the
    same time.  Instead, the logic follows now just
    what the attributes are requesting (e.g. OS_main
    supersedes OS_task and naked supersedes OS_main).

The original subject has a typo: The PR is PR83738.


	PR target/83738
	* doc/invoke.texi (AVR Options) [-mmain-is-OS_task]: Document it.
	* config/avr/avr.opt (-mmain-is-OS_task): New target option.
	* config/avr/avr.c (avr_set_current_function): Don't error if
	naked, OS_task or OS_main are specified at the same time.
	(avr_function_ok_for_sibcall): Don't disable sibcalls for OS_task,
	OS_main.
	(avr_insert_attributes) [-mmain-is-OS_task] <main>: Add OS_task
	attribute.
	* common/config/avr/avr-common.c (avr_option_optimization_table):
	Switch on -mmain-is-OS_task for optimizing compilations.
Index: common/config/avr/avr-common.c
===================================================================
--- common/config/avr/avr-common.c	(revision 256338)
+++ common/config/avr/avr-common.c	(working copy)
@@ -31,6 +31,7 @@ static const struct default_options avr_
     // a frame without need when it tries to be smart around calls.
     { OPT_LEVELS_ALL, OPT_fcaller_saves, NULL, 0 },
     { OPT_LEVELS_1_PLUS_NOT_DEBUG, OPT_mgas_isr_prologues, NULL, 1 },
+    { OPT_LEVELS_1_PLUS, OPT_mmain_is_OS_task, NULL, 1 },
     { OPT_LEVELS_NONE, 0, NULL, 0 }
   };
 
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 256338)
+++ config/avr/avr.c	(working copy)
@@ -1030,9 +1030,6 @@ avr_no_gccisr_function_p (tree func)
 static void
 avr_set_current_function (tree decl)
 {
-  location_t loc;
-  const char *isr;
-
   if (decl == NULL_TREE
       || current_function_decl == NULL_TREE
       || current_function_decl == error_mark_node
@@ -1040,7 +1037,7 @@ avr_set_current_function (tree decl)
       || cfun->machine->attributes_checked_p)
     return;
 
-  loc = DECL_SOURCE_LOCATION (decl);
+  location_t loc = DECL_SOURCE_LOCATION (decl);
 
   cfun->machine->is_naked = avr_naked_function_p (decl);
   cfun->machine->is_signal = avr_signal_function_p (decl);
@@ -1049,21 +1046,19 @@ avr_set_current_function (tree decl)
   cfun->machine->is_OS_main = avr_OS_main_function_p (decl);
   cfun->machine->is_no_gccisr = avr_no_gccisr_function_p (decl);
 
-  isr = cfun->machine->is_interrupt ? "interrupt" : "signal";
+  const char *isr = cfun->machine->is_interrupt ? "interrupt" : "signal";
 
   /* Too much attributes make no sense as they request conflicting features. */
 
-  if (cfun->machine->is_OS_task + cfun->machine->is_OS_main
-      + (cfun->machine->is_signal || cfun->machine->is_interrupt) > 1)
-    error_at (loc, "function attributes %qs, %qs and %qs are mutually"
-              " exclusive", "OS_task", "OS_main", isr);
-
-  /* 'naked' will hide effects of 'OS_task' and 'OS_main'.  */
-
-  if (cfun->machine->is_naked
-      && (cfun->machine->is_OS_task || cfun->machine->is_OS_main))
-    warning_at (loc, OPT_Wattributes, "function attributes %qs and %qs have"
-                " no effect on %qs function", "OS_task", "OS_main", "naked");
+  if (cfun->machine->is_OS_task
+      && (cfun->machine->is_signal || cfun->machine->is_interrupt))
+    error_at (loc, "function attributes %qs and %qs are mutually exclusive",
+              "OS_task", isr);
+
+  if (cfun->machine->is_OS_main
+      && (cfun->machine->is_signal || cfun->machine->is_interrupt))
+    error_at (loc, "function attributes %qs and %qs are mutually exclusive",
+              "OS_main", isr);
 
   if (cfun->machine->is_interrupt || cfun->machine->is_signal)
     {
@@ -3526,12 +3521,7 @@ avr_function_ok_for_sibcall (tree decl_c
   if (cfun->machine->is_interrupt
       || cfun->machine->is_signal
       || cfun->machine->is_naked
-      || avr_naked_function_p (decl_callee)
-      /* FIXME: For OS_task and OS_main, this might be over-conservative.  */
-      || (avr_OS_task_function_p (decl_callee)
-          != cfun->machine->is_OS_task)
-      || (avr_OS_main_function_p (decl_callee)
-          != cfun->machine->is_OS_main))
+      || avr_naked_function_p (decl_callee))
     {
       return false;
     }
@@ -10101,13 +10091,29 @@ avr_pgm_check_var_decl (tree node)
 }
 
 
-/* Add the section attribute if the variable is in progmem.  */
+/* Implement `TARGET_INSERT_ATTRIBUTES'.  */
 
 static void
 avr_insert_attributes (tree node, tree *attributes)
 {
   avr_pgm_check_var_decl (node);
 
+  if (TARGET_MAIN_IS_OS_TASK
+      && TREE_CODE (node) == FUNCTION_DECL
+      && MAIN_NAME_P (DECL_NAME (node))
+      // FIXME:  We'd like to also test `flag_hosted' which is only
+      // available in the C-ish fronts, hence no such test for now.
+      // Instead, we test the return type of "main" which is not exactly
+      // the same but good enough.
+      && INTEGRAL_TYPE_P (TREE_TYPE (TREE_TYPE (node)))
+      && NULL == lookup_attribute ("OS_task", *attributes))
+    {
+      *attributes = tree_cons (get_identifier ("OS_task"),
+                               NULL, *attributes);
+    }
+
+  /* Add the section attribute if the variable is in progmem.  */
+
   if (TREE_CODE (node) == VAR_DECL
       && (TREE_STATIC (node) || DECL_EXTERNAL (node))
       && avr_progmem_p (node, *attributes))
Index: config/avr/avr.opt
===================================================================
--- config/avr/avr.opt	(revision 256338)
+++ config/avr/avr.opt	(working copy)
@@ -64,6 +64,10 @@ mbranch-cost=
 Target Report Joined RejectNegative UInteger Var(avr_branch_cost) Init(0)
 Set the branch costs for conditional branch instructions.  Reasonable values are small, non-negative integers.  The default branch cost is 0.
 
+mmain-is-OS_task
+Target Report Mask(MAIN_IS_OS_TASK)
+Treat main as if it had attribute OS_task.
+
 morder1
 Target Report Undocumented Mask(ORDER_1)
 
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 256338)
+++ doc/invoke.texi	(working copy)
@@ -669,7 +669,8 @@ -imacros @var{file}  -imultilib @var{dir
 -mbranch-cost=@var{cost} @gol
 -mcall-prologues  -mgas-isr-prologues  -mint8 @gol
 -mn_flash=@var{size}  -mno-interrupts @gol
--mrelax  -mrmw  -mstrict-X  -mtiny-stack  -mfract-convert-truncate @gol
+-mmain-is-OS_task -mrelax  -mrmw  -mstrict-X  -mtiny-stack @gol
+-mfract-convert-truncate @gol
 -mshort-calls  -nodevicelib @gol
 -Waddr-space-convert  -Wmisspelled-isr}
 
@@ -16462,6 +16463,12 @@ and @code{long long} is 4 bytes.  Please
 conform to the C standards, but it results in smaller code
 size.
 
+@item -mmain-is-OS_task
+@opindex mmain-is-OS_task
+Do not save registers in @code{main}.  The effect is similar to
+attaching attribute @ref{AVR Function Attributes,,@code{OS_task}}
+to @code{main}. It is activated per default if optimization is on.
+
 @item -mn-flash=@var{num}
 @opindex mn-flash
 Assume that the flash memory has a size of
diff mbox series

Patch

Index: common/config/avr/avr-common.c
===================================================================
--- common/config/avr/avr-common.c	(revision 256338)
+++ common/config/avr/avr-common.c	(working copy)
@@ -31,6 +31,7 @@  static const struct default_options avr_
     // a frame without need when it tries to be smart around calls.
     { OPT_LEVELS_ALL, OPT_fcaller_saves, NULL, 0 },
     { OPT_LEVELS_1_PLUS_NOT_DEBUG, OPT_mgas_isr_prologues, NULL, 1 },
+    { OPT_LEVELS_1_PLUS, OPT_mOS_main, NULL, 1 },
     { OPT_LEVELS_NONE, 0, NULL, 0 }
   };
 
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 256338)
+++ config/avr/avr.c	(working copy)
@@ -1167,6 +1167,23 @@  avr_starting_frame_offset (void)
 }
 
 
+/* Return true if we are supposed to be in main().  This is only used
+   to determine if the callee-saved registers don't need to be saved
+   because the caller of main (crt*.o) doesn't use any of them.  */
+
+static bool
+avr_in_main_p ()
+{
+  return (TARGET_OS_MAIN
+          && MAIN_NAME_P (DECL_NAME (current_function_decl))
+          // FIXME:  We'd like to also test `flag_hosted' which is only
+          // available in the C-ish fronts, so no such test for now.
+          // Instead, we test the return type of "main" which is not exactly
+          // the same but good enough.
+          && INTEGRAL_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl))));
+}
+
+
 /* Return the number of hard registers to push/pop in the prologue/epilogue
    of the current function, and optionally store these registers in SET.  */
 
@@ -1180,10 +1197,11 @@  avr_regs_to_save (HARD_REG_SET *set)
     CLEAR_HARD_REG_SET (*set);
   count = 0;
 
-  /* No need to save any registers if the function never returns or
-     has the "OS_task" or "OS_main" attribute.  */
+  /* No need to save any registers if the function never returns or has the
+     "OS_task" or "OS_main" attribute.  Dito if we are in "main".  */
 
   if (TREE_THIS_VOLATILE (current_function_decl)
+      || avr_in_main_p()
       || cfun->machine->is_OS_task
       || cfun->machine->is_OS_main)
     return 0;
@@ -1651,6 +1669,7 @@  avr_prologue_setup_frame (HOST_WIDE_INT
                    && size < size_max
                    && live_seq
                    && !isr_p
+                   && !avr_in_main_p()
                    && !cfun->machine->is_OS_task
                    && !cfun->machine->is_OS_main
                    && !AVR_TINY);
@@ -1713,7 +1732,9 @@  avr_prologue_setup_frame (HOST_WIDE_INT
           emit_push_byte (reg, true);
 
       if (frame_pointer_needed
-          && (!(cfun->machine->is_OS_task || cfun->machine->is_OS_main)))
+          && (!(avr_in_main_p()
+                || cfun->machine->is_OS_task
+                || cfun->machine->is_OS_main)))
         {
           /* Push frame pointer.  Always be consistent about the
              ordering of pushes -- epilogue_restores expects the
@@ -1834,6 +1855,9 @@  avr_prologue_setup_frame (HOST_WIDE_INT
           if (cfun->machine->is_interrupt)
             irq_state = 1;
 
+          /* IRQs might be on when entering "main", hence avr_in_main_p
+             is *not* included in the following test.  */
+
           if (TARGET_NO_INTERRUPTS
               || cfun->machine->is_signal
               || cfun->machine->is_OS_main)
@@ -2122,6 +2146,7 @@  avr_expand_epilogue (bool sibcall_p)
               && !isr_p
               && !cfun->machine->is_OS_task
               && !cfun->machine->is_OS_main
+              && !avr_in_main_p()
               && !AVR_TINY);
 
   if (minimize
@@ -2227,7 +2252,9 @@  avr_expand_epilogue (bool sibcall_p)
     } /* size != 0 */
 
   if (frame_pointer_needed
-      && !(cfun->machine->is_OS_task || cfun->machine->is_OS_main))
+      && !(avr_in_main_p()
+           || cfun->machine->is_OS_task
+           || cfun->machine->is_OS_main))
     {
       /* Restore previous frame_pointer.  See avr_expand_prologue for
          rationale for not using pophi.  */
Index: config/avr/avr.opt
===================================================================
--- config/avr/avr.opt	(revision 256338)
+++ config/avr/avr.opt	(working copy)
@@ -70,6 +70,10 @@  Target Report Undocumented Mask(ORDER_1)
 morder2
 Target Report Undocumented Mask(ORDER_2)
 
+mOS_main
+Target Report Mask(OS_MAIN)
+Treat main as if it had attribute OS_main.
+
 mtiny-stack
 Target Report Mask(TINY_STACK)
 Change only the low 8 bits of the stack pointer.
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 256338)
+++ doc/invoke.texi	(working copy)
@@ -669,7 +669,8 @@  -imacros @var{file}  -imultilib @var{dir
 -mbranch-cost=@var{cost} @gol
 -mcall-prologues  -mgas-isr-prologues  -mint8 @gol
 -mn_flash=@var{size}  -mno-interrupts @gol
--mrelax  -mrmw  -mstrict-X  -mtiny-stack  -mfract-convert-truncate @gol
+-mOS_main -mrelax  -mrmw  -mstrict-X  -mtiny-stack @gol
+-mfract-convert-truncate @gol
 -mshort-calls  -nodevicelib @gol
 -Waddr-space-convert  -Wmisspelled-isr}
 
@@ -16472,6 +16473,12 @@  Assume that the flash memory has a size
 Generated code is not compatible with hardware interrupts.
 Code size is smaller.
 
+@item -mOS_main
+@opindex mOS_main
+Do not save registers in @code{main}.  The effect is similar to
+attaching attribute @ref{AVR Function Attributes,,@code{OS_main}}
+to @code{main}. It is activated per default if optimization is on.
+
 @item -mrelax
 @opindex mrelax
 Try to replace @code{CALL} resp.@: @code{JMP} instruction by the shorter