[PATCH 0/3] MSP430: Improve attribute handling

[PATCH 0/3] MSP430: Improve attribute handling

[Jozef Lawrynowicz]

The following series of patches improves the handling of msp430-specific
attributes by making use of generic mechanisms for performing common
tasks (i.e. handling attribute conflicts, putting data objects in sections).

The patches also transition the msp430 back end to fully use the generic
handling of the "noinit" attribute.

Successfully bootstrapped and regtested on x86_64-pc-linux-gnu.
Successfully regtested for msp430-elf.

As a further sanity test I built GCC for arm-eabi and ran
execute.exp=noinit-attribute.c to confirm the noinit attribute still
works as expected for ARM.

Ok for trunk?

Jozef Lawrynowicz (3):
  Implement TARGET_HANDLE_GENERIC_ATTRIBUTE
  MSP430: Setup exclusion tables for function and data attributes
  MSP430: Use default_elf_select_section to determine sections for data
    where possible

 gcc/c-family/c-attribs.c                      |  39 ++-
 gcc/config/msp430/msp430.c                    | 320 ++++++++++++------
 gcc/doc/tm.texi                               |   8 +
 gcc/doc/tm.texi.in                            |   2 +
 gcc/hooks.c                                   |   6 +
 gcc/hooks.h                                   |   1 +
 gcc/target.def                                |  11 +
 .../gcc.target/msp430/data-attributes-2.c     |  51 +++
 .../gcc.target/msp430/function-attributes-4.c |  27 +-
 .../msp430/region-attribute-misuse.c          |   6 +-
 10 files changed, 336 insertions(+), 135 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/msp430/data-attributes-2.c

[PATCH 1/3] Implement TARGET_HANDLE_GENERIC_ATTRIBUTE

[Jozef Lawrynowicz]

[-- Attachment #1: Type: text/plain, Size: 380 bytes --]

The attached patch adds a new target hook "TARGET_HANDLE_GENERIC_ATTRIBUTE"
which enables a back end to perform additional processing of an attribute that
is normally handled by a front end.

So far only the "section" and "noinit" attribute make use of this hook, as the
msp430 back end requires additional attribute conflict checking to be performed
on these generic attributes.

[-- Attachment #2: 0001-Implement-TARGET_HANDLE_GENERIC_ATTRIBUTE.patch --]
[-- Type: text/x-patch, Size: 8554 bytes --]

From e693da709114df378e2ea8b1d3729b105c99a495 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Wed, 28 Aug 2019 14:09:20 +0100
Subject: [PATCH 1/3] Implement TARGET_HANDLE_GENERIC_ATTRIBUTE

gcc/ChangeLog:

2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config/msp430/msp430.c (TARGET_HANDLE_GENERIC_ATTRIBUTE): Define.
	(msp430_handle_generic_attribute): New function.
	* doc/tm.texi: Regenerate.
	* doc/tm.texi.in: Add TARGET_HANDLE_GENERIC_ATTRIBUTE.
	* hooks.c (hook_tree_treeptr_tree_tree_int_boolptr_null): New.
	* hooks.h (hook_tree_treeptr_tree_tree_int_boolptr_null): New.
	* target.def: Define new hook TARGET_HANDLE_GENERIC_ATTRIBUTE.

gcc/c-family/ChangeLog:

2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* c-attribs.c (handle_section_attribute): Call the
	handle_generic_attribute target hook after performing target
	independent processing.
	(handle_noinit_attribute): Likewise.
---
 gcc/c-family/c-attribs.c   | 39 +++++++++++++++++++++++++++----------
 gcc/config/msp430/msp430.c | 40 ++++++++++++++++++++++++++++++++++++++
 gcc/doc/tm.texi            |  8 ++++++++
 gcc/doc/tm.texi.in         |  2 ++
 gcc/hooks.c                |  6 ++++++
 gcc/hooks.h                |  1 +
 gcc/target.def             | 11 +++++++++++
 7 files changed, 97 insertions(+), 10 deletions(-)

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 820c83fa3b9..e572c6502bb 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -1875,6 +1875,7 @@ handle_section_attribute (tree *node, tree ARG_UNUSED (name), tree args,
 			  int ARG_UNUSED (flags), bool *no_add_attrs)
 {
   tree decl = *node;
+  tree res = NULL_TREE;
 
   if (!targetm_common.have_named_sections)
     {
@@ -1922,12 +1923,20 @@ handle_section_attribute (tree *node, tree ARG_UNUSED (name), tree args,
       goto fail;
     }
 
-  set_decl_section_name (decl, TREE_STRING_POINTER (TREE_VALUE (args)));
-  return NULL_TREE;
+  res = targetm.handle_generic_attribute (node, name, args, flags,
+					  no_add_attrs);
+
+  /* If the back end confirms the attribute can be added then continue onto
+     final processing.  */
+  if (!(* no_add_attrs))
+    {
+      set_decl_section_name (decl, TREE_STRING_POINTER (TREE_VALUE (args)));
+      return res;
+    }
 
 fail:
   *no_add_attrs = true;
-  return NULL_TREE;
+  return res;
 }
 
 /* If in c++-11, check if the c++-11 alignment constraint with respect
@@ -2249,6 +2258,7 @@ handle_noinit_attribute (tree * node,
 		  bool *no_add_attrs)
 {
   const char *message = NULL;
+  tree res = NULL_TREE;
 
   gcc_assert (DECL_P (*node));
   gcc_assert (args == NULL);
@@ -2271,14 +2281,23 @@ handle_noinit_attribute (tree * node,
       *no_add_attrs = true;
     }
   else
-    /* If this var is thought to be common, then change this.  Common
-       variables are assigned to sections before the backend has a
-       chance to process them.  Do this only if the attribute is
-       valid.  */
-    if (DECL_COMMON (*node))
-      DECL_COMMON (*node) = 0;
+    {
+      res = targetm.handle_generic_attribute (node, name, args, flags,
+					      no_add_attrs);
+      /* If the back end confirms the attribute can be added then continue onto
+	 final processing.  */
+      if (!(* no_add_attrs))
+	{
+	  /* If this var is thought to be common, then change this.  Common
+	     variables are assigned to sections before the backend has a
+	     chance to process them.  Do this only if the attribute is
+	     valid.  */
+	  if (DECL_COMMON (* node))
+	    DECL_COMMON (* node) = 0;
+	}
+    }
 
-  return NULL_TREE;
+  return res;
 }
 
 
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index c5cf7044aef..d54a3749437 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -1518,6 +1518,46 @@ const struct attribute_spec msp430_attribute_table[] =
     { NULL,		0, 0, false, false, false, false, NULL,  NULL }
   };
 
+#undef TARGET_HANDLE_GENERIC_ATTRIBUTE
+#define TARGET_HANDLE_GENERIC_ATTRIBUTE msp430_handle_generic_attribute
+
+tree
+msp430_handle_generic_attribute (tree * node,
+				 tree   name,
+				 tree   args ATTRIBUTE_UNUSED,
+				 int    flags ATTRIBUTE_UNUSED,
+				 bool * no_add_attrs)
+
+{
+  const char * message = NULL;
+
+  if (!(TREE_NAME_EQ (name, ATTR_NOINIT) || TREE_NAME_EQ (name, "section")))
+    return NULL_TREE;
+
+  /* The front end has set up an exclusion between the "noinit" and "section"
+     attributes.  */
+  if (has_attr (ATTR_LOWER, * node))
+    message = G_("ignoring attribute %qE because it conflicts with "
+		 "attribute %<lower%>");
+  else if (has_attr (ATTR_UPPER, * node))
+    message = G_("ignoring attribute %qE because it conflicts with "
+		 "attribute %<upper%>");
+  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)
+    {
+      warning (OPT_Wattributes, message, name);
+      * no_add_attrs = true;
+    }
+
+  return NULL_TREE;
+}
+
 #undef  TARGET_ASM_FUNCTION_PROLOGUE
 #define TARGET_ASM_FUNCTION_PROLOGUE	msp430_start_function
 
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 55069088b52..0b5a08d490e 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -10391,6 +10391,14 @@ attributes, or a copy of the list may be made if further changes are
 needed.
 @end deftypefn
 
