[committed] MSP430: Remove target-specific handling of the "persistent" attribute

Message ID 20201123143822.oanogp722x6c65ok@jozef-acer-manjaro
State New
Headers show
Series
  • [committed] MSP430: Remove target-specific handling of the "persistent" attribute
Related show

Commit Message

Jozef Lawrynowicz Nov. 23, 2020, 2:38 p.m.
The "persistent" attribute is now handled generically, and does not
need specific support in the MSP430 back end.

Successfully built and regtested C/C++ testsuites for msp430-elf.

Committed as obvious.
From 77ee207e17d02e4aec502c6aedd9b0ba36a08de3 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>

Date: Mon, 23 Nov 2020 14:24:43 +0000
Subject: [PATCH] MSP430: Remove target-specific handling of the "persistent"
 attribute

The "persistent" attribute is now handled generically, and does not
need specific support in the MSP430 back end.

gcc/ChangeLog:

	* config/msp430/msp430.c (msp430_section_attr): Don't warn for "lower"
	attribute used with "noinit" or "persistent" attributes.
	(msp430_persist_attr): Remove.
	(attr_lower_exclusions): Remove ATTR_PERSIST exclusion.
	(attr_upper_exclusions): Likewise.
	(attr_either_exclusions): Likewise.
	(attr_persist_exclusions): Remove.
	(msp430_attribute_table): Remove ATTR_PERSIST handling.
	(msp430_handle_generic_attribute): Remove ATTR_PERSIST section conflict
	handling.
	(TARGET_ASM_INIT_SECTIONS): Remove.
	(msp430_init_sections): Remove.
	(msp430_select_section): Use default_elf_select_section for decls with
	the "persistent" attribute.
	(msp430_section_type_flags): Remove ".persistent" section handling.
	* doc/extend.texi (MSP430 Variable Attributes): Remove "noinit" and
	"persistent" documentation.

gcc/testsuite/ChangeLog:

	* g++.target/msp430/data-attributes.C: Remove expected warnings for
	"lower" attribute conflicts.
	Adjust expected wording for "persistent" attribute misuse.
	* gcc.target/msp430/data-attributes-2.c: Likewise.
	* gcc.target/msp430/pr78818-auto-warn.c: Likewise.
---
 gcc/config/msp430/msp430.c                    | 114 +++---------------
 gcc/doc/extend.texi                           |  17 ---
 .../g++.target/msp430/data-attributes.C       |  19 +--
 .../gcc.target/msp430/data-attributes-2.c     |  14 +--
 .../gcc.target/msp430/pr78818-auto-warn.c     |   4 +-
 5 files changed, 36 insertions(+), 132 deletions(-)

-- 
2.29.2

Patch

diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index 51f49edffa8..db3a9ff5330 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -1968,15 +1968,18 @@  msp430_section_attr (tree * node,
 
   const char * message = NULL;
 
-  /* The "noinit" and "section" attributes are handled generically, so we
-     cannot set up additional target-specific attribute exclusions using the
-     existing mechanism.  */
-  if (has_attr (ATTR_NOINIT, *node))
+  /* The "noinit", "persistent", and "section" attributes are handled
+     generically, so we cannot set up additional target-specific attribute
+     exclusions using the existing mechanism.  */
+  if (has_attr (ATTR_NOINIT, *node) && !TREE_NAME_EQ (name, "lower"))
     message = G_("ignoring attribute %qE because it conflicts with "
 		 "attribute %<noinit%>");
   else if (has_attr ("section", *node) && !TREE_NAME_EQ (name, "lower"))
     message = G_("ignoring attribute %qE because it conflicts with "
 		 "attribute %<section%>");
+  else if (has_attr (ATTR_PERSIST, *node) && !TREE_NAME_EQ (name, "lower"))
+    message = G_("ignoring attribute %qE because it conflicts with "
+		 "attribute %<persistent%>");
   /* It does not make sense to use upper/lower/either attributes without
      -mlarge.
      Without -mlarge, "lower" is the default and only region, so is redundant.
@@ -1997,56 +2000,6 @@  msp430_section_attr (tree * node,
   return NULL_TREE;
 }
 
-static tree
-msp430_persist_attr (tree *node,
-		  tree   name,
-		  tree   args,
-		  int    flags ATTRIBUTE_UNUSED,
-		  bool * no_add_attrs ATTRIBUTE_UNUSED)
-{
-  const char * message = NULL;
-
-  gcc_assert (DECL_P (* node));
-  gcc_assert (args == NULL);
-  gcc_assert (TREE_NAME_EQ (name, ATTR_PERSIST));
-
-  /* Check for the section attribute separately from DECL_SECTION_NAME so
-     we can provide a clearer warning.  */
-  if (has_attr ("section", *node))
-    message = G_("ignoring attribute %qE because it conflicts with "
-		 "attribute %<section%>");
-  /* Check that it's possible for the variable to have a section.  */
-  else if ((TREE_STATIC (*node) || DECL_EXTERNAL (*node) || in_lto_p)
-	   && (DECL_SECTION_NAME (*node)))
-    message = G_("%qE attribute cannot be applied to variables with specific "
-		 "sections");
-  else if (has_attr (ATTR_NOINIT, *node))
-    message = G_("ignoring attribute %qE because it conflicts with "
-		 "attribute %<noinit%>");
-  else if (TREE_CODE (*node) != VAR_DECL)
-    message = G_("%qE attribute only applies to variables");
-  else if (!TREE_STATIC (*node) && !TREE_PUBLIC (*node)
-	   && !DECL_EXTERNAL (*node))
-    message = G_("%qE attribute has no effect on automatic variables");
-  else if (DECL_COMMON (*node) || DECL_INITIAL (*node) == NULL)
-    message = G_("variables marked with %qE attribute must be initialized");
-  else
-    /* It's not clear if there is anything that can be set here to prevent the
-       front end placing the variable before the back end can handle it, in a
-       similar way to how DECL_COMMON is cleared for .noinit variables in
-       handle_noinit_attribute (gcc/c-family/c-attribs.c).
-       So just place the variable in the .persistent section now.  */
-    set_decl_section_name (* node, ".persistent");
-
-  if (message)
-    {
-      warning (OPT_Wattributes, message, name);
-      * no_add_attrs = true;
-    }
-
-  return NULL_TREE;
-}
-
 /* Helper to define attribute exclusions.  */
 #define ATTR_EXCL(name, function, type, variable)	\
   { name, function, type, variable }
@@ -2081,7 +2034,6 @@  static const struct attribute_spec::exclusions attr_lower_exclusions[] =
 {
   ATTR_EXCL (ATTR_UPPER, true, true, true),
   ATTR_EXCL (ATTR_EITHER, true, true, true),
-  ATTR_EXCL (ATTR_PERSIST, true, true, true),
   ATTR_EXCL (NULL, false, false, false)
 };
 
@@ -2089,7 +2041,6 @@  static const struct attribute_spec::exclusions attr_upper_exclusions[] =
 {
   ATTR_EXCL (ATTR_LOWER, true, true, true),
   ATTR_EXCL (ATTR_EITHER, true, true, true),
-  ATTR_EXCL (ATTR_PERSIST, true, true, true),
   ATTR_EXCL (NULL, false, false, false)
 };
 
@@ -2097,15 +2048,6 @@  static const struct attribute_spec::exclusions attr_either_exclusions[] =
 {
   ATTR_EXCL (ATTR_LOWER, true, true, true),
   ATTR_EXCL (ATTR_UPPER, true, true, true),
-  ATTR_EXCL (ATTR_PERSIST, true, true, true),
-  ATTR_EXCL (NULL, false, false, false)
-};
-
-static const struct attribute_spec::exclusions attr_persist_exclusions[] =
-{
-  ATTR_EXCL (ATTR_LOWER, true, true, true),
-  ATTR_EXCL (ATTR_UPPER, true, true, true),
-  ATTR_EXCL (ATTR_EITHER, true, true, true),
   ATTR_EXCL (NULL, false, false, false)
 };
 
@@ -2133,9 +2075,6 @@  const struct attribute_spec msp430_attribute_table[] =
     { ATTR_EITHER,      0, 0, true,  false, false, false, msp430_section_attr,
       attr_either_exclusions },
 
