From 5c56509cfa0f285549eceefb457dc59134d30d0e Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Thu, 18 Oct 2018 22:16:22 +0100
Subject: [PATCH] Extend MSP430 data attribute handling

2018-10-19  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* gcc/config/msp430/msp430.c (msp430_data_attr): Warn if data marked
	with the persistent attribute is not intialized.
	(gen_prefix): Do not add prefixes to data to be placed in the 
	.noinit or .persistent section.
	(msp430_section_type_flags): Use global const string to reference
	.noinit and .persistent sections.

	gcc/testsuite
	* gcc.target/msp430/data-attributes.c: Extend test to check behaviour
	of static noinit/persistent data.
	* gcc.target/msp430/function-attributes-4.c: Extend test to check
	behaviour of noinit/persistent attributes when applied to functions.
	* gcc.target/msp430/attr-critical.c: New test.
	* gcc.target/msp430/attr-naked.c: Likewise.
	* gcc.target/msp430/attr-reentrant.c: Likewise.
	* gcc.target/msp430/attr-wakeup.c: Likewise.
	* gcc.target/msp430/data-attributes-2.c: Likewise.
	* gcc.target/msp430/data-attributes-3.c: Likewise.
---
 gcc/config/msp430/msp430.c                         | 93 ++++++++++++++++------
 gcc/testsuite/gcc.target/msp430/attr-critical.c    | 11 +++
 gcc/testsuite/gcc.target/msp430/attr-naked.c       | 11 +++
 gcc/testsuite/gcc.target/msp430/attr-reentrant.c   | 11 +++
 gcc/testsuite/gcc.target/msp430/attr-wakeup.c      | 19 +++++
 .../gcc.target/msp430/data-attributes-2.c          | 15 ++++
 .../gcc.target/msp430/data-attributes-3.c          | 11 +++
 gcc/testsuite/gcc.target/msp430/data-attributes.c  | 28 ++++---
 .../gcc.target/msp430/function-attributes-4.c      | 10 +++
 9 files changed, 174 insertions(+), 35 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/msp430/attr-critical.c
 create mode 100644 gcc/testsuite/gcc.target/msp430/attr-naked.c
 create mode 100644 gcc/testsuite/gcc.target/msp430/attr-reentrant.c
 create mode 100644 gcc/testsuite/gcc.target/msp430/attr-wakeup.c
 create mode 100644 gcc/testsuite/gcc.target/msp430/data-attributes-2.c
 create mode 100644 gcc/testsuite/gcc.target/msp430/data-attributes-3.c

diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index 7d305b1..73a693e 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -1804,6 +1804,8 @@ const char * const  ATTR_UPPER  = "upper";
 const char * const  ATTR_EITHER = "either";
 const char * const  ATTR_NOINIT = "noinit";
 const char * const  ATTR_PERSIST = "persistent";
+const char * const  SEC_NOINIT = ".noinit";
+const char * const  SEC_PERSIST = ".persistent";
 
 static inline bool
 has_attr (const char * attr, tree decl)
@@ -2037,37 +2039,71 @@ msp430_data_attr (tree * node,
   gcc_assert (DECL_P (* node));
   gcc_assert (args == NULL);
 
-  if (TREE_CODE (* node) != VAR_DECL)
-    message = G_("%qE attribute only applies to variables");
+  /* Variable name to use in warning string, defaults to attribute name.  */
+  tree var = name;
+  tree decl = *node;
 
-  /* 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))
-    message = G_("%qE attribute cannot be applied to variables with specific sections");
+  if (TREE_CODE (decl) != VAR_DECL)
+    {
+      message = G_("%qE attribute only applies to variables");
+      goto fail;
+    }
 
-  if (!message && TREE_NAME_EQ (name, ATTR_PERSIST) && !TREE_STATIC (* node)
-      && !TREE_PUBLIC (* node) && !DECL_EXTERNAL (* node))
-    message = G_("%qE attribute has no effect on automatic variables");
+  if (!(TREE_STATIC (decl) || TREE_PUBLIC (decl) || DECL_EXTERNAL (decl)
+	|| in_lto_p))
+    {
+      message = G_("%qE attribute has no effect on automatic variables");
+      goto fail;
+    }
 
-  /* 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))
-    set_decl_section_name (* node, ".persistent");
+  if ((TREE_NAME_EQ (name, ATTR_PERSIST)
+       && has_attr (ATTR_NOINIT, decl))
+      || (TREE_NAME_EQ (name, ATTR_NOINIT)
+	  && has_attr (ATTR_PERSIST, decl)))
+    {
+      message
+	= G_("variable %qE cannot have both noinit and persistent attributes");
+      var = decl;
+      goto fail;
+    }
 
-  /* 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 (DECL_SECTION_NAME (decl) != NULL)
+    {
+      message = G_("%qE attribute cannot be applied to variables with "
+		   "specific sections");
+      goto fail;
+    }
 
-  if (message)
+  if (TREE_NAME_EQ (name, ATTR_PERSIST))
     {
-      warning (OPT_Wattributes, message, name);
-      * no_add_attrs = true;
+      if (DECL_INITIAL (decl) == NULL
+	  || initializer_zerop (DECL_INITIAL (decl)))
+	{
+	  message = G_("variable %qE was declared persistent and should be "
+		       "explicitly initialized");
+	  var = decl;
+	  goto fail;
+	}
     }
-    
+
+  /* In some cases the front-end will place data before msp430_select_section
+     gets a chance, so just put noinit and persistent variables in the correct
+     section now.  */
+  if (TREE_NAME_EQ (name, ATTR_PERSIST))
+    set_decl_section_name (decl, SEC_PERSIST);
+  else if (TREE_NAME_EQ (name, ATTR_NOINIT))
+    set_decl_section_name (decl, SEC_NOINIT);
+  else
+    /* Only persistent and noinit attributes are supported in this handler.  */
+    gcc_unreachable ();
+
+  goto done;
+
+fail:
+  warning (OPT_Wattributes, message, var);
+  * no_add_attrs = true;
+
+done:
   return NULL_TREE;
 }
 