+@deftypefn {Target Hook} tree TARGET_HANDLE_GENERIC_ATTRIBUTE (tree *@var{node}, tree @var{name}, tree @var{args}, int @var{flags}, bool *@var{no_add_attrs})
+Define this target hook if you want to be able to perform additional
+target-specific processing of an attribute which is handled generically
+by a front end.  The arguments are the same as those which are passed to
+attribute handlers.  So far this only affects the @var{noinit} and
+@var{section} attribute.
+@end deftypefn
+
 @deftypefn {Target Hook} bool TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P (const_tree @var{fndecl})
 @cindex inlining
 This target hook returns @code{true} if it is OK to inline @var{fndecl}
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 98e710058bc..a9200551ed4 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -7200,6 +7200,8 @@ on this implementation detail.
 
 @hook TARGET_INSERT_ATTRIBUTES
 
+@hook TARGET_HANDLE_GENERIC_ATTRIBUTE
+
 @hook TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P
 
 @hook TARGET_OPTION_VALID_ATTRIBUTE_P
diff --git a/gcc/hooks.c b/gcc/hooks.c
index f95659b3807..ca731c440e7 100644
--- a/gcc/hooks.c
+++ b/gcc/hooks.c
@@ -442,6 +442,12 @@ hook_tree_tree_tree_tree_null (tree, tree, tree)
   return NULL;
 }
 
+tree
+hook_tree_treeptr_tree_tree_int_boolptr_null (tree *, tree, tree, int, bool *)
+{
+  return NULL;
+}
+
 /* Generic hook that takes an rtx_insn *and returns a NULL string.  */
 const char *
 hook_constcharptr_const_rtx_insn_null (const rtx_insn *)
diff --git a/gcc/hooks.h b/gcc/hooks.h
index 0bc8117c2c8..040eff008db 100644
--- a/gcc/hooks.h
+++ b/gcc/hooks.h
@@ -109,6 +109,7 @@ extern tree hook_tree_void_null (void);
 extern tree hook_tree_tree_tree_null (tree, tree);
 extern tree hook_tree_tree_tree_tree_null (tree, tree, tree);
 extern tree hook_tree_tree_int_treep_bool_null (tree, int, tree *, bool);
+extern tree hook_tree_treeptr_tree_tree_int_boolptr_null (tree *, tree, tree, int, bool *);
 
 extern unsigned hook_uint_void_0 (void);
 extern unsigned int hook_uint_mode_0 (machine_mode);
diff --git a/gcc/target.def b/gcc/target.def
index b2332d8215c..ca7e7ad96b4 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -2208,6 +2208,17 @@ needed.",
  void, (tree node, tree *attr_ptr),
  hook_void_tree_treeptr)
 
+/* Perform additional target-specific processing of generic attributes.  */
+DEFHOOK
+(handle_generic_attribute,
+ "Define this target hook if you want to be able to perform additional\n\
+target-specific processing of an attribute which is handled generically\n\
+by a front end.  The arguments are the same as those which are passed to\n\
+attribute handlers.  So far this only affects the @var{noinit} and\n\
+@var{section} attribute.",
+ tree, (tree *node, tree name, tree args, int flags, bool *no_add_attrs),
+ hook_tree_treeptr_tree_tree_int_boolptr_null)
+
 /* Return true if FNDECL (which has at least one machine attribute)
    can be inlined despite its machine attributes, false otherwise.  */
 DEFHOOK
-- 
2.17.1

[PATCH 2/3][MSP430] Setup exclusion tables for function and data attributes

[Jozef Lawrynowicz]

