[1/3] Implement TARGET_HANDLE_GENERIC_ATTRIBUTE

Message ID 20190830110943.685c4c9e@jozef-kubuntu
State New
Headers show
Series
  • MSP430: Improve attribute handling
Related show

Commit Message

Jozef Lawrynowicz Aug. 30, 2019, 10:09 a.m.
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.

Comments

Jeff Law Sept. 3, 2019, 7:37 p.m. | #1
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
Jozef Lawrynowicz Sept. 3, 2019, 9 p.m. | #2
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
Jeff Law Sept. 3, 2019, 9:16 p.m. | #3
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

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.
---
 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