@@ -2254,6 +2290,11 @@ gen_prefix (tree decl)
   if (! msp430x)
     return NULL;
 
+  /* Do not add prefixes to data to be placed in the .noinit or .persistent
+     section.  */
+  if (has_attr (ATTR_NOINIT, decl) || has_attr (ATTR_PERSIST, decl))
+    return NULL;
+
   if (has_attr (ATTR_UPPER, decl))
     return upper_prefix;
 
@@ -2396,9 +2437,9 @@ 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)
+  else if (strcmp (name, SEC_NOINIT) == 0)
     return SECTION_WRITE | SECTION_BSS | SECTION_NOTYPE;
-  else if (strcmp (name, ".persistent") == 0)
+  else if (strcmp (name, SEC_PERSIST) == 0)
     return SECTION_WRITE | SECTION_NOTYPE;
   
   return default_section_type_flags (decl, name, reloc);
diff --git a/gcc/testsuite/gcc.target/msp430/attr-critical.c b/gcc/testsuite/gcc.target/msp430/attr-critical.c
new file mode 100644
index 0000000..bf9a0d0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/attr-critical.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-final { scan-assembler-times "start of prologue.*PUSH\\s+SR.*DINT.*end of prologue" 1 } } */
+/* { dg-final { scan-assembler-times "start of epilogue.*POP\\s+SR.*RET" 1 } } */
+
+extern int a;
+
+void __attribute__((critical))
+critical_fn(void)
+{
+  while(a);
+}
diff --git a/gcc/testsuite/gcc.target/msp430/attr-naked.c b/gcc/testsuite/gcc.target/msp430/attr-naked.c
new file mode 100644
index 0000000..a12f36e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/attr-naked.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-final { scan-assembler-not "prologue" } } */
+/* { dg-final { scan-assembler-not "epilogue" } } */
+
+extern int a;
+
+void __attribute__((naked))
+naked_fn(void)
+{
+  while(a);
+}
diff --git a/gcc/testsuite/gcc.target/msp430/attr-reentrant.c b/gcc/testsuite/gcc.target/msp430/attr-reentrant.c
new file mode 100644
index 0000000..72e8d55
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/attr-reentrant.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-final { scan-assembler-times "start of prologue.*DINT.*end of prologue" 1 } } */
+/* { dg-final { scan-assembler-times "start of epilogue.*EINT.*RET" 1 } } */
+
+extern int a;
+
+void __attribute__((reentrant))
+reentrant_fn(void)
+{
+  while(a);
+}
diff --git a/gcc/testsuite/gcc.target/msp430/attr-wakeup.c b/gcc/testsuite/gcc.target/msp430/attr-wakeup.c
new file mode 100644
index 0000000..63ac3b4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/attr-wakeup.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-final { scan-assembler-times "BIC.W\\s+#240, \\S+SP" 1 } } */
+
+extern int a;
+
+/* The SR is TOS on entry to an interrupt function. Clear bits 0xF0 (240) in
+   the SR to exit out of low power mode.  */
+void __attribute__((wakeup,interrupt))
+wake_interrupt_fn(void)
+{
+  while(a);
+}
+
+/* wakeup attribute is silently ignored for non-interrupt functions.  */
+void __attribute__((wakeup))
+wake_fn(void)
+{
+  while(a);
+}
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 0000000..5889108
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/data-attributes-2.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+
+int __attribute__((persistent,noinit)) a = 22; /* { dg-warning "variable 'a' cannot have both noinit and persistent attributes" } */
+int __attribute__((noinit,persistent)) b; /* { dg-warning "variable 'b' cannot have both noinit and persistent attributes" } */
+int __attribute__((noinit,section(".foo"))) c; /* { dg-error "section of 'c' conflicts with" } */
+int __attribute__((persistent,section(".foo"))) d = 10; /* { dg-error "section of 'd' conflicts with" } */
+int __attribute__((section(".foo"),noinit)) e; /* { dg-warning "cannot be applied to variables with specific" } */
+int __attribute__((section(".foo"),persistent)) f = 10; /* { dg-warning "cannot be applied to variables with specific" } */
+
+int main (void)
+{
+  int __attribute__((noinit)) la; /* { dg-warning "'noinit' attribute has no effect on automatic variables" } */
+  int __attribute__((persistent)) lb; /* { dg-warning "'persistent' attribute has no effect on automatic variables" } */
+  return la;
+}
diff --git a/gcc/testsuite/gcc.target/msp430/data-attributes-3.c b/gcc/testsuite/gcc.target/msp430/data-attributes-3.c
new file mode 100644
index 0000000..b7b6560
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/data-attributes-3.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+
+int __attribute__((noinit)) a = 5; /* { dg-error "only zero initializers are allowed in section '.noinit'" } */
+int __attribute__((persistent)) b; /* { dg-warning "variable 'b' was declared persistent and should be explicitly initialized" } */
+
+int main (void)
+{
+  static int __attribute__((noinit)) c;
+  return c;
+}
+
diff --git a/gcc/testsuite/gcc.target/msp430/data-attributes.c b/gcc/testsuite/gcc.target/msp430/data-attributes.c
index 10dd171..06e88cb 100644
--- a/gcc/testsuite/gcc.target/msp430/data-attributes.c
+++ b/gcc/testsuite/gcc.target/msp430/data-attributes.c
@@ -10,12 +10,17 @@ extern void exit (int) __attribute__ ((noreturn));
 int a;
 int b = 0;
 int c = 1;