-    { ATTR_PERSIST,     0, 0, true,  false, false, false, msp430_persist_attr,
-      attr_persist_exclusions },
-
     { NULL,		0, 0, false, false, false, false, NULL,  NULL }
   };
 
@@ -2152,14 +2091,13 @@  msp430_handle_generic_attribute (tree *node,
 {
   const char *message = NULL;
 
-  /* The front end has set up an exclusion between the "noinit" and "section"
-     attributes.  */
-  if (!(TREE_NAME_EQ (name, ATTR_NOINIT) || TREE_NAME_EQ (name, "section")))
-    return NULL_TREE;
-
-  /* We allow the "lower" attribute to be used on variables with the "section"
-     attribute.  */
-  if (has_attr (ATTR_LOWER, *node) && !TREE_NAME_EQ (name, "section"))
+  /* Permit the "lower" attribute to be set on variables with the "section",
+     "noinit" and "persistent" attributes.  This is used to indicate that the
+     corresponding output section will be in lower memory, so a 430X
+     instruction is not required to handle it.  */
+  if (has_attr (ATTR_LOWER, *node)
+      && !(TREE_NAME_EQ (name, "section") || TREE_NAME_EQ (name, ATTR_PERSIST)
+	   || TREE_NAME_EQ (name, ATTR_NOINIT)))
     message = G_("ignoring attribute %qE because it conflicts with "
 		 "attribute %<lower%>");
   else if (has_attr (ATTR_UPPER, *node))
@@ -2168,9 +2106,6 @@  msp430_handle_generic_attribute (tree *node,
   else if (has_attr (ATTR_EITHER, *node))
     message = G_("ignoring attribute %qE because it conflicts with "
 		 "attribute %<either%>");
-  else if (has_attr (ATTR_PERSIST, *node))
-    message = G_("ignoring attribute %qE because it conflicts with "
-		 "attribute %<persistent%>");
 
   if (message)
     {
@@ -2428,18 +2363,6 @@  gen_prefix (tree decl)
   return NULL;
 }
 
-static section * persist_section;
-
-#undef  TARGET_ASM_INIT_SECTIONS
-#define TARGET_ASM_INIT_SECTIONS msp430_init_sections
-
-static void
-msp430_init_sections (void)
-{
-  persist_section = get_unnamed_section (0, output_section_asm_op,
-					 ".section .persistent,\"aw\"");
-}
-
 #undef  TARGET_ASM_SELECT_SECTION
 #define TARGET_ASM_SELECT_SECTION msp430_select_section
 
@@ -2465,11 +2388,8 @@  msp430_select_section (tree decl, int reloc, unsigned HOST_WIDE_INT align)
       && is_interrupt_func (decl))
     return get_section (".lowtext", SECTION_CODE | SECTION_WRITE , decl);
 
-  if (has_attr (ATTR_PERSIST, decl))
-    return persist_section;
-
-  /* ATTR_NOINIT is handled generically.  */
-  if (has_attr (ATTR_NOINIT, decl))
+  /* The "noinit" and "persistent" attributes are handled generically.  */
+  if (has_attr (ATTR_NOINIT, decl) || has_attr (ATTR_PERSIST, decl))
     return default_elf_select_section (decl, reloc, align);
 
   prefix = gen_prefix (decl);
@@ -2565,8 +2485,6 @@  msp430_section_type_flags (tree decl, const char * name, int reloc)
     name += strlen (upper_prefix);
   else if (strncmp (name, either_prefix, strlen (either_prefix)) == 0)
     name += strlen (either_prefix);
-  else if (strcmp (name, ".persistent") == 0)
-    return SECTION_WRITE | SECTION_NOTYPE;
 
   return default_section_type_flags (decl, name, reloc);
 }
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 6018347daed..23ede966bae 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -7863,23 +7863,6 @@  The @code{shared} attribute is only available on Microsoft Windows@.
 @subsection MSP430 Variable Attributes
 
 @table @code
-@item noinit
-@cindex @code{noinit} variable attribute, MSP430 
-Any data with the @code{noinit} attribute will not be initialised by
-the C runtime startup code, or the program loader.  Not initialising
-data in this way can reduce program startup times.
-
-@item persistent
-@cindex @code{persistent} variable attribute, MSP430 
-Any variable with the @code{persistent} attribute will not be
-initialised by the C runtime startup code.  Instead its value will be
-set once, when the application is loaded, and then never initialised
-again, even if the processor is reset or the program restarts.
-Persistent data is intended to be placed into FLASH RAM, where its
-value will be retained across resets.  The linker script being used to
-create the application should ensure that persistent data is correctly
-placed.
-
 @item upper
 @itemx either
 @cindex @code{upper} variable attribute, MSP430 
diff --git a/gcc/testsuite/g++.target/msp430/data-attributes.C b/gcc/testsuite/g++.target/msp430/data-attributes.C
index 4e2139e93f7..b7faf2ce7e5 100644
--- a/gcc/testsuite/g++.target/msp430/data-attributes.C
+++ b/gcc/testsuite/g++.target/msp430/data-attributes.C
@@ -2,22 +2,25 @@ 
 /* { dg-skip-if "" { *-*-* } { "-mcpu=msp430" } } */
 /* { dg-options "-mlarge" } */
 
-/* The msp430-specific variable attributes "lower", "upper", either", "noinit"
+/* The msp430-specific variable attributes "upper", either", "noinit"
    and "persistent", all conflict with one another.
+   "lower" can be used to indicate that a variable with a section set by the
+   "section", "noinit", or "persistent" attributes is in lower memory, so it
+   does not conflict with these.
    These attributes also conflict with the "section" attribute, since they
    specify sections to put the variables into.  */
 int __attribute__((persistent)) p = 10;
-int __attribute__((persistent,lower)) pl = 20; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'persistent'" } */
+int __attribute__((persistent,lower)) pl = 20;
 int __attribute__((persistent,upper)) pu = 20; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'persistent'" } */
 int __attribute__((persistent,either)) pe = 20; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'persistent'" } */
 /* This one results in an error because the handler for persistent sets the
    section to .persistent there and then.  */
-int __attribute__((persistent,section(".data.foo"))) ps = 20; /* { dg-error "section of 'ps' conflicts with previous declaration" } */
-int __attribute__((persistent,noinit)) pn = 2; /* { dg-warning "'noinit' attribute cannot be applied to variables with specific sections" } */
-int __attribute__((persistent)) zz; /* { dg-warning "variables marked with 'persistent' attribute must be initialized" } */
+int __attribute__((persistent,section(".data.foo"))) ps = 20; /* { dg-warning "ignoring attribute 'section' because it conflicts with attribute 'persistent'" } */
+int __attribute__((persistent,noinit)) pn = 2; /* { dg-warning "ignoring attribute 'noinit' because it conflicts with attribute 'persistent'" } */
+int __attribute__((persistent)) zz; /* { dg-warning "ignoring 'persistent' attribute set on uninitialized variable" } */
 
 int __attribute__((noinit)) n;
-int __attribute__((noinit,lower)) nl; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'noinit'" } */
+int __attribute__((noinit,lower)) nl;
 int __attribute__((noinit,upper)) nu; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'noinit'" } */
 int __attribute__((noinit,either)) ne; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'noinit'" } */
 int __attribute__((noinit,persistent)) np; /* { dg-warning "ignoring attribute 'persistent' because it conflicts with attribute 'noinit'" } */
@@ -26,8 +29,8 @@  int __attribute__((noinit,section(".data.foo"))) ns; /* { dg-warning "ignoring a
 int __attribute__((lower)) l = 20;
 int __attribute__((lower,upper)) lu = 20; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'lower'" } */
 int __attribute__((lower,either)) le = 20; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'lower'" } */
-int __attribute__((lower,persistent)) lp = 20; /* { dg-warning "ignoring attribute 'persistent' because it conflicts with attribute 'lower'" } */
-int __attribute__((lower,noinit)) ln; /* { dg-warning "ignoring attribute 'noinit' because it conflicts with attribute 'lower'" } */
+int __attribute__((lower,persistent)) lp = 20;
+int __attribute__((lower,noinit)) ln;
 int __attribute__((lower,section(".data.foo"))) ls = 30;
 
 int __attribute__((upper)) u = 20;
diff --git a/gcc/testsuite/gcc.target/msp430/data-attributes-2.c b/gcc/testsuite/gcc.target/msp430/data-attributes-2.c
index 4e2139e93f7..cf456f1b2a8 100644
--- a/gcc/testsuite/gcc.target/msp430/data-attributes-2.c
+++ b/gcc/testsuite/gcc.target/msp430/data-attributes-2.c
@@ -7,17 +7,17 @@ 
    These attributes also conflict with the "section" attribute, since they
    specify sections to put the variables into.  */
 int __attribute__((persistent)) p = 10;
-int __attribute__((persistent,lower)) pl = 20; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'persistent'" } */
+int __attribute__((persistent,lower)) pl = 20;
 int __attribute__((persistent,upper)) pu = 20; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'persistent'" } */
 int __attribute__((persistent,either)) pe = 20; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'persistent'" } */
 /* This one results in an error because the handler for persistent sets the
    section to .persistent there and then.  */
-int __attribute__((persistent,section(".data.foo"))) ps = 20; /* { dg-error "section of 'ps' conflicts with previous declaration" } */
-int __attribute__((persistent,noinit)) pn = 2; /* { dg-warning "'noinit' attribute cannot be applied to variables with specific sections" } */
-int __attribute__((persistent)) zz; /* { dg-warning "variables marked with 'persistent' attribute must be initialized" } */
+int __attribute__((persistent,section(".data.foo"))) ps = 20; /* { dg-warning "ignoring attribute 'section' because it conflicts with attribute 'persistent'" } */
+int __attribute__((persistent,noinit)) pn = 2; /* { dg-warning "ignoring attribute 'noinit' because it conflicts with attribute 'persistent'" } */
+int __attribute__((persistent)) zz; /* { dg-warning "ignoring 'persistent' attribute set on uninitialized variable" } */
 
 int __attribute__((noinit)) n;
-int __attribute__((noinit,lower)) nl; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'noinit'" } */
+int __attribute__((noinit,lower)) nl;
 int __attribute__((noinit,upper)) nu; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'noinit'" } */
 int __attribute__((noinit,either)) ne; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'noinit'" } */
 int __attribute__((noinit,persistent)) np; /* { dg-warning "ignoring attribute 'persistent' because it conflicts with attribute 'noinit'" } */
@@ -26,8 +26,8 @@  int __attribute__((noinit,section(".data.foo"))) ns; /* { dg-warning "ignoring a
 int __attribute__((lower)) l = 20;
 int __attribute__((lower,upper)) lu = 20; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'lower'" } */
 int __attribute__((lower,either)) le = 20; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'lower'" } */
-int __attribute__((lower,persistent)) lp = 20; /* { dg-warning "ignoring attribute 'persistent' because it conflicts with attribute 'lower'" } */
-int __attribute__((lower,noinit)) ln; /* { dg-warning "ignoring attribute 'noinit' because it conflicts with attribute 'lower'" } */
+int __attribute__((lower,persistent)) lp = 20;
+int __attribute__((lower,noinit)) ln;
 int __attribute__((lower,section(".data.foo"))) ls = 30;
 
 int __attribute__((upper)) u = 20;
diff --git a/gcc/testsuite/gcc.target/msp430/pr78818-auto-warn.c b/gcc/testsuite/gcc.target/msp430/pr78818-auto-warn.c
index 3dba361071f..7ed5ca83000 100644
--- a/gcc/testsuite/gcc.target/msp430/pr78818-auto-warn.c
+++ b/gcc/testsuite/gcc.target/msp430/pr78818-auto-warn.c
@@ -8,8 +8,8 @@  static __attribute__((persistent)) int persistent_4_g = 0;
 int
 main (void)
 {
-  __attribute__((persistent)) int persistent_1 = 1; /* { dg-warning "attribute has no effect on automatic" } */
-  __attribute__((persistent)) int persistent_2 = 0; /* { dg-warning "attribute has no effect on automatic" } */
+  __attribute__((persistent)) int persistent_1 = 1; /* { dg-error "'persistent' attribute cannot be specified for local variables" } */
+  __attribute__((persistent)) int persistent_2 = 0; /* { dg-error "'persistent' attribute cannot be specified for local variables" } */
   static __attribute__((persistent)) int persistent_3 = 1;
   static __attribute__((persistent)) int persistent_4 = 0;
   return 0;