Patchwork [AVR] : FIX PR43746 (merge progmem strings)

login
register
mail settings
Submitter Georg-Johann Lay
Date June 22, 2011, 4:26 p.m.
Message ID <4E0217CD.6040802@gjlay.de>
Download mbox | patch
Permalink /patch/101505/
State New
Headers show

Comments

Georg-Johann Lay - June 22, 2011, 4:26 p.m.
This patch will put progmem strings into a mergeable strings section.

progmem_section is globally renamed to progmem_sw_table_section as
it's only used for switch_case jumptables.

The code that attached the explicit section name ".progmem" in
avr_insert_attributes is removed.

Instead, avr_init_sections sets up two new sections progmem_section
and progmem_string_section which are selected in new hook
avr_asm_select_section if appropriate, i.e. if progmem attribute is
specified.

The classic implementation of progmem always chose .progmem section
regardless of -fdata-sections.  The new code behaves similar: If
attribute "progmem" is on, avr_asm_named_section will put data in
.progmem resp. .progmem.string.

Note that .progmem.string coincides with .progmem if -fmerge-constants
if off, e.g. if optimization is off.

As Eric recently reopened PR43746, there is obviously need for such
tweak to save flash.

Tested without regressions and lightly tested with own code.

Johann


	PR target/43746
	* config/avr/avr.h (progmem_section): Rename progmem_section to
	progmem_sw_table_section.
	(ASM_OUTPUT_CASE_LABEL): Ditto.
	* config/avr/avr.c (avr_output_addr_vec_elt): Ditto.
	(avr_output_bld): Ditto.
	(progmem_string_section): New variable.
	(progmem_sw_table_section): New variable.
	(TARGET_ASM_SELECT_SECTION): Define to...
	(avr_asm_select_section): ...this new function.
	(avr_progmem_string_p): New function.
	(avr_insert_attributes): Don't attach explicit section names to
	'progmem' variables.
	(avr_output_progmem_section_asm_op): Remove.
	(avr_asm_init_sections): Setup progmem_section,
	progmem_string_section, progmem_sw_table_section.
	(avr_asm_named_section): Put 'progmem' data into .progmem resp.
	.progmem.string section.
	(avr_section_type_flags): Setup flags for these sections.

Patch

Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 175201)
+++ config/avr/avr.c	(working copy)
@@ -83,6 +83,7 @@  static bool avr_function_value_regno_p (
 static void avr_insert_attributes (tree, tree *);
 static void avr_asm_init_sections (void);
 static unsigned int avr_section_type_flags (tree, const char *, int);
+static section * avr_asm_select_section (tree, int, unsigned HOST_WIDE_INT);
 
 static void avr_reorg (void);
 static void avr_asm_out_ctor (rtx, int);
@@ -131,7 +132,9 @@  const struct base_arch_s *avr_current_ar
 /* Current device.  */
 const struct mcu_type_s *avr_current_device;
 
-section *progmem_section;
+static GTY(()) section *progmem_section;
+static GTY(()) section *progmem_string_section;
+section *progmem_sw_table_section;
 
 /* To track if code will use .bss and/or .data.  */
 bool avr_need_clear_bss_p = false;
@@ -195,6 +198,8 @@  static const struct attribute_spec avr_a
 #define TARGET_INSERT_ATTRIBUTES avr_insert_attributes
 #undef TARGET_SECTION_TYPE_FLAGS
 #define TARGET_SECTION_TYPE_FLAGS avr_section_type_flags
+#undef TARGET_ASM_SELECT_SECTION
+#define TARGET_ASM_SELECT_SECTION avr_asm_select_section
 
 /* `TARGET_ASM_NAMED_SECTION' must be defined in avr.h.  */
 
@@ -5170,42 +5175,63 @@  avr_progmem_p (tree decl, tree attribute
   return 0;
 }
 
-/* Add the section attribute if the variable is in progmem.  */
+
+/* Return true if DECL is a string to be put into
+   .progmem.string section, false otherwise.  */
+
+static bool
+avr_progmem_string_p (tree decl)
+{
+  if (decl && DECL_P (decl)
+      && avr_progmem_p (decl, DECL_ATTRIBUTES (decl)))
+    {
+      tree array = TREE_TYPE (decl);
+      
+      return (ARRAY_TYPE == TREE_CODE (array)
+              && TYPE_STRING_FLAG (TREE_TYPE (array))
+              && 1 == tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (array)), 1));
+    }
+
+  return false;
+}
+
+
+/* Implement `TARGET_INSERT_ATTRIBUTES'.  */
 
 static void
 avr_insert_attributes (tree node, tree *attributes)
 {
   if (TREE_CODE (node) == VAR_DECL
       && (TREE_STATIC (node) || DECL_EXTERNAL (node))
-      && avr_progmem_p (node, *attributes))
+      && avr_progmem_p (node, *attributes)
+      && !TREE_READONLY (node))
     {
-      if (TREE_READONLY (node)) 
-        {
-          static const char dsec[] = ".progmem.data";
+      /* Just allow const data to get into .progmem section.  */
 
-          *attributes = tree_cons (get_identifier ("section"),
-                                   build_tree_list (NULL, build_string (strlen (dsec), dsec)),
-                                   *attributes);
-        }
-      else
-        {
-          error ("variable %q+D must be const in order to be put into"
-                 " read-only section by means of %<__attribute__((progmem))%>",
-                 node);
-        }
+      error ("variable %q+D must be const in order to be put into"
+             " read-only section by means of %<__attribute__((progmem))%>",
+             node);
     }
 }
 
