From c6def571a47df5394ca9f5c5c4918f0ffcf97375 Mon Sep 17 00:00:00 2001 From: Jozef Lawrynowicz 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 * 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 * 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 %"); + else if (has_attr ("section", * node)) + message = G_("ignoring attribute %qE because it conflicts with " + "attribute %"); /* 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 %"); /* 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 %"); + 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