[-- Attachment #1: Type: text/plain, Size: 257 bytes --]

The attached patch removes hard-coded warnings from msp430 attribute handlers,
and replaces them with exclusion rules specified in the attribute_spec table.

Where msp430 attributes conflict with generic attributes, hard-coded warnings
are still necessary.

[-- Attachment #2: 0002-MSP430-Setup-exclusion-tables-for-function-and-data-.patch --]
[-- Type: text/x-patch, Size: 23216 bytes --]

From c6def571a47df5394ca9f5c5c4918f0ffcf97375 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Wed, 28 Aug 2019 17:44:16 +0100
Subject: [PATCH 2/3] MSP430: Setup exclusion tables for function and data
 attributes

gcc/ChangeLog:

2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config/msp430/msp430.c (msp430_attr): Remove warnings about
	conflicting msp430-specific attributes.
	(msp430_section_attr): Likewise.
	Add warnings about conflicts with generic "noinit" and "section"
	attributes.
	Fix grammar in -mlarge error message.
	(msp430_data_attr): Rename to msp430_persist_attr.
	Add warnings about conflicts with generic "noinit" and "section"
	attributes.
	Add warning for when variable is not initialized.
	Chain conditionals which prevent the attribute being added.
	(ATTR_EXCL): New helper.
	(attr_reent_exclusions): New exclusion table.
	(attr_naked_exclusions): Likewise.
	(attr_crit_exclusions): Likewise.
	(attr_lower_exclusions): Likewise.
	(attr_upper_exclusions): Likewise.
	(attr_either_exclusions): Likewise.
	(attr_persist_exclusions): Likewise.
	(msp430_attribute_table): Update with exclusion rules.
	(msp430_output_aligned_decl_common): Don't output common symbol if decl
	has a section.

gcc/testsuite/ChangeLog:

2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* gcc.target/msp430/data-attributes-2.c: New test.
	* gcc.target/msp430/function-attributes-4.c: Update dg-warning
	strings.
	* gcc.target/msp430/region-attribute-misuse.c: Likewise.

---
 gcc/config/msp430/msp430.c                    | 199 +++++++++++-------
 .../gcc.target/msp430/data-attributes-2.c     |  51 +++++
 .../gcc.target/msp430/function-attributes-4.c |  27 +--
 .../msp430/region-attribute-misuse.c          |   6 +-
 4 files changed, 189 insertions(+), 94 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/msp430/data-attributes-2.c

diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index d54a3749437..1b2e9683a94 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -1345,35 +1345,19 @@ msp430_attr (tree * node,
 	}
       if (is_critical_func (* node))
 	{
+	  /* We always ignore the critical attribute when interrupt and
+	     critical are used together.  */
 	  warning (OPT_Wattributes,
 		   "critical attribute has no effect on interrupt functions");
 	  DECL_ATTRIBUTES (*node) = remove_attribute (ATTR_CRIT,
 						      DECL_ATTRIBUTES (* node));
 	}
     }
-  else if (TREE_NAME_EQ (name, ATTR_REENT))
-    {
-      if (is_naked_func (* node))
-	message = "naked functions cannot be reentrant";
-      else if (is_critical_func (* node))
-	message = "critical functions cannot be reentrant";
-    }
   else if (TREE_NAME_EQ (name, ATTR_CRIT))
     {
-      if (is_naked_func (* node))
-	message = "naked functions cannot be critical";
-      else if (is_reentrant_func (* node))
-	message = "reentrant functions cannot be critical";
-      else if (is_interrupt_func ( *node))
+      if (is_interrupt_func ( *node))
 	message = "critical attribute has no effect on interrupt functions";
     }
-  else if (TREE_NAME_EQ (name, ATTR_NAKED))
-    {
-      if (is_critical_func (* node))
-	message = "critical functions cannot be naked";
-      else if (is_reentrant_func (* node))
-	message = "reentrant functions cannot be naked";
-    }
 
   if (message)
     {
@@ -1396,32 +1380,15 @@ msp430_section_attr (tree * node,
 
   const char * message = NULL;
 
-  if (TREE_NAME_EQ (name, ATTR_UPPER))
-    {
-      if (has_attr (ATTR_LOWER, * node))
-	message = "already marked with 'lower' attribute";
-      else if (has_attr (ATTR_EITHER, * node))
-	message = "already marked with 'either' attribute";
-      else if (! msp430x)
-	message = "upper attribute needs a 430X cpu";
-    }
-  else if (TREE_NAME_EQ (name, ATTR_LOWER))
-    {
-      if (has_attr (ATTR_UPPER, * node))
-	message = "already marked with 'upper' attribute";
-      else if (has_attr (ATTR_EITHER, * node))
-	message = "already marked with 'either' attribute";
-    }
-  else
-    {
-      gcc_assert (TREE_NAME_EQ (name, ATTR_EITHER));
-
-      if (has_attr (ATTR_LOWER, * node))
-	message = "already marked with 'lower' attribute";
-      else if (has_attr (ATTR_UPPER, * node))
-	message = "already marked with 'upper' attribute";
-    }
-
+  /* 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))
+    message = G_("ignoring attribute %qE because it conflicts with "
+		 "attribute %<noinit%>");
+  else if (has_attr ("section", * node))
+    message = G_("ignoring attribute %qE because it conflicts with "
+		 "attribute %<section%>");
   /* 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.
@@ -1429,9 +1396,9 @@ msp430_section_attr (tree * node,
      upper region, which for data could result in relocation overflows, and for
      code could result in stack mismanagement and incorrect call/return
      instructions.  */
-  if (!TARGET_LARGE)
-    message = G_("%qE attribute ignored. large memory model (%<-mlarge%>) "
-		 "is required");
+  else if (!TARGET_LARGE)
+    message = G_("%qE attribute ignored.  Large memory model (%<-mlarge%>) "
+		 "is required.");
 
   if (message)
     {
@@ -1443,7 +1410,7 @@ msp430_section_attr (tree * node,
 }
 
 static tree
-msp430_data_attr (tree * node,
+msp430_persist_attr (tree * node,
 		  tree   name,
 		  tree   args,
 		  int    flags ATTRIBUTE_UNUSED,
@@ -1453,34 +1420,36 @@ msp430_data_attr (tree * node,
 
   gcc_assert (DECL_P (* node));
   gcc_assert (args == NULL);
+  gcc_assert (TREE_NAME_EQ (name, ATTR_PERSIST));
 
-  if (TREE_CODE (* node) != VAR_DECL)
-    message = G_("%qE attribute only applies to variables");
-
+  /* 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.  */
-  if ((TREE_STATIC (* node) || DECL_EXTERNAL (* node) || in_lto_p)
-      && DECL_SECTION_NAME (* node))
+  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");
-
-  if (!message && TREE_NAME_EQ (name, ATTR_PERSIST) && !TREE_STATIC (* node)
-      && !TREE_PUBLIC (* node) && !DECL_EXTERNAL (* node))
+  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");
-
-  /* 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 used below.
-     So just place the variable in the .persistent section now.  */
-  if ((TREE_STATIC (* node) || DECL_EXTERNAL (* node) || in_lto_p)
-      && TREE_NAME_EQ (name, ATTR_PERSIST))
+  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 this var is thought to be common, then change this.  Common variables
-     are assigned to sections before the backend has a chance to process
-     them.  */
-  if (DECL_COMMON (* node))
-    DECL_COMMON (* node) = 0;
-
   if (message)
     {
       warning (OPT_Wattributes, message, name);
@@ -1490,6 +1459,67 @@ msp430_data_attr (tree * node,
   return NULL_TREE;
 }
 
+/* Helper to define attribute exclusions.  */
+#define ATTR_EXCL(name, function, type, variable)	\
+  { name, function, type, variable }
+
+/* "reentrant", "critical" and "naked" functions must conflict because
+   they all modify the prologue or epilogue of functions in mutually exclusive
+   ways.  */
+static const struct attribute_spec::exclusions attr_reent_exclusions[] =
+{
+  ATTR_EXCL (ATTR_NAKED, true, true, true),
+  ATTR_EXCL (ATTR_CRIT, true, true, true),
+  ATTR_EXCL (NULL, false, false, false)
+};
+
+static const struct attribute_spec::exclusions attr_naked_exclusions[] =
+{
+  ATTR_EXCL (ATTR_REENT, true, true, true),
+  ATTR_EXCL (ATTR_CRIT, true, true, true),
+  ATTR_EXCL (NULL, false, false, false)
+};
+
+static const struct attribute_spec::exclusions attr_crit_exclusions[] =
+{
+  ATTR_EXCL (ATTR_REENT, true, true, true),
+  ATTR_EXCL (ATTR_NAKED, true, true, true),
+  ATTR_EXCL (NULL, false, false, false)
+};
+
+/* Attributes which put the given object in a specific section must conflict
+   with one another.  */
+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)
+};
+
+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)
+};
+
+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)
+};
 
 #undef  TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLE		msp430_attribute_table
@@ -1500,20 +1530,23 @@ const struct attribute_spec msp430_attribute_table[] =
     /* { name, min_num_args, max_num_args, decl_req, type_req, fn_type_req,
 	 affects_type_identity, handler, exclude } */
     { ATTR_INTR,	0, 1, true,  false, false, false, msp430_attr, NULL },
-    { ATTR_NAKED,       0, 0, true,  false, false, false, msp430_attr, NULL },
-    { ATTR_REENT,       0, 0, true,  false, false, false, msp430_attr, NULL },
-    { ATTR_CRIT,	0, 0, true,  false, false, false, msp430_attr, NULL },
+    { ATTR_NAKED,       0, 0, true,  false, false, false, msp430_attr,
+      attr_naked_exclusions },
+    { ATTR_REENT,       0, 0, true,  false, false, false, msp430_attr,
+      attr_reent_exclusions },
+    { ATTR_CRIT,	0, 0, true,  false, false, false, msp430_attr,
+      attr_crit_exclusions },
     { ATTR_WAKEUP,      0, 0, true,  false, false, false, msp430_attr, NULL },
 
     { ATTR_LOWER,       0, 0, true,  false, false, false, msp430_section_attr,
-      NULL },
+      attr_lower_exclusions },
     { ATTR_UPPER,       0, 0, true,  false, false, false, msp430_section_attr,
-      NULL },
+      attr_upper_exclusions },
     { ATTR_EITHER,      0, 0, true,  false, false, false, msp430_section_attr,
-      NULL },
+      attr_either_exclusions },
 
