public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RE: [PATCH] RL78 new "vector" function attribute
@ 2018-01-29 13:36 Sebastian Perta
  2018-01-29 22:01 ` DJ Delorie
  0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Perta @ 2018-01-29 13:36 UTC (permalink / raw)
  To: gcc-patches

Hello,

The below patch adds a new vector attribute for RL78, it is basically a copy
past of what DJ has done for RX a while ago:
https://gcc.gnu.org/ml/gcc-patches/2014-05/msg02387.html

The patch adds also a test case and updates extend.texi with the new
attribute.

Regression test is OK, tested with the following command:
make -k check-gcc RUNTESTFLAGS=--target_board=rl78-sim

As you can see I had to duplicate some of the code from RX for this on the
other hand if I try to make this attribute generic 
It will become available for all targets which I'm not sure if it is
desirable. Please let me know if this is OK to checkin or should I 
look further into avoiding the code duplication.

Best Regards,
Sebastian

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 257142)
+++ ChangeLog	(working copy)
@@ -1,3 +1,14 @@
+2018-01-29  Sebastian Perta  <sebastian.perta@renesas.com>
+
+	* config/rl78/rl78.c (add_vector_labels): New function.
+	* config/rl78/rl78.c (rl78_handle_vector_attribute): New function.
+	* config/rl78/rl78.c (rl78_start_function): Call add_vector_labels.
+	* config/rl78/rl78.c (rl78_handle_func_attribute): Removed the
assert 
+	which checks that no arguments are passed.
+	* config/rl78/rl78.c (rl78_attribute_table): Add "vector" attribute.
+	* testsuite/gcc.target/rl78/test_auto_vector.c: New file.
+	* doc/extend.texi: documentation for the new attribute
+	
 2018-01-29  Richard Biener  <rguenther@suse.de>
 
 	PR tree-optimization/84057




Index: config/rl78/rl78.c
===================================================================
--- config/rl78/rl78.c	(revision 257142)
+++ config/rl78/rl78.c	(working copy)
@@ -809,7 +809,6 @@
 			    bool * no_add_attrs)
 {
   gcc_assert (DECL_P (* node));
-  gcc_assert (args == NULL_TREE);
 
   if (TREE_CODE (* node) != FUNCTION_DECL)
     {
@@ -868,6 +867,28 @@
   return NULL_TREE;
 }
 
+/* Check "vector" attribute.  */
+
+static tree
+rl78_handle_vector_attribute (tree * node,
+			    tree   name,
+			    tree   args,
+			    int    flags ATTRIBUTE_UNUSED,
+			    bool * no_add_attrs)
+{
+  gcc_assert (DECL_P (* node));
+  gcc_assert (args != NULL_TREE);
+
+  if (TREE_CODE (* node) != FUNCTION_DECL)
+    {
+      warning (OPT_Wattributes, "%qE attribute only applies to functions",
+	       name);
+      * no_add_attrs = true;
+    }
+
+  return NULL_TREE;
+}
+
 #undef  TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLE		rl78_attribute_table
 
@@ -876,7 +897,7 @@
 {
   /* Name, min_len, max_len, decl_req, type_req, fn_type_req,
      affects_type_identity, handler, exclude.  */
-  { "interrupt",      0, 0, true, false, false, false,
+  { "interrupt",      0, -1, true, false, false, false,
     rl78_handle_func_attribute, NULL },
   { "brk_interrupt",  0, 0, true, false, false, false,
     rl78_handle_func_attribute, NULL },
@@ -884,6 +905,8 @@
     rl78_handle_naked_attribute, NULL },
   { "saddr",          0, 0, true, false, false, false,
     rl78_handle_saddr_attribute, NULL },
+  { "vector",         1, -1, true, false, false, 
+	rl78_handle_vector_attribute, false },
   { NULL,             0, 0, false, false, false, false, NULL, NULL }
 };
 
@@ -1583,6 +1606,62 @@
 #undef  TARGET_ASM_FUNCTION_PROLOGUE
 #define TARGET_ASM_FUNCTION_PROLOGUE	rl78_start_function
 
+static void
+add_vector_labels (FILE *file, const char *aname)
+{
+  tree vec_attr;
+  tree val_attr;
+  const char *vname = "vect";
+  const char *s;
+  int vnum;
+
+  /* This node is for the vector/interrupt tag itself */
+  vec_attr = lookup_attribute (aname, DECL_ATTRIBUTES
(current_function_decl));
+  if (!vec_attr)
+    return;
+
+  /* Now point it at the first argument */
+  vec_attr = TREE_VALUE (vec_attr);
+
+  /* Iterate through the arguments.  */
+  while (vec_attr)
+    {
+      val_attr = TREE_VALUE (vec_attr);
+      switch (TREE_CODE (val_attr))
+	{
+	case STRING_CST:
+	  s = TREE_STRING_POINTER (val_attr);
+	  goto string_id_common;
+
+	case IDENTIFIER_NODE:
+	  s = IDENTIFIER_POINTER (val_attr);
+
+	string_id_common:
+	  if (strcmp (s, "$default") == 0)
+	    {
+	      fprintf (file, "\t.global\t$tableentry$default$%s\n", vname);
+	      fprintf (file, "$tableentry$default$%s:\n", vname);
+	    }
+	  else
+	    vname = s;
+	  break;
+
+	case INTEGER_CST:
+	  vnum = TREE_INT_CST_LOW (val_attr);
+
+	  fprintf (file, "\t.global\t$tableentry$%d$%s\n", vnum, vname);
+	  fprintf (file, "$tableentry$%d$%s:\n", vnum, vname);
+	  break;
+
+	default:
+	  ;
+	}
+
+      vec_attr = TREE_CHAIN (vec_attr);
+    }
+
+}
+
 /* We don't use this to actually emit the function prologue.  We use
    this to insert a comment in the asm file describing the
    function.  */
@@ -1590,6 +1669,9 @@
 rl78_start_function (FILE *file)
 {
   int i;
+  
+  add_vector_labels (file, "interrupt");
+  add_vector_labels (file, "vector");
 
   if (cfun->machine->framesize == 0)
     return;
Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 257142)
+++ doc/extend.texi	(working copy)
@@ -5150,6 +5150,28 @@
 handlers intended to be used with the @code{BRK} opcode (i.e.@: those
 that must end with @code{RETB} instead of @code{RETI}).
 
+On RL78, you may specify one or more vector numbers as arguments
+to the attribute, as well as naming an alternate table name.
+Parameters are handled sequentially, so one handler can be assigned to
+multiple entries in multiple tables.  One may also pass the magic
+string @code{"$default"} which causes the function to be used for any
+unfilled slots in the current table.
+
+This example shows a simple assignment of a function to one vector in
+the default table (note that preprocessor macros may be used for
+chip-specific symbolic vector names):
+@smallexample
+void __attribute__ ((interrupt (5))) txd1_handler ();
+@end smallexample
+
+This example assigns a function to two slots in the default table
+(using preprocessor macros defined elsewhere) and makes it the default
+for the @code{dct} table:
+@smallexample
+void __attribute__ ((interrupt (RXD1_VECT,RXD2_VECT,"dct","$default")))
+	txd1_handler ();
+@end smallexample
+
 @item naked
 @cindex @code{naked} function attribute, RL78
 This attribute allows the compiler to construct the
Index: testsuite/gcc.target/rl78/test_auto_vector.c
===================================================================
--- testsuite/gcc.target/rl78/test_auto_vector.c	(nonexistent)
+++ testsuite/gcc.target/rl78/test_auto_vector.c	(working copy)
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+
+void __attribute__ ((interrupt (5))) interrupt_5_handler ();
+
+void interrupt_5_handler ()
+{
+}
+
+void __attribute__ ((vector (4))) interrupt_4_handler ();
+
+void interrupt_4_handler ()
+{
+}
+
+void __attribute__ ((interrupt)) interrupt_handler ();
+
+void interrupt_handler ()
+{
+}
+
+/* { dg-final { scan-assembler "tableentry" } } */


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] RL78 new "vector" function attribute
  2018-01-29 13:36 [PATCH] RL78 new "vector" function attribute Sebastian Perta
@ 2018-01-29 22:01 ` DJ Delorie
  2018-02-06 13:39   ` Sebastian Perta
  0 siblings, 1 reply; 6+ messages in thread
From: DJ Delorie @ 2018-01-29 22:01 UTC (permalink / raw)
  To: Sebastian Perta; +Cc: gcc-patches


If the RX and RL78 now share interrupt/vector semantics, can we combine
the docs?  I.e. instead of a new section for RL78, can we change the RX
section to say something like "For RX and RL78..." ?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] RL78 new "vector" function attribute
  2018-01-29 22:01 ` DJ Delorie
