diff mbox

[AVR] : Cleanup progmem_section handling.

Message ID 4E57B702.5060500@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay Aug. 26, 2011, 3:08 p.m. UTC
Georg-Johann Lay wrote:
> progmem_section is a section to put jump tables in.
> 
> This patch puts jump tables in individual sections if
> -ffunction-section is on and does some more cleanup around
> that, i.e. implement TARGET_ASM_FUNCTION_RODATA_SECTION hook.
> 
> progmem_section is renamed to progmem_swtable_section and is
> local to avr.c now.
> 
> What I don't understand is the old restriction that ASM_OUTPUT_ALIGN
> only printed .p2align for powers > 1; I changed that to >= 1 so that
> jump tables get aligned properly.
> 
> Passed without regressions.

This is an update of the patch.  The differences to the original patch
   http://gcc.gnu.org/ml/gcc-patches/2011-08/msg02090.html
are:

* avr_asm_function_rodata_section now is based on
  varasm.c:default_function_rodata_section and thus more general and
  consistent.  Basically, it replaces .rodata prefix with
  .progmem.gcc_sw_table prefix etc.

* ASM_OUTPUT_BEFORE_CASE_LABEL don't use ASM_OUTPUT_ALIGN to output the
  alignment, and ASM_OUTPUT_ALIGN is unchanged.

Passed without regressions.

Actually, it's bit more than a cleanup now with the new
TARGET_ASM_FUNCTION_RODATA_SECTION...

Ok to commit?

Johann

	* config/avr/avr.h (progmem_section): Remove Declaration.
	* config/avr/avr.c (progmem_section): Make static and rename to
	progmem_swtable_section.
	(avr_output_addr_vec_elt): No need to switch sections.
	(avr_asm_init_sections): Use output_section_asm_op as section
	callback for progmem_swtable_section.
	(avr_output_progmem_section_asm_op): Remove Function.
	(TARGET_ASM_FUNCTION_RODATA_SECTION): New Define.
	(avr_asm_function_rodata_section): New static Function.
	* config/avr/elf.h (ASM_OUTPUT_BEFORE_CASE_LABEL): Define to
	output alignment 2**1 for jump tables.

Comments

Denis Chertykov Aug. 29, 2011, 3 p.m. UTC | #1
2011/8/26 Georg-Johann Lay <avr@gjlay.de>:
> Georg-Johann Lay wrote:
>> progmem_section is a section to put jump tables in.
>>
>> This patch puts jump tables in individual sections if
>> -ffunction-section is on and does some more cleanup around
>> that, i.e. implement TARGET_ASM_FUNCTION_RODATA_SECTION hook.
>>
>> progmem_section is renamed to progmem_swtable_section and is
>> local to avr.c now.
>>
>> What I don't understand is the old restriction that ASM_OUTPUT_ALIGN
>> only printed .p2align for powers > 1; I changed that to >= 1 so that
>> jump tables get aligned properly.
>>
>> Passed without regressions.
>
> This is an update of the patch.  The differences to the original patch
>   http://gcc.gnu.org/ml/gcc-patches/2011-08/msg02090.html
> are:
>
> * avr_asm_function_rodata_section now is based on
>  varasm.c:default_function_rodata_section and thus more general and
>  consistent.  Basically, it replaces .rodata prefix with
>  .progmem.gcc_sw_table prefix etc.
>
> * ASM_OUTPUT_BEFORE_CASE_LABEL don't use ASM_OUTPUT_ALIGN to output the
>  alignment, and ASM_OUTPUT_ALIGN is unchanged.
>
> Passed without regressions.
>
> Actually, it's bit more than a cleanup now with the new
> TARGET_ASM_FUNCTION_RODATA_SECTION...
>
> Ok to commit?
>

Ok.

Denis.
diff mbox

Patch

Index: config/avr/elf.h
===================================================================
--- config/avr/elf.h	(revision 178099)
+++ config/avr/elf.h	(working copy)
@@ -37,9 +37,10 @@ 
 #define ASM_DECLARE_FUNCTION_NAME(FILE, NAME, DECL)     \
   avr_asm_declare_function_name ((FILE), (NAME), (DECL))
 
+/* Output alignment 2**1 for jump tables.  */
 #undef ASM_OUTPUT_BEFORE_CASE_LABEL
 #define ASM_OUTPUT_BEFORE_CASE_LABEL(FILE, PREFIX, NUM, TABLE) \
-  switch_to_section (progmem_section);
+  fprintf (FILE, "\t.p2align\t1\n");
 
 /* Be conservative in crtstuff.c.  */
 #undef INIT_SECTION_ASM_OP
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 178099)
+++ config/avr/avr.c	(working copy)
@@ -113,6 +113,7 @@  static void avr_function_arg_advance (cu
 static bool avr_function_ok_for_sibcall (tree, tree);
 static void avr_asm_named_section (const char *name, unsigned int flags, tree decl);
 static void avr_encode_section_info (tree, rtx, int);
+static section* avr_asm_function_rodata_section (tree);
 
 /* Allocate registers from r25 to r8 for parameters for function calls.  */
 #define FIRST_CUM_REG 26
@@ -135,7 +136,8 @@  const struct base_arch_s *avr_current_ar
 /* Current device.  */
 const struct mcu_type_s *avr_current_device;
 
-section *progmem_section;
+/* Section to put switch tables in.  */
+static GTY(()) section *progmem_swtable_section;
 
 /* To track if code will use .bss and/or .data.  */
 bool avr_need_clear_bss_p = false;
@@ -263,6 +265,8 @@  static const struct attribute_spec avr_a
 #undef TARGET_EXPAND_BUILTIN
 #define TARGET_EXPAND_BUILTIN avr_expand_builtin
 
+#undef TARGET_ASM_FUNCTION_RODATA_SECTION
+#define TARGET_ASM_FUNCTION_RODATA_SECTION avr_asm_function_rodata_section
 
 struct gcc_target targetm = TARGET_INITIALIZER;
 
@@ -5036,18 +5040,6 @@  avr_insert_attributes (tree node, tree *
     }
 }
 