-    { ATTR_PERSIST,     0, 0, true,  false, false, false, msp430_data_attr,
-      NULL },
+    { ATTR_PERSIST,     0, 0, true,  false, false, false, msp430_persist_attr,
+      attr_persist_exclusions },
 
     { NULL,		0, 0, false, false, false, false, NULL,  NULL }
   };
@@ -1922,7 +1955,15 @@ msp430_output_aligned_decl_common (FILE *		  stream,
 				   unsigned HOST_WIDE_INT size,
 				   unsigned int		  align)
 {
-  if (msp430_data_region == MSP430_REGION_ANY)
+  /* Only emit a common symbol if the variable does not have a specific section
+     assigned.  */
+  if (msp430_data_region == MSP430_REGION_ANY
+      && !(decl != NULL_TREE && DECL_SECTION_NAME (decl))
+      && !has_attr (ATTR_EITHER, decl)
+      && !has_attr (ATTR_LOWER, decl)
+      && !has_attr (ATTR_UPPER, decl)
+      && !has_attr (ATTR_PERSIST, decl)
+      && !has_attr (ATTR_NOINIT, decl))
     {
       fprintf (stream, COMMON_ASM_OP);
       assemble_name (stream, name);
diff --git a/gcc/testsuite/gcc.target/msp430/data-attributes-2.c b/gcc/testsuite/gcc.target/msp430/data-attributes-2.c
new file mode 100644
index 00000000000..d049194425d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/data-attributes-2.c
@@ -0,0 +1,51 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430" } } */
+/* { dg-options "-mlarge" } */
+
+/* The msp430-specific variable attributes "lower", "upper", either", "noinit"
+   and "persistent", all conflict with one another.
+   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,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__((noinit)) n;
+int __attribute__((noinit,lower)) nl; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'noinit'" } */
+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'" } */
+int __attribute__((noinit,section(".data.foo"))) ns; /* { dg-warning "ignoring attribute 'section' because it conflicts with attribute 'noinit'" } */
+
+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,section(".data.foo"))) ls = 30; /* { dg-warning "ignoring attribute 'section' because it conflicts with attribute 'lower'" } */
+
+int __attribute__((upper)) u = 20;
+int __attribute__((upper,lower)) ul = 20; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'upper'" } */
+int __attribute__((upper,either)) ue = 20; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'upper'" } */
+int __attribute__((upper,persistent)) up = 20; /* { dg-warning "ignoring attribute 'persistent' because it conflicts with attribute 'upper'" } */
+int __attribute__((upper,noinit)) un; /* { dg-warning "ignoring attribute 'noinit' because it conflicts with attribute 'upper'" } */
+int __attribute__((upper,section(".data.foo"))) us = 30; /* { dg-warning "ignoring attribute 'section' because it conflicts with attribute 'upper'" } */
+
+int __attribute__((either)) e = 20;
+int __attribute__((either,lower)) el = 20; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'either'" } */
+int __attribute__((either,upper)) ee = 20; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'either'" } */
+int __attribute__((either,persistent)) ep = 20; /* { dg-warning "ignoring attribute 'persistent' because it conflicts with attribute 'either'" } */
+int __attribute__((either,noinit)) en; /* { dg-warning "ignoring attribute 'noinit' because it conflicts with attribute 'either'" } */
+int __attribute__((either,section(".data.foo"))) es = 30; /* { dg-warning "ignoring attribute 'section' because it conflicts with attribute 'either'" } */
+
+int __attribute__((section(".data.foo"))) s = 20;
+int __attribute__((section(".data.foo"),noinit)) sn; /* { dg-warning "ignoring attribute 'noinit' because it conflicts with attribute 'section'" } */
+int __attribute__((section(".data.foo"),persistent)) sp = 20; /* { dg-warning "ignoring attribute 'persistent' because it conflicts with attribute 'section'" } */
+int __attribute__((section(".data.foo"),lower)) sl = 2; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'section'" } */
+int __attribute__((section(".data.foo"),upper)) su = 20; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'section'" } */
+int __attribute__((section(".data.foo"),either)) se = 2; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'section'" } */
diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-4.c b/gcc/testsuite/gcc.target/msp430/function-attributes-4.c
index 07d13c95ff1..86224cc1436 100644
--- a/gcc/testsuite/gcc.target/msp430/function-attributes-4.c
+++ b/gcc/testsuite/gcc.target/msp430/function-attributes-4.c
@@ -1,6 +1,9 @@
 /* { dg-do compile } */
 /* Check that the foo interrupt vectors aren't actually removed.  */
 /* { dg-final { scan-assembler-times "__interrupt_vector_foo" 2 } } */