-int __attribute__((noinit)) d;
-int __attribute__((persistent)) e = 2;
+int __attribute__((noinit)) gd;
+int __attribute__((persistent)) ge = 2;
 
 int
 main (void)
 {
+  /* Persistent/noinit static local variables should behave the same as their
+     global counterparts.  */
+  static int __attribute__((noinit)) ld;
+  static int __attribute__((persistent)) le = 2;
+
   /* Make sure that the C startup code has correctly initialised the ordinary variables.  */
   if (a != 0)
     abort ();
@@ -25,26 +30,31 @@ main (void)
      This does not support FLASH, and as a side effect it does not support
      reinitialising initialised data.  Hence we only test b and c if this
      is the first time through this test, or large support has been enabled.  */
-  if (e == 2)
+  if (ge == 2)
 #endif
   if (b != 0 || c != 1)
     abort ();
-  
-  switch (e)
+
+  /* The local and global persistent variables should always have the same
+     value.  */
+  if (ge != le)
+    abort();
+  switch (ge)
     {
     case 2:
       /* First time through - change all the values.  */
-      a = b = c = d = e = 3;
+      a = b = c = gd = ge = ld = le = 3;
       break;
 
     case 3:
-      /* Second time through - make sure that d has not been reset.  */
-      if (d != 3)
+      /* Second time through - make sure that gd and ld have not been reset,
+	 and that they have the same value.  */
+      if (gd != 3 || gd != ld)
 	abort ();
       exit (0);
 
     default:
-      /* Any other value for e is an error.  */
+      /* Any other value for ge is an error.  */
       abort ();
     }
 
diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-4.c b/gcc/testsuite/gcc.target/msp430/function-attributes-4.c
index 07d13c9..4a41282 100644
--- a/gcc/testsuite/gcc.target/msp430/function-attributes-4.c
+++ b/gcc/testsuite/gcc.target/msp430/function-attributes-4.c
@@ -66,6 +66,16 @@ fn12(void)
 { /* { dg-warning "naked functions cannot be critical" } */
 }
 
+void __attribute__((noinit))
+fn13(void)
+{ /* { dg-warning "'noinit' attribute only applies to variables" } */
+}
+
+void __attribute__((persistent))
+fn14(void)
+{ /* { dg-warning "'persistent' attribute only applies to variables" } */
+}
+
 int __attribute__((interrupt))
 isr1 (void)
 { /* { dg-warning "interrupt handlers must be void" } */
-- 
2.7.4

