* [Patch,AVR]: Fix PR 43746
@ 2011-09-08 18:34 Georg-Johann Lay
2011-09-12 9:37 ` Denis Chertykov
0 siblings, 1 reply; 2+ messages in thread
From: Georg-Johann Lay @ 2011-09-08 18:34 UTC (permalink / raw)
To: gcc-patches; +Cc: Anatoly Sokolov, Denis Chertykov, Eric Weddington
[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]
This patch adds support for named progmem sections.
The problem with the current implementation is that all progmem stuff is put
into .progmem.data and thus no -gc-sections will have effect or constant
merging cannot merge constants/strings in progmem.
This patch avoids the hard coded .progmem.data section attribute in
avr_insert_attributes. Instead, it uses avr_asm_named_section and
avr_asm_select_section to choose the right section resp. to add the correct
section prefix for progmem data.
Tested without regressions.
Initially I intended to add a -fprogmem-sections command line option that works
similar but independent to -fdata-section. That way data sections could be
used and strings in progmem merged. However, I did not find a straight forward
way without cluttering avr BE with lots of code from varasm.c. Thus, for now,
there is no -fprogmem-sections, i.e. progmem-sections are in sync with
data-sections.
Ok to install?
gcc/
PR target/43746
* config/avr/avr.c (AVR_SECTION_PROGMEM): New Define.
(progmem_section): New Variable.
(avr_asm_init_sections): Initialize it.
(TARGET_ASM_SELECT_SECTION): Define to...
(avr_asm_select_section): ... this new Function.
(avr_replace_prefix): New Function.
(avr_asm_function_rodata_section): Use it.
(avr_insert_attributes): Don't add section attribute for PROGMEM.
(avr_section_type_flags): Use avr_progmem_p instead of section
name to detect if object is in PROGMEM.
(avr_asm_named_section): Set section name prefix for objects in
PROGMEM.
testsuite/
* testsuite/gcc.target/avr/torture/avr-torture.exp
(AVR_TORTURE_OPTIONS): Add test cases "-O2 -fdata-sections" and
"-O2 -fmerge-all-constants".
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr43746-progmerge.diff --]
[-- Type: text/x-patch; name="pr43746-progmerge.diff", Size: 6973 bytes --]
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c (revision 178528)
+++ config/avr/avr.c (working copy)
@@ -54,6 +54,8 @@
/* Return true if STR starts with PREFIX and false, otherwise. */
#define STR_PREFIX_P(STR,PREFIX) (0 == strncmp (STR, PREFIX, strlen (PREFIX)))
+#define AVR_SECTION_PROGMEM (SECTION_MACH_DEP << 0)
+
static void avr_option_override (void);
static int avr_naked_function_p (tree);
static int interrupt_function_p (tree);
@@ -114,6 +116,7 @@ static bool avr_function_ok_for_sibcall
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);
+static section* avr_asm_select_section (tree, int, unsigned HOST_WIDE_INT);
/* Allocate registers from r25 to r8 for parameters for function calls. */
#define FIRST_CUM_REG 26
@@ -139,6 +142,9 @@ const struct mcu_type_s *avr_current_dev
/* Section to put switch tables in. */
static GTY(()) section *progmem_swtable_section;
+/* Unnamed section associated to __attribute__((progmem)) aka. PROGMEM. */
+static GTY(()) section *progmem_section;
+
/* To track if code will use .bss and/or .data. */
bool avr_need_clear_bss_p = false;
bool avr_need_copy_data_p = false;
@@ -206,6 +212,8 @@ static const struct attribute_spec avr_a
#define TARGET_ASM_INIT_SECTIONS avr_asm_init_sections
#undef TARGET_ENCODE_SECTION_INFO
#define TARGET_ENCODE_SECTION_INFO avr_encode_section_info
+#undef TARGET_ASM_SELECT_SECTION
+#define TARGET_ASM_SELECT_SECTION avr_asm_select_section
#undef TARGET_REGISTER_MOVE_COST
#define TARGET_REGISTER_MOVE_COST avr_register_move_cost
@@ -270,6 +278,31 @@ static const struct attribute_spec avr_a
struct gcc_target targetm = TARGET_INITIALIZER;
\f
+
+/* Custom function to replace string prefix.
+
+ Return a ggc-allocated string with strlen (OLD_PREFIX) characters removed
+ from the start of OLD_STR and then prepended with NEW_PREFIX. */
+
+static inline const char*
+avr_replace_prefix (const char *old_str,
+ const char *old_prefix, const char *new_prefix)
+{
+ char *new_str;
+ size_t len = strlen (old_str) + strlen (new_prefix) - strlen (old_prefix);
+
+ gcc_assert (strlen (old_prefix) <= strlen (old_str));
+
+ /* Unfortunately, ggc_alloc_string returns a const char* and thus cannot be
+ used here. */
+
+ new_str = (char*) ggc_alloc_atomic (1 + len);
+
+ strcat (stpcpy (new_str, new_prefix), old_str + strlen (old_prefix));
+
+ return (const char*) new_str;
+}
+
static void
avr_option_override (void)
{
@@ -5034,15 +5067,7 @@ avr_insert_attributes (tree node, tree *
if (error_mark_node == node0)
return;
- if (TYPE_READONLY (node0))
- {
- static const char dsec[] = ".progmem.data";
-
- *attributes = tree_cons (get_identifier ("section"),
- build_tree_list (NULL, build_string (strlen (dsec), dsec)),
- *attributes);
- }
- else
+ if (!TYPE_READONLY (node0))
{
error ("variable %q+D must be const in order to be put into"
" read-only section by means of %<__attribute__((progmem))%>",
@@ -5119,6 +5144,10 @@ avr_asm_init_sections (void)
",\"ax\",@progbits");
}
+ progmem_section
+ = get_unnamed_section (0, output_section_asm_op,
+ "\t.section\t.progmem.data,\"a\",@progbits");
+
/* Override section callbacks to keep track of `avr_need_clear_bss_p'
resp. `avr_need_copy_data_p'. */
@@ -5173,11 +5202,7 @@ avr_asm_function_rodata_section (tree de
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));
+ const char *rname = avr_replace_prefix (name, old_prefix, new_prefix);
flags &= ~SECTION_CODE;
flags |= AVR_HAVE_JMP_CALL ? 0 : SECTION_CODE;
@@ -5197,6 +5222,22 @@ avr_asm_function_rodata_section (tree de
static void
avr_asm_named_section (const char *name, unsigned int flags, tree decl)
{
+ if (flags & AVR_SECTION_PROGMEM)
+ {
+ const char *old_prefix = ".rodata";
+ const char *new_prefix = ".progmem.data";
+ const char *sname = new_prefix;
+
+ if (STR_PREFIX_P (name, old_prefix))
+ {
+ sname = avr_replace_prefix (name, old_prefix, new_prefix);
+ }
+
+ default_elf_asm_named_section (sname, flags, decl);
+
+ return;
+ }
+
if (!avr_need_copy_data_p)
avr_need_copy_data_p = (STR_PREFIX_P (name, ".data")
|| STR_PREFIX_P (name, ".rodata")
@@ -5223,8 +5264,12 @@ avr_section_type_flags (tree decl, const
".noinit section");
}
- if (STR_PREFIX_P (name, ".progmem.data"))
- flags &= ~SECTION_WRITE;
+ if (decl && DECL_P (decl)
+ && avr_progmem_p (decl, DECL_ATTRIBUTES (decl)))
+ {
+ flags &= ~SECTION_WRITE;
+ flags |= AVR_SECTION_PROGMEM;
+ }
return flags;
}
@@ -5254,6 +5299,36 @@ avr_encode_section_info (tree decl, rtx
}
+/* Implement `TARGET_ASM_SELECT_SECTION' */
+
+static section *
+avr_asm_select_section (tree decl, int reloc, unsigned HOST_WIDE_INT align)
+{
+ section * sect = default_elf_select_section (decl, reloc, align);
+
+ if (decl && DECL_P (decl)
+ && avr_progmem_p (decl, DECL_ATTRIBUTES (decl)))
+ {
+ if (sect->common.flags & SECTION_NAMED)
+ {
+ const char * name = sect->named.name;
+ const char * old_prefix = ".rodata";
+ const char * new_prefix = ".progmem.data";
+
+ if (STR_PREFIX_P (name, old_prefix))
+ {
+ const char *sname = avr_replace_prefix (name, old_prefix, new_prefix);
+
+ return get_section (sname, sect->common.flags, sect->named.decl);
+ }
+ }
+
+ return progmem_section;
+ }
+
+ return sect;
+}
+
/* Implement `TARGET_ASM_FILE_START'. */
/* Outputs some appropriate text to go at the start of an assembler
file. */
Index: testsuite/gcc.target/avr/torture/avr-torture.exp
===================================================================
--- testsuite/gcc.target/avr/torture/avr-torture.exp (revision 178527)
+++ testsuite/gcc.target/avr/torture/avr-torture.exp (working copy)
@@ -39,6 +39,8 @@ dg-init
{ -O1 } \
{ -O2 } \
{ -O2 -mcall-prologues } \
+ { -O2 -fdata-sections } \
+ { -O2 -fmerge-all-constants } \
{ -Os -fomit-frame-pointer } \
{ -Os -fomit-frame-pointer -finline-functions } \
{ -O3 -g } \
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Patch,AVR]: Fix PR 43746
2011-09-08 18:34 [Patch,AVR]: Fix PR 43746 Georg-Johann Lay
@ 2011-09-12 9:37 ` Denis Chertykov
0 siblings, 0 replies; 2+ messages in thread
From: Denis Chertykov @ 2011-09-12 9:37 UTC (permalink / raw)
To: Georg-Johann Lay; +Cc: gcc-patches, Anatoly Sokolov, Eric Weddington
2011/9/8 Georg-Johann Lay <avr@gjlay.de>:
> This patch adds support for named progmem sections.
>
> The problem with the current implementation is that all progmem stuff is put
> into .progmem.data and thus no -gc-sections will have effect or constant
> merging cannot merge constants/strings in progmem.
>
> This patch avoids the hard coded .progmem.data section attribute in
> avr_insert_attributes. Instead, it uses avr_asm_named_section and
> avr_asm_select_section to choose the right section resp. to add the correct
> section prefix for progmem data.
>
> Tested without regressions.
>
> Initially I intended to add a -fprogmem-sections command line option that works
> similar but independent to -fdata-section. That way data sections could be
> used and strings in progmem merged. However, I did not find a straight forward
> way without cluttering avr BE with lots of code from varasm.c. Thus, for now,
> there is no -fprogmem-sections, i.e. progmem-sections are in sync with
> data-sections.
>
> Ok to install?
Please, commit.
Denis.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-09-12 6:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-08 18:34 [Patch,AVR]: Fix PR 43746 Georg-Johann Lay
2011-09-12 9:37 ` Denis Chertykov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).