@ 2018-02-06 13:39   ` Sebastian Perta
  2018-02-06 22:57     ` DJ Delorie
  0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Perta @ 2018-02-06 13:39 UTC (permalink / raw)
  To: DJ Delorie; +Cc: gcc-patches

Hi DJ,

I've updated the patch (extend.texi) as you suggested.
Please let me know if this is OK to check-in, thank you!

Best Regards,
Sebastian

Index: config/rl78/rl78.c
===================================================================
--- config/rl78/rl78.c(revision 257142)
+++ config/rl78/rl78.c(working copy)
@@ -809,7 +809,6 @@
     bool * no_add_attrs)
 {
   gcc_assert (DECL_P (* node));
-  gcc_assert (args == NULL_TREE);

   if (TREE_CODE (* node) != FUNCTION_DECL)
     {
@@ -868,6 +867,28 @@
   return NULL_TREE;
 }

+/* Check "vector" attribute.  */
+
+static tree
+rl78_handle_vector_attribute (tree * node,
+    tree   name,
+    tree   args,
+    int    flags ATTRIBUTE_UNUSED,
+    bool * no_add_attrs)
+{
+  gcc_assert (DECL_P (* node));
+  gcc_assert (args != NULL_TREE);
+
+  if (TREE_CODE (* node) != FUNCTION_DECL)
+    {
+      warning (OPT_Wattributes, "%qE attribute only applies to functions",
+       name);
+      * no_add_attrs = true;
+    }
+
+  return NULL_TREE;
+}
+
 #undef  TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLErl78_attribute_table

@@ -876,7 +897,7 @@
 {
   /* Name, min_len, max_len, decl_req, type_req, fn_type_req,
      affects_type_identity, handler, exclude.  */
-  { "interrupt",      0, 0, true, false, false, false,
+  { "interrupt",      0, -1, true, false, false, false,
     rl78_handle_func_attribute, NULL },
   { "brk_interrupt",  0, 0, true, false, false, false,
     rl78_handle_func_attribute, NULL },
@@ -884,6 +905,8 @@
     rl78_handle_naked_attribute, NULL },
   { "saddr",          0, 0, true, false, false, false,
     rl78_handle_saddr_attribute, NULL },
+  { "vector",         1, -1, true, false, false,
+rl78_handle_vector_attribute, false },
   { NULL,             0, 0, false, false, false, false, NULL, NULL }
 };

@@ -1583,6 +1606,62 @@
 #undef  TARGET_ASM_FUNCTION_PROLOGUE
 #define TARGET_ASM_FUNCTION_PROLOGUErl78_start_function

+static void
+add_vector_labels (FILE *file, const char *aname)
+{
+  tree vec_attr;
+  tree val_attr;
+  const char *vname = "vect";
+  const char *s;
+  int vnum;
+
+  /* This node is for the vector/interrupt tag itself */
+  vec_attr = lookup_attribute (aname, DECL_ATTRIBUTES (current_function_decl));
+  if (!vec_attr)
+    return;
+
+  /* Now point it at the first argument */
+  vec_attr = TREE_VALUE (vec_attr);
+
+  /* Iterate through the arguments.  */
+  while (vec_attr)
+    {
+      val_attr = TREE_VALUE (vec_attr);
+      switch (TREE_CODE (val_attr))
+{
+case STRING_CST:
+  s = TREE_STRING_POINTER (val_attr);
+  goto string_id_common;
+
+case IDENTIFIER_NODE:
+  s = IDENTIFIER_POINTER (val_attr);
+
+string_id_common:
+  if (strcmp (s, "$default") == 0)
+    {
+      fprintf (file, "\t.global\t$tableentry$default$%s\n", vname);
+      fprintf (file, "$tableentry$default$%s:\n", vname);
+    }
+  else
+    vname = s;
+  break;
+
+case INTEGER_CST:
+  vnum = TREE_INT_CST_LOW (val_attr);
+
+  fprintf (file, "\t.global\t$tableentry$%d$%s\n", vnum, vname);
+  fprintf (file, "$tableentry$%d$%s:\n", vnum, vname);
+  break;
+
+default:
+  ;
+}
+
+      vec_attr = TREE_CHAIN (vec_attr);
+    }
+
+}
+
 /* We don't use this to actually emit the function prologue.  We use
    this to insert a comment in the asm file describing the
    function.  */
@@ -1590,6 +1669,9 @@
 rl78_start_function (FILE *file)
 {
   int i;
+
+  add_vector_labels (file, "interrupt");
+  add_vector_labels (file, "vector");

   if (cfun->machine->framesize == 0)
     return;
Index: doc/extend.texi
===================================================================
--- doc/extend.texi(revision 257142)
+++ doc/extend.texi(working copy)
@@ -5182,7 +5182,7 @@
 function entry and exit sequences suitable for use in an interrupt handler
 when this attribute is present.

-On RX targets, you may specify one or more vector numbers as arguments
+On RX and RL78 targets, you may specify one or more vector numbers as arguments
 to the attribute, as well as naming an alternate table name.
 Parameters are handled sequentially, so one handler can be assigned to
 multiple entries in multiple tables.  One may also pass the magic
Index: testsuite/gcc.target/rl78/test_auto_vector.c
===================================================================
--- testsuite/gcc.target/rl78/test_auto_vector.c(nonexistent)
+++ testsuite/gcc.target/rl78/test_auto_vector.c(working copy)
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+
+void __attribute__ ((interrupt (5))) interrupt_5_handler ();
+
+void interrupt_5_handler ()
+{
+}
+
+void __attribute__ ((vector (4))) interrupt_4_handler ();
+
+void interrupt_4_handler ()
+{
+}
+
+void __attribute__ ((interrupt)) interrupt_handler ();
+
+void interrupt_handler ()
+{
+}
+
+/* { dg-final { scan-assembler "tableentry" } } */

> -----Original Message-----
> From: DJ Delorie [mailto:dj@redhat.com]
> Sent: 29 January 2018 21:11
> To: Sebastian Perta <Sebastian.Perta@renesas.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] RL78 new "vector" function attribute
>
>
> If the RX and RL78 now share interrupt/vector semantics, can we combine
> the docs?  I.e. instead of a new section for RL78, can we change the RX
> section to say something like "For RX and RL78..." ?



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] RL78 new "vector" function attribute
  2018-02-06 13:39   ` Sebastian Perta
@ 2018-02-06 22:57     ` DJ Delorie
  2018-02-12 14:08       ` Sebastian Perta
  0 siblings, 1 reply; 6+ messages in thread
From: DJ Delorie @ 2018-02-06 22:57 UTC (permalink / raw)
  To: Sebastian Perta; +Cc: gcc-patches


Sebastian Perta <Sebastian.Perta@renesas.com> writes:
> I've updated the patch (extend.texi) as you suggested.
> Please let me know if this is OK to check-in, thank you!

Looks OK to me, but wait a day or two for a docs person to comment on...

> -On RX targets, you may specify one or more vector numbers as arguments
> +On RX and RL78 targets, you may specify one or more vector numbers as arguments

...if the new line is too long and if a paragraph reformat is warranted.

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] RL78 new "vector" function attribute
  2018-02-06 22:57     ` DJ Delorie