-/* A get_unnamed_section callback for switching to progmem_section.  */
-
-static void
-avr_output_progmem_section_asm_op (const void *arg ATTRIBUTE_UNUSED)
-{
-  fprintf (asm_out_file,
-	   "\t.section .progmem.gcc_sw_table, \"%s\", @progbits\n",
-	   AVR_HAVE_JMP_CALL ? "a" : "ax");
-  /* Should already be aligned, this is just to be safe if it isn't.  */
-  fprintf (asm_out_file, "\t.p2align 1\n");
-}
-
 
 /* Implement `ASM_OUTPUT_ALIGNED_DECL_LOCAL'.  */
 /* Implement `ASM_OUTPUT_ALIGNED_DECL_COMMON'.  */
@@ -5098,9 +5090,23 @@  avr_output_bss_section_asm_op (const voi
 static void
 avr_asm_init_sections (void)
 {
-  progmem_section = get_unnamed_section (AVR_HAVE_JMP_CALL ? 0 : SECTION_CODE,
-					 avr_output_progmem_section_asm_op,
-					 NULL);
+  /* Set up a section for jump tables.  Alignment is handled by
+     ASM_OUTPUT_BEFORE_CASE_LABEL.  */
+  
+  if (AVR_HAVE_JMP_CALL)
+    {
+      progmem_swtable_section
+        = get_unnamed_section (0, output_section_asm_op,
+                               "\t.section\t.progmem.gcc_sw_table"
+                               ",\"a\",@progbits");
+    }
+  else
+    {
+      progmem_swtable_section
+        = get_unnamed_section (SECTION_CODE, output_section_asm_op,
+                               "\t.section\t.progmem.gcc_sw_table"
+                               ",\"ax\",@progbits");
+    }
 
   /* Override section callbacks to keep track of `avr_need_clear_bss_p'
      resp. `avr_need_copy_data_p'.  */
@@ -5111,6 +5117,69 @@  avr_asm_init_sections (void)
 }
 
 
+/* Implement `TARGET_ASM_FUNCTION_RODATA_SECTION'.  */
+
+static section*
+avr_asm_function_rodata_section (tree decl)
+{
+  /* If a function is unused and optimized out by -ffunction-sections
+     and --gc-sections, ensure that the same will happen for its jump
+     tables by putting them into individual sections.  */
+
+  unsigned int flags;
+  section * frodata;
+
+  /* Get the frodata section from the default function in varasm.c
+     but treat function-associated data-like jump tables as code
+     rather than as user defined data.  */
+  {
+    int fdata = flag_data_sections;
+
+    flag_data_sections = flag_function_sections;
+    frodata = default_function_rodata_section (decl);
+    flag_data_sections = fdata;
+    flags = frodata->common.flags;
+  }
+
+  if (frodata != readonly_data_section
+      && flags & SECTION_NAMED)
+    {
+      /* Adjust section flags and replace section name prefix.  */
+
+      unsigned int i;
+
+      static const char* const prefix[] =
+        {
+          ".rodata",          ".progmem.gcc_sw_table",
+          ".gnu.linkonce.r.", ".gnu.linkonce.t."
+        };
+
+      for (i = 0; i < sizeof (prefix) / sizeof (*prefix); i += 2)
+        {
+          const char * old_prefix = prefix[i];
+          const char * new_prefix = prefix[i+1];
+          const char * name = frodata->named.name;
+
+          if (STR_PREFIX_P (name, old_prefix))
+            {
+              char *rname = (char*) alloca (1 + strlen (name)
+                                            + strlen (new_prefix)
+                                            - strlen (old_prefix));
+              
+              strcat (stpcpy (rname, new_prefix), name + strlen (old_prefix));
+
+              flags &= ~SECTION_CODE;
+              flags |= AVR_HAVE_JMP_CALL ? 0 : SECTION_CODE;
+              
+              return get_section (rname, flags, frodata->named.decl);
+            }
+        }
+    }
+        
+  return progmem_swtable_section;
+}
+
+
 /* Implement `TARGET_ASM_NAMED_SECTION'.  */
 /* Track need of __do_clear_bss, __do_copy_data for named sections.  */
 
@@ -6693,7 +6762,6 @@  avr_output_bld (rtx operands[], int bit_
 void
 avr_output_addr_vec_elt (FILE *stream, int value)
 {
-  switch_to_section (progmem_section);
   if (AVR_HAVE_JMP_CALL)
     fprintf (stream, "\t.word gs(.L%d)\n", value);
   else
Index: config/avr/avr.h
===================================================================
--- config/avr/avr.h	(revision 178099)
+++ config/avr/avr.h	(working copy)
@@ -127,10 +127,6 @@  extern const struct base_arch_s avr_arch
 
 #define TARGET_CPU_CPP_BUILTINS()	avr_cpu_cpp_builtins (pfile)
 
-#if !defined(IN_LIBGCC2) && !defined(IN_TARGET_LIBS)
-extern GTY(()) section *progmem_section;
-#endif
-
 #define AVR_HAVE_JMP_CALL (avr_current_arch->have_jmp_call && !TARGET_SHORT_CALLS)
 #define AVR_HAVE_MUL (avr_current_arch->have_mul)
 #define AVR_HAVE_MOVW (avr_current_arch->have_movw_lpmx)