+/* Check that the out-of-range interrupt vectors aren't actually removed.  */
+/* { dg-final { scan-assembler "__interrupt_vector_65" } } */
+/* { dg-final { scan-assembler "__interrupt_vector_100" } } */
 
 /* Check that warnings are emitted when attributes are used incorrectly and
    that attributes are interpreted correctly whether leading and trailing
@@ -8,62 +11,62 @@
 
 void __attribute__((__naked__,__reentrant__))
 fn1(void)
-{ /* { dg-warning "naked functions cannot be reentrant" } */
+{ /* { dg-warning "ignoring attribute 'reentrant' because it conflicts with attribute 'naked'" } */
 }
 
 void __attribute__((naked,reentrant))
 fn2(void)
-{ /* { dg-warning "naked functions cannot be reentrant" } */
+{ /* { dg-warning "ignoring attribute 'reentrant' because it conflicts with attribute 'naked'" } */
 }
 
 void __attribute__((__reentrant__,__naked__))
 fn3(void)
-{ /* { dg-warning "reentrant functions cannot be naked" } */
+{ /* { dg-warning "ignoring attribute 'naked' because it conflicts with attribute 'reentrant'" } */
 }
 
 void __attribute__((reentrant,naked))
 fn4(void)
-{ /* { dg-warning "reentrant functions cannot be naked" } */
+{ /* { dg-warning "ignoring attribute 'naked' because it conflicts with attribute 'reentrant'" } */
 }
 
 void __attribute__((__critical__,__reentrant__))
 fn5(void)
-{ /* { dg-warning "critical functions cannot be reentrant" } */
+{ /* { dg-warning "ignoring attribute 'reentrant' because it conflicts with attribute 'critical'" } */
 }
 
 void __attribute__((critical,reentrant))
 fn6(void)
-{ /* { dg-warning "critical functions cannot be reentrant" } */
+{ /* { dg-warning "ignoring attribute 'reentrant' because it conflicts with attribute 'critical'" } */
 }
 
 void __attribute__((__reentrant__,__critical__))
 fn7(void)
-{ /* { dg-warning "reentrant functions cannot be critical" } */
+{ /* { dg-warning "ignoring attribute 'critical' because it conflicts with attribute 'reentrant'" } */
 }
 
 void __attribute__((reentrant,critical))
 fn8(void)
-{ /* { dg-warning "reentrant functions cannot be critical" } */
+{ /* { dg-warning "ignoring attribute 'critical' because it conflicts with attribute 'reentrant'" } */
 }
 
 void __attribute__((__critical__,__naked__))
 fn9(void)
-{ /* { dg-warning "critical functions cannot be naked" } */
+{ /* { dg-warning "ignoring attribute 'naked' because it conflicts with attribute 'critical'" } */
 }
 
 void __attribute__((critical,naked))
 fn10(void)
-{ /* { dg-warning "critical functions cannot be naked" } */
+{ /* { dg-warning "ignoring attribute 'naked' because it conflicts with attribute 'critical'" } */
 }
 
 void __attribute__((__naked__,__critical__))
 fn11(void)
-{ /* { dg-warning "naked functions cannot be critical" } */
+{ /* { dg-warning "ignoring attribute 'critical' because it conflicts with attribute 'naked'" } */
 }
 
 void __attribute__((naked,critical))
 fn12(void)
-{ /* { dg-warning "naked functions cannot be critical" } */
+{ /* { dg-warning "ignoring attribute 'critical' because it conflicts with attribute 'naked'" } */
 }
 
 int __attribute__((interrupt))
diff --git a/gcc/testsuite/gcc.target/msp430/region-attribute-misuse.c b/gcc/testsuite/gcc.target/msp430/region-attribute-misuse.c
index fe4617b917c..a108e274123 100644
--- a/gcc/testsuite/gcc.target/msp430/region-attribute-misuse.c
+++ b/gcc/testsuite/gcc.target/msp430/region-attribute-misuse.c
@@ -5,9 +5,9 @@
 /* { dg-final { scan-assembler ".section.*lower.data" } } */
 /* { dg-final { scan-assembler ".section.*either.data" } } */
 
-int __attribute__((upper)) upper_bss; /* { dg-warning "'upper' attribute ignored. large memory model .'-mlarge'. is required" } */
-int __attribute__((lower)) lower_bss; /* { dg-warning "'lower' attribute ignored. large memory model .'-mlarge'. is required" } */
-int __attribute__((either)) either_bss; /* { dg-warning "'either' attribute ignored. large memory model .'-mlarge'. is required" } */
+int __attribute__((upper)) upper_bss; /* { dg-warning "'upper' attribute ignored.  Large memory model .'-mlarge'. is required" } */
+int __attribute__((lower)) lower_bss; /* { dg-warning "'lower' attribute ignored.  Large memory model .'-mlarge'. is required" } */
+int __attribute__((either)) either_bss; /* { dg-warning "'either' attribute ignored.  Large memory model .'-mlarge'. is required" } */
 
 /* Verify that even without -mlarge, objects can still be placed in
    upper/lower/either regions manually.  */
-- 
2.17.1

[PATCH 3/3][MSP430] Use default_elf_select_section to select sections for data where possible

[Jozef Lawrynowicz]

[-- Attachment #1: Type: text/plain, Size: 353 bytes --]

With the "noinit" attribute now handled generically, direct assignment of
data with the "noinit" attribute to the ".noinit" attribute can be removed from
the msp430 back end, and default_elf_select_section can be used instead.

default_elf_select_section can also be used for SECCAT_RODATA_MERGE_* sections,
to enable the linker to merge constant data.

[-- Attachment #2: 0003-MSP430-Use-default_elf_select_section-to-determine-s.patch --]
[-- Type: text/x-patch, Size: 5518 bytes --]

From 2d2bc7f11b7d5bfc918351a5963b041f69aac673 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Wed, 28 Aug 2019 17:54:22 +0100
Subject: [PATCH 3/3] MSP430: Use default_elf_select_section to determine
 sections for data where possible

gcc/ChangeLog:

2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config/msp430/msp430.c (msp430_init_sections): Remove handling of the
	noinit section.
	(msp430_select_section): Handle decls with the "noinit" attribute with
	default_elf_select_section.
	Handle SECCAT_RODATA_MERGE_* section types with
	default_elf_select_section.
	Add comments about handling of unsupported section types.
	(msp430_section_type_flags): Remove handling of the noinit section.

---
 gcc/config/msp430/msp430.c | 81 +++++++++++++++++++++++---------------
 1 file changed, 50 insertions(+), 31 deletions(-)

diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index 1b2e9683a94..d409ea15df2 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -1785,7 +1785,6 @@ gen_prefix (tree decl)
   return NULL;
 }
 
-static section * noinit_section;
 static section * persist_section;
 
 #undef  TARGET_ASM_INIT_SECTIONS
@@ -1794,8 +1793,6 @@ static section * persist_section;
 static void
 msp430_init_sections (void)
 {
-  noinit_section = get_unnamed_section (0, output_section_asm_op,
-					".section .noinit,\"aw\"");
   persist_section = get_unnamed_section (0, output_section_asm_op,
 					 ".section .persistent,\"aw\"");
 }
@@ -1806,6 +1803,10 @@ msp430_init_sections (void)
 static section *
 msp430_select_section (tree decl, int reloc, unsigned HOST_WIDE_INT align)
 {
+  const char * prefix;
+  const char * sec_name;
+  const char * base_sec_name;
+
   gcc_assert (decl != NULL_TREE);
 
   if (TREE_CODE (decl) == STRING_CST
@@ -1821,51 +1822,71 @@ 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);
 
-  const char * prefix = gen_prefix (decl);
-  if (prefix == NULL)
-    {
-      if (TREE_CODE (decl) == FUNCTION_DECL)
-	return text_section;
-      /* FIXME: ATTR_NOINIT is handled generically in
-	 default_elf_select_section.  */
-      else if (has_attr (ATTR_NOINIT, decl))
-	return noinit_section;
-      else if (has_attr (ATTR_PERSIST, decl))
-	return persist_section;
-      else
-	return default_select_section (decl, reloc, align);
-    }
+  if (has_attr (ATTR_PERSIST, decl))
+    return persist_section;
+
+  /* ATTR_NOINIT is handled generically.  */
+  if (has_attr (ATTR_NOINIT, decl))
+    return default_elf_select_section (decl, reloc, align);
+
+  prefix = gen_prefix (decl);
 
-  const char * sec;
   switch (categorize_decl_for_section (decl, reloc))
     {
-    case SECCAT_TEXT:   sec = ".text";   break;
-    case SECCAT_DATA:   sec = ".data";   break;
-    case SECCAT_BSS:    sec = ".bss";    break;
-    case SECCAT_RODATA: sec = ".rodata"; break;
+    case SECCAT_TEXT:
+      if (!prefix)
+	return text_section;
+      base_sec_name = ".text";
+      break;
+    case SECCAT_DATA:
+      if (!prefix)
+	return data_section;
+      base_sec_name = ".data";
+      break;
+    case SECCAT_BSS:
+      if (!prefix)
+	return bss_section;
+      base_sec_name = ".bss";
+      break;
+    case SECCAT_RODATA:
+      if (!prefix)
+	return readonly_data_section;
+      base_sec_name = ".rodata";
+      break;
 
+    /* Enable merging of constant data by the GNU linker using
+       default_elf_select_section and therefore enabling creation of
+       sections with the SHF_MERGE flag.  */
     case SECCAT_RODATA_MERGE_STR:
     case SECCAT_RODATA_MERGE_STR_INIT:
     case SECCAT_RODATA_MERGE_CONST:
+      return default_elf_select_section (decl, reloc, align);
+
+    /* The sections listed below are are not supported for MSP430.
+       They should not be generated, but in case they are, we use
+       default_select_section so they get placed in sections
+       the msp430 assembler and linker understand.  */
+    /* "small data" sections are not supported.  */
     case SECCAT_SRODATA:
-    case SECCAT_DATA_REL:
-    case SECCAT_DATA_REL_LOCAL:
-    case SECCAT_DATA_REL_RO:
-    case SECCAT_DATA_REL_RO_LOCAL:
     case SECCAT_SDATA:
     case SECCAT_SBSS:
+    /* Thread-local storage (TLS) is not supported.  */
     case SECCAT_TDATA:
     case SECCAT_TBSS:
+    /* Sections used by a dynamic linker are not supported.  */
+    case SECCAT_DATA_REL:
+    case SECCAT_DATA_REL_LOCAL:
+    case SECCAT_DATA_REL_RO:
+    case SECCAT_DATA_REL_RO_LOCAL:
       return default_select_section (decl, reloc, align);
 
     default:
       gcc_unreachable ();
     }
 
-  const char * dec_name = DECL_SECTION_NAME (decl);
-  char * name = ACONCAT ((prefix, sec, dec_name, NULL));
+  sec_name = ACONCAT ((prefix, base_sec_name, DECL_SECTION_NAME (decl), NULL));
 
-  return get_named_section (decl, name, 0);
+  return get_named_section (decl, sec_name, 0);
 }
 
 #undef  TARGET_ASM_FUNCTION_SECTION
@@ -1901,8 +1922,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, ".noinit") == 0)
-    return SECTION_WRITE | SECTION_BSS | SECTION_NOTYPE;
   else if (strcmp (name, ".persistent") == 0)
     return SECTION_WRITE | SECTION_NOTYPE;
 
-- 
2.17.1

Re: [PATCH 1/3] Implement TARGET_HANDLE_GENERIC_ATTRIBUTE

[Jeff Law]

On 8/30/19 4:09 AM, Jozef Lawrynowicz wrote:
> The attached patch adds a new target hook "TARGET_HANDLE_GENERIC_ATTRIBUTE"
> which enables a back end to perform additional processing of an attribute that
> is normally handled by a front end.
> 
> So far only the "section" and "noinit" attribute make use of this hook, as the
> msp430 back end requires additional attribute conflict checking to be performed
> on these generic attributes.
> 
> 
> 0001-Implement-TARGET_HANDLE_GENERIC_ATTRIBUTE.patch
> 
> From e693da709114df378e2ea8b1d3729b105c99a495 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> Date: Wed, 28 Aug 2019 14:09:20 +0100
> Subject: [PATCH 1/3] Implement TARGET_HANDLE_GENERIC_ATTRIBUTE
> 
> gcc/ChangeLog:
> 
> 2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* config/msp430/msp430.c (TARGET_HANDLE_GENERIC_ATTRIBUTE): Define.
> 	(msp430_handle_generic_attribute): New function.
> 	* doc/tm.texi: Regenerate.
> 	* doc/tm.texi.in: Add TARGET_HANDLE_GENERIC_ATTRIBUTE.
> 	* hooks.c (hook_tree_treeptr_tree_tree_int_boolptr_null): New.
> 	* hooks.h (hook_tree_treeptr_tree_tree_int_boolptr_null): New.
> 	* target.def: Define new hook TARGET_HANDLE_GENERIC_ATTRIBUTE.
> 
> gcc/c-family/ChangeLog:
> 
> 2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* c-attribs.c (handle_section_attribute): Call the
> 	handle_generic_attribute target hook after performing target
> 	independent processing.
> 	(handle_noinit_attribute): Likewise.
Just a nit.  In a couple places in c-attribs.c you have:

> +  if (!(* no_add_attrs))

Drop the whitespace between the * and no_add_attrs.

OK with that change.

jeff

Re: [PATCH 2/3][MSP430] Setup exclusion tables for function and data attributes

[Jeff Law]

On 8/30/19 4:11 AM, Jozef Lawrynowicz wrote:
> The attached patch removes hard-coded warnings from msp430 attribute handlers,
> and replaces them with exclusion rules specified in the attribute_spec table.
> 
> Where msp430 attributes conflict with generic attributes, hard-coded warnings
> are still necessary.
> 
> 
> 0002-MSP430-Setup-exclusion-tables-for-function-and-data-.patch
> 
> From c6def571a47df5394ca9f5c5c4918f0ffcf97375 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> Date: Wed, 28 Aug 2019 17:44:16 +0100
> Subject: [PATCH 2/3] MSP430: Setup exclusion tables for function and data
>  attributes
> 
> gcc/ChangeLog:
> 
> 2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* config/msp430/msp430.c (msp430_attr): Remove warnings about
> 	conflicting msp430-specific attributes.
> 	(msp430_section_attr): Likewise.
> 	Add warnings about conflicts with generic "noinit" and "section"
> 	attributes.
> 	Fix grammar in -mlarge error message.
> 	(msp430_data_attr): Rename to msp430_persist_attr.
> 	Add warnings about conflicts with generic "noinit" and "section"
> 	attributes.
> 	Add warning for when variable is not initialized.
> 	Chain conditionals which prevent the attribute being added.
> 	(ATTR_EXCL): New helper.
> 	(attr_reent_exclusions): New exclusion table.
> 	(attr_naked_exclusions): Likewise.
> 	(attr_crit_exclusions): Likewise.
> 	(attr_lower_exclusions): Likewise.
> 	(attr_upper_exclusions): Likewise.
> 	(attr_either_exclusions): Likewise.
> 	(attr_persist_exclusions): Likewise.
> 	(msp430_attribute_table): Update with exclusion rules.
> 	(msp430_output_aligned_decl_common): Don't output common symbol if decl
> 	has a section.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* gcc.target/msp430/data-attributes-2.c: New test.
> 	* gcc.target/msp430/function-attributes-4.c: Update dg-warning
> 	strings.
> 	* gcc.target/msp430/region-attribute-misuse.c: Likewise.
> 
OK
jeff

Re: [PATCH 3/3][MSP430] Use default_elf_select_section to select sections for data where possible

[Jeff Law]

On 8/30/19 4:14 AM, Jozef Lawrynowicz wrote:
> With the "noinit" attribute now handled generically, direct assignment of
> data with the "noinit" attribute to the ".noinit" attribute can be removed from
> the msp430 back end, and default_elf_select_section can be used instead.
> 
> default_elf_select_section can also be used for SECCAT_RODATA_MERGE_* sections,
> to enable the linker to merge constant data.
> 
> 
> 0003-MSP430-Use-default_elf_select_section-to-determine-s.patch
> 
> From 2d2bc7f11b7d5bfc918351a5963b041f69aac673 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> Date: Wed, 28 Aug 2019 17:54:22 +0100
> Subject: [PATCH 3/3] MSP430: Use default_elf_select_section to determine
>  sections for data where possible
> 
> gcc/ChangeLog:
> 
> 2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* config/msp430/msp430.c (msp430_init_sections): Remove handling of the
> 	noinit section.
> 	(msp430_select_section): Handle decls with the "noinit" attribute with
> 	default_elf_select_section.
> 	Handle SECCAT_RODATA_MERGE_* section types with
> 	default_elf_select_section.
> 	Add comments about handling of unsupported section types.
> 	(msp430_section_type_flags): Remove handling of the noinit section.
And OK as well.

jeff

Re: [PATCH 1/3] Implement TARGET_HANDLE_GENERIC_ATTRIBUTE

[Jozef Lawrynowicz]

On Tue, 3 Sep 2019 13:37:57 -0600
Jeff Law <law@redhat.com> wrote:

> On 8/30/19 4:09 AM, Jozef Lawrynowicz wrote:
> > The attached patch adds a new target hook "TARGET_HANDLE_GENERIC_ATTRIBUTE"
> > which enables a back end to perform additional processing of an attribute that
> > is normally handled by a front end.
> > 
> > So far only the "section" and "noinit" attribute make use of this hook, as the
> > msp430 back end requires additional attribute conflict checking to be performed
> > on these generic attributes.
> > 
> > 
> > 0001-Implement-TARGET_HANDLE_GENERIC_ATTRIBUTE.patch
> > 
> > From e693da709114df378e2ea8b1d3729b105c99a495 Mon Sep 17 00:00:00 2001
> > From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> > Date: Wed, 28 Aug 2019 14:09:20 +0100
> > Subject: [PATCH 1/3] Implement TARGET_HANDLE_GENERIC_ATTRIBUTE
> > 
> > gcc/ChangeLog:
> > 
> > 2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> > 
> > 	* config/msp430/msp430.c (TARGET_HANDLE_GENERIC_ATTRIBUTE): Define.
> > 	(msp430_handle_generic_attribute): New function.
> > 	* doc/tm.texi: Regenerate.
> > 	* doc/tm.texi.in: Add TARGET_HANDLE_GENERIC_ATTRIBUTE.
> > 	* hooks.c (hook_tree_treeptr_tree_tree_int_boolptr_null): New.
> > 	* hooks.h (hook_tree_treeptr_tree_tree_int_boolptr_null): New.
> > 	* target.def: Define new hook TARGET_HANDLE_GENERIC_ATTRIBUTE.
> > 
> > gcc/c-family/ChangeLog:
> > 
> > 2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> > 
> > 	* c-attribs.c (handle_section_attribute): Call the
> > 	handle_generic_attribute target hook after performing target
> > 	independent processing.
> > 	(handle_noinit_attribute): Likewise.  
> Just a nit.  In a couple places in c-attribs.c you have:
> 
> > +  if (!(* no_add_attrs))  
> 
> Drop the whitespace between the * and no_add_attrs.
> 
> OK with that change.

Thanks, I fixed that and other instances of "* <varname>" in the patches
before applying. Seems to be a common style slip-up in the msp430 backend.

Jozef

> 
> jeff

Re: [PATCH 1/3] Implement TARGET_HANDLE_GENERIC_ATTRIBUTE

[Jeff Law]

On 9/3/19 3:00 PM, Jozef Lawrynowicz wrote:
> On Tue, 3 Sep 2019 13:37:57 -0600
> Jeff Law <law@redhat.com> wrote:
> 
>> On 8/30/19 4:09 AM, Jozef Lawrynowicz wrote:
>>> The attached patch adds a new target hook "TARGET_HANDLE_GENERIC_ATTRIBUTE"
>>> which enables a back end to perform additional processing of an attribute that
>>> is normally handled by a front end.
>>>
>>> So far only the "section" and "noinit" attribute make use of this hook, as the
>>> msp430 back end requires additional attribute conflict checking to be performed
>>> on these generic attributes.
>>>
>>>
>>> 0001-Implement-TARGET_HANDLE_GENERIC_ATTRIBUTE.patch
>>>
>>> From e693da709114df378e2ea8b1d3729b105c99a495 Mon Sep 17 00:00:00 2001
>>> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
>>> Date: Wed, 28 Aug 2019 14:09:20 +0100
>>> Subject: [PATCH 1/3] Implement TARGET_HANDLE_GENERIC_ATTRIBUTE
>>>
>>> gcc/ChangeLog:
>>>
>>> 2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
>>>
>>> 	* config/msp430/msp430.c (TARGET_HANDLE_GENERIC_ATTRIBUTE): Define.
>>> 	(msp430_handle_generic_attribute): New function.
>>> 	* doc/tm.texi: Regenerate.
>>> 	* doc/tm.texi.in: Add TARGET_HANDLE_GENERIC_ATTRIBUTE.
>>> 	* hooks.c (hook_tree_treeptr_tree_tree_int_boolptr_null): New.
>>> 	* hooks.h (hook_tree_treeptr_tree_tree_int_boolptr_null): New.
>>> 	* target.def: Define new hook TARGET_HANDLE_GENERIC_ATTRIBUTE.
>>>
>>> gcc/c-family/ChangeLog:
>>>
>>> 2019-08-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
>>>
>>> 	* c-attribs.c (handle_section_attribute): Call the
>>> 	handle_generic_attribute target hook after performing target
>>> 	independent processing.
>>> 	(handle_noinit_attribute): Likewise.  
>> Just a nit.  In a couple places in c-attribs.c you have:
>>
>>> +  if (!(* no_add_attrs))  
>>
>> Drop the whitespace between the * and no_add_attrs.
>>
>> OK with that change.
> 
> Thanks, I fixed that and other instances of "* <varname>" in the patches
> before applying. Seems to be a common style slip-up in the msp430 backend.
Yea, saw it's relatively common in the msp430 target files, given how
often it shows up in there I didn't call it out in the msp target files
in your patch, just in the generic bits.

Jeff