@ 2018-02-12 14:08       ` Sebastian Perta
  2018-02-12 23:44         ` DJ Delorie
  0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Perta @ 2018-02-12 14:08 UTC (permalink / raw)
  To: 'DJ Delorie'; +Cc: gcc-patches, 'Jakub Jelinek'

Hi DJ,

>>Looks OK to me, but wait a day or two for a docs person to comment on...
6 days no comments so far, can I check in now?

>>if the new line is too long
There are many other lines which have the same length or are even longer
this is why I let it as it is.

Also based on comments from Jakub (on a different patch) I corrected the
Changelog entry for this patch (see below). Is this OK?

Best Regards,
Sebastian

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 257588)
+++ ChangeLog	(working copy)
@@ -1,3 +1,13 @@
+2018-02-12  Sebastian Perta  <sebastian.perta@renesas.com>
+
+	* config/rl78/rl78.c (add_vector_labels): New function.
+	* config/rl78/rl78.c (rl78_handle_vector_attribute): New function.
+	* config/rl78/rl78.c (rl78_start_function): Call add_vector_labels.
+	* config/rl78/rl78.c (rl78_handle_func_attribute): Removed the
assert 
+	which checks that no arguments are passed.
+	* config/rl78/rl78.c (rl78_attribute_table): Add "vector" attribute.
+	* doc/extend.texi: Documentation for the new attribute.
+
 2018-02-12  Richard Biener  <rguenther@suse.de>
 
 	PR tree-optimization/84037
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog	(revision 257588)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2018-02-12  Sebastian Perta  <sebastian.perta@renesas.com>
+
+	* gcc.target/rl78/test_auto_vector.c: New test.
+
 2018-02-12  Tamar Christina  <tamar.christina@arm.com>
 
 	PR target/82641



