On 30/05/2017 12:50, Nick Clifton wrote:
When I applied this patch to the sources and ran the new test, I encountered
an internal compiler error:
msp430-elf/gcc/xgcc [...] pr78818-auto-warn.c [...]
[...]
gcc/testsuite/gcc.target/msp430/pr78818-auto-warn.c: In function 'main':
gcc/testsuite/gcc.target/msp430/pr78818-auto-warn.c:10:3: internal compiler
error: in get, at cgraph.h:403
0xd30d3b symtab_node::get(tree_node const*)
gcc/current/gcc/cgraph.h:400
0xd30d3b decl_section_name(tree_node const*)
gcc/current/gcc/tree.c:700
0xd9ff22 msp430_data_attr
gcc/current/gcc/config/msp430/msp430.c:1998
It seems that there is a problem with calling the DECL_SECTION_NAME macro on
the
line just before your new code. Are you able to reproduce this problem ?
Hi Nick,
Apologies for the delay in replying.
I have reproduced this issue with current trunk and on the gcc-7-branch,
but it does not reproduce on the gcc-6-branch.
The ICE isn't caused by my patch but the pr78818-auto-warn.c test does
expose it.
I've attached an additional patch to fix the ICE (0001*). The ICE was
caused by a new gcc_checking_assert added in GCC7 that checks that
"symtab_node" has been called on a sane object. Added some additional
check in msp430.c:data_attr that prevent section names being looked up
on variables that can't have a section.
The 0002* patch has been updated as this also caused an ICE when running
the test case for the same reason as above, which was exposed when the
first ICE was fixed.
I have tested these patches on trunk this time (previous testing was
done on performed on gcc-6-branch only) and can confirm there are no
regressions and the new tests build successfully.
Ok for trunk and gcc-7-branch?
Thanks,
Jozef
From 8a41a45f2f771ae540b16ec007ded499a9ed5244 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <joze...@somniumtech.com>
Date: Mon, 5 Jun 2017 18:07:22 +0000
Subject: [PATCH 1/3] MSP430: Check if a variable can have a section before
checking for a section name
2017-06-XX Jozef Lawrynowicz <joze...@somniumtech.com>
gcc/
* config/msp430/msp430.c (msp430_data_attr): Check that it's possible
for a variable to have a section before checking if the section has a
name.
---
gcc/config/msp430/msp430.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index dd53dea..cdd765b 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -1995,8 +1995,10 @@ msp430_data_attr (tree * node,
if (TREE_CODE (* node) != VAR_DECL)
message = "%qE attribute only applies to variables";
- if (DECL_SECTION_NAME (* node))
- message = "%qE attribute cannot be applied to variables with specific
sections";
+ /* 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 = "%qE attribute cannot be applied to variables with specific
sections";
/* 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.
*/
--
1.8.3.1
From e9404da28ade51c0303394f6ab12528b1c62afca Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <joze...@somniumtech.com>
Date: Fri, 12 May 2017 13:04:59 +0000
Subject: [PATCH 2/3] MSP430: Fix persistent attribute not placing data into
the .persistent section
2017-06-XX Jozef Lawrynowicz <joze...@somniumtech.com>
gcc/
PR target/78818
* config/msp430/msp430.c (msp430_unique_section): Set section to
.persistent if persistent attribute is set.
gcc/testsuite
PR target/78818
* gcc.target/msp430/msp430.exp: Search for tests in subfolders as well
as
main directory.
* gcc.target/msp430/pr78818/pr78818-real.c: New template for tests.
* gcc.target/msp430/pr78818/pr78818-auto.c: New test.
* gcc.target/msp430/pr78818/pr78818-data-region.c: New test.
* gcc.target/msp430/pr78818/pr78818-data-sec.c: Likewise.
---
gcc/config/msp430/msp430.c | 8 ++++++++
gcc/testsuite/gcc.target/msp430/msp430.exp | 4 ++--
gcc/testsuite/gcc.target/msp430/pr78818/pr78818-auto.c | 5 +++++
gcc/testsuite/gcc.target/msp430/pr78818/pr78818-data-region.c | 7 +++++++
gcc/testsuite/gcc.target/msp430/pr78818/pr78818-data-sec.c | 8 ++++++++
gcc/testsuite/gcc.target/msp430/pr78818/pr78818-real.c | 9 +++++++++
6 files changed, 39 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/msp430/pr78818/pr78818-auto.c
create mode 100644
gcc/testsuite/gcc.target/msp430/pr78818/pr78818-data-region.c
create mode 100644 gcc/testsuite/gcc.target/msp430/pr78818/pr78818-data-sec.c
create mode 100644 gcc/testsuite/gcc.target/msp430/pr78818/pr78818-real.c
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index cdd765b..c359c77 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -2000,6 +2000,14 @@ msp430_data_attr (tree * node,
&& DECL_SECTION_NAME (* node))
message = "%qE attribute cannot be applied to variables with specific
sections";
+ /* 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 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))
diff --git a/gcc/testsuite/gcc.target/msp430/msp430.exp
b/gcc/testsuite/gcc.target/msp430/msp430.exp
index e54d531..a056417 100644
--- a/gcc/testsuite/gcc.target/msp430/msp430.exp
+++ b/gcc/testsuite/gcc.target/msp430/msp430.exp
@@ -34,8 +34,8 @@ if ![info exists DEFAULT_CFLAGS] then {
dg-init
# Main loop.
-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cCS\]]] \
- "" $DEFAULT_CFLAGS
+dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/{*,*/*}.\[cCS\]]] \
+ "" $DEFAULT_CFLAGS
# All done.
dg-finish
diff --git a/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-auto.c
b/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-auto.c
new file mode 100644
index 0000000..1fb0b28
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-auto.c
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+
+/* { dg-final { scan-assembler-not "\\.comm" } } */
+
+#include "pr78818-real.c"
diff --git a/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-data-region.c
b/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-data-region.c
new file mode 100644
index 0000000..5da6a82
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-data-region.c
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+
+/* { dg-options "-mdata-region=either" } */
+
+/* { dg-final { scan-assembler-not "\\.either" } } */
+
+#include "pr78818-real.c"
diff --git a/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-data-sec.c
b/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-data-sec.c
new file mode 100644
index 0000000..2d40ea7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-data-sec.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+
+/* { dg-options "-fdata-sections" } */
+
+/* { dg-final { scan-assembler-not "\\.data" } } */
+/* { dg-final { scan-assembler-not "\\.bss" } } */
+
+#include "pr78818-real.c"
diff --git a/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-real.c
b/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-real.c
new file mode 100644
index 0000000..504ed4a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-real.c
@@ -0,0 +1,9 @@
+__attribute__((persistent)) int persistent_1 = 1;
+__attribute__((persistent)) int persistent_2 = 0;
+static __attribute__((persistent)) int persistent_3 = 1;
+static __attribute__((persistent)) int persistent_4 = 0;
+
+int main (void)
+{
+ return 0;
+}
--
1.8.3.1
From 0edf1ea2774381eeaac395d1b010aae03ec98418 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <joze...@somniumtech.com>
Date: Fri, 5 May 2017 15:31:39 +0000
Subject: [PATCH 3/3] MSP430: Emit warning when persistent attribute is used on
an automatic variable
2017-06-XX Jozef Lawrynowicz <joze...@somniumtech.com>
gcc/
PR target/78818
* config/msp430/msp430.c (msp430_unique_section): Warn if .persistent
attribute is used on an automatic variable.
gcc/testsuite
PR target/78818
* gcc.target/msp430/pr78818/pr78818-auto-warn.c: New test.
---
gcc/config/msp430/msp430.c | 5 +++++
.../gcc.target/msp430/pr78818/pr78818-auto-warn.c | 15 +++++++++++++++
2 files changed, 20 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/msp430/pr78818/pr78818-auto-warn.c
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index c359c77..dfd2af6 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -40,6 +40,7 @@
#include "expr.h"
#include "langhooks.h"
#include "builtins.h"
+#include "intl.h"
/* This file should be included last. */
#include "target-def.h"
@@ -2000,6 +2001,10 @@ msp430_data_attr (tree * node,
&& DECL_SECTION_NAME (* node))
message = "%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))
+ 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.
diff --git a/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-auto-warn.c
b/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-auto-warn.c
new file mode 100644
index 0000000..1d92fe1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/pr78818/pr78818-auto-warn.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+
+__attribute__((persistent)) int persistent_1_g = 1;
+__attribute__((persistent)) int persistent_2_g = 0;
+static __attribute__((persistent)) int persistent_3_g = 1;
+static __attribute__((persistent)) int persistent_4_g = 0;
+
+int main (void)
+{
+ __attribute__((persistent)) int persistent_1 = 1; /* { dg-warning "attribute
has no effect on automatic" } */
+ __attribute__((persistent)) int persistent_2 = 0; /* { dg-warning "attribute
has no effect on automatic" } */
+ static __attribute__((persistent)) int persistent_3 = 1;
+ static __attribute__((persistent)) int persistent_4 = 0;
+ return 0;
+}
--
1.8.3.1