-/* A get_unnamed_section callback for switching to progmem_section.  */
 
-static void
-avr_output_progmem_section_asm_op (const void *arg ATTRIBUTE_UNUSED)
+/* Implement `TARGET_ASM_SELECT_SECTION'.  */
+
+static section *
+avr_asm_select_section (tree exp, int reloc, unsigned HOST_WIDE_INT align)
 {
-  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");
+  if (DECL_P (exp)
+      && avr_progmem_p (exp, DECL_ATTRIBUTES (exp)))
+    {
+      /* Put ordinary (8-bit) strings into section .progmem.string
+         so that they can be merged.  */
+      
+      return (avr_progmem_string_p (exp)
+              ? progmem_string_section
+              : progmem_section);
+    }
+ 
+  return default_elf_select_section (exp, reloc, align);
 }
 
 
@@ -5266,9 +5292,32 @@  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);
+  /* For switch/case jumptables.  */
+  
+  progmem_sw_table_section = AVR_HAVE_JMP_CALL
+    ? get_unnamed_section (0, output_section_asm_op,
+                           "\t.section .progmem.gcc_sw_table"
+                           ",\"a\",@progbits\n\t.p2align 1\n")
+    : get_unnamed_section (SECTION_CODE, output_section_asm_op,
+                           "\t.section .progmem.gcc_sw_table"
+                           ",\"ax\",@progbits\n\t.p2align 1\n");
+
+  /* For data attributed 'progmem'.  */
+  
+  progmem_section
+    = get_unnamed_section (0, output_section_asm_op,
+                           "\t.section .progmem,\"a\",@progbits");
+
+  /* For char strings attributed 'progmem', so we can merge
+     strings will get merged if -fmerge-constants is on.  */
+  
+  progmem_string_section = flag_merge_constants
+    ? get_unnamed_section (SECTION_STRINGS | SECTION_MERGE | 1,
+                           output_section_asm_op,
+                           "\t.section .progmem.string"
+                           ",\"aMS\",@progbits,1")
+    : progmem_section;
+
   readonly_data_section = data_section;
 
   data_section->unnamed.callback = avr_output_data_section_asm_op;
@@ -5289,7 +5338,24 @@  avr_asm_named_section (const char *name,
   
   if (!avr_need_clear_bss_p)
     avr_need_clear_bss_p = (0 == strncmp (name, ".bss", 4));
-  
+
+  if (decl && DECL_P (decl))
+    {
+      /* Attribute 'progmem' is stronger than -fdata-sections.  */
+      
+      if (avr_progmem_string_p (decl))
+        {
+          switch_to_section (progmem_string_section);
+          return;
+        }
+      
+      if (avr_progmem_p (decl, DECL_ATTRIBUTES (decl)))
+        {
+          switch_to_section (progmem_section);
+          return;
+        }
+    }
+    
   default_elf_asm_named_section (name, flags, decl);
 }
 
@@ -5308,6 +5374,15 @@  avr_section_type_flags (tree decl, const
 		 ".noinit section");
     }
 
+  if (0 == strncmp (name, ".progmem", strlen (".progmem")))
+    flags &= ~SECTION_WRITE;
+
+  if (0 == strncmp (name, ".progmem.string", strlen (".progmem.string"))
+      && flag_merge_constants >= 1)
+    {
+      flags |= SECTION_MERGE | SECTION_STRINGS | 1;
+    }
+
   return flags;
 }
 
@@ -6393,7 +6468,7 @@  avr_output_bld (rtx operands[], int bit_
 void
 avr_output_addr_vec_elt (FILE *stream, int value)
 {
-  switch_to_section (progmem_section);
+  switch_to_section (progmem_sw_table_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 175201)
+++ config/avr/avr.h	(working copy)
@@ -108,7 +108,7 @@  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;
+extern GTY(()) section *progmem_sw_table_section;
 #endif
 
 #define AVR_HAVE_JMP_CALL (avr_current_arch->have_jmp_call && !TARGET_SHORT_CALLS)
@@ -625,7 +625,7 @@  sprintf (STRING, "*.%s%lu", PREFIX, (uns
   avr_output_addr_vec_elt(STREAM, VALUE)
 
 #define ASM_OUTPUT_CASE_LABEL(STREAM, PREFIX, NUM, TABLE) \
-  (switch_to_section (progmem_section), \
+  (switch_to_section (progmem_sw_table_section), \
    (*targetm.asm_out.internal_label) (STREAM, PREFIX, NUM))
 
 #define ASM_OUTPUT_SKIP(STREAM, N)		\