> -----Original Message-----
> From: DJ Delorie [mailto:dj@redhat.com]
> Sent: 06 February 2018 22:57
> To: Sebastian Perta <Sebastian.Perta@renesas.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] RL78 new "vector" function attribute
> 
> 
> Sebastian Perta <Sebastian.Perta@renesas.com> writes:
> > I've updated the patch (extend.texi) as you suggested.
> > Please let me know if this is OK to check-in, thank you!
> 
> Looks OK to me, but wait a day or two for a docs person to comment on...
> 
> > -On RX targets, you may specify one or more vector numbers as arguments
> > +On RX and RL78 targets, you may specify one or more vector numbers as
> arguments
> 
> ...if the new line is too long and if a paragraph reformat is warranted.
> 
> Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] RL78 new "vector" function attribute
  2018-02-12 14:08       ` Sebastian Perta
@ 2018-02-12 23:44         ` DJ Delorie
  0 siblings, 0 replies; 6+ messages in thread
From: DJ Delorie @ 2018-02-12 23:44 UTC (permalink / raw)
  To: Sebastian Perta; +Cc: gcc-patches, jakub

"Sebastian Perta" <sebastian.perta@renesas.com> writes:
>>>Looks OK to me, but wait a day or two for a docs person to comment on...
> 6 days no comments so far, can I check in now?

Yup, go ahead.

>>>if the new line is too long
> There are many other lines which have the same length or are even longer
> this is why I let it as it is.

Ok.

> Also based on comments from Jakub (on a different patch) I corrected the
> Changelog entry for this patch (see below). Is this OK?

Yup.  Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-02-12 23:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-29 13:36 [PATCH] RL78 new "vector" function attribute Sebastian Perta
2018-01-29 22:01 ` DJ Delorie
2018-02-06 13:39   ` Sebastian Perta
2018-02-06 22:57     ` DJ Delorie
2018-02-12 14:08       ` Sebastian Perta
2018-02-12 23:44         ` DJ Delorie

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