From patchwork Thu Oct 18 23:24:43 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jozef Lawrynowicz X-Patchwork-Id: 986392 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-487860-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=mittosystems.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="HjyTqyC1"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=mittosystems.com header.i=@mittosystems.com header.b="VSQo0Ipt"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 42blWx3Yzlz9s9J for ; Fri, 19 Oct 2018 10:25:15 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :subject:to:cc:message-id:date:mime-version:content-type; q=dns; s=default; b=Gqs2aOZniINNKmTdHfG5G4jtzfcV/jJu4kvik2KyouenZNoasS 5zkibGkawxdwCSrYxUBB1j3fQTPtLEmV1rtkq1dsLAtqtBm2mXmN2KxOVXTIoKv1 +A091ncCZTmT8UaKYNvhKyb3aGM3zjZGl4GUSoVmwtR6k0LUCGxDARrD4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :subject:to:cc:message-id:date:mime-version:content-type; s= default; bh=zlhgszH8M6N4SXremNoExPerX8E=; b=HjyTqyC1bEECMYuOfhgz q9a7/SuwnZdiXG7fXfJeQCS4kIO/bwFsy2YTkIqFK45rAow/1qhd2o9WZQDSNxm8 Psg0QTpHWJv14K5A/9Owx/0DXh637H5ZDEtvNUc2HaMjzyfP9QWM7twz0nN3k5T0 68+UU8s7Z1wTLOBdL4Ngybs= Received: (qmail 62514 invoked by alias); 18 Oct 2018 23:24:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 62479 invoked by uid 89); 18 Oct 2018 23:24:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=UD:W, 3031, silently, handlers X-HELO: mail-wr1-f42.google.com Received: from mail-wr1-f42.google.com (HELO mail-wr1-f42.google.com) (209.85.221.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 18 Oct 2018 23:24:47 +0000 Received: by mail-wr1-f42.google.com with SMTP id w5-v6so35446187wrt.2 for ; Thu, 18 Oct 2018 16:24:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mittosystems.com; s=google; h=from:subject:to:cc:message-id:date:user-agent:mime-version :content-language; bh=6kE7gH/ml/9N+9u3wZzZPr5qeB84x/d/yD446HywOZQ=; b=VSQo0Iptnr0Khgvn3gGzIdtvtOBPe41u6zx9zXpmsjTajhGo+4uvXuS8Kzk2ZMscuy MjWmDd75piG3NNR5Ef+BGJRorh2l1aofdKTZfyV4ByQBcPNb3UTY2ZXuwg3mBekwpRKa HVG885Pc6z0Y4iUBq7dbKl1I2CkrtVqktpJNbpJ+d7lUPkqQNK3Ej4igQRJUdY3Jty0V R+8jKP8T3NYNAjObuszW4QfTnQkja221sCa0PdAV/kmLJJCP0A2GtTnk1+Tb6sb5F2gv C6UQBzLp6uAw6HZeutE9CUZfunnE01+D2a181uhg24Cg3BLW8vBG+dWTdNn61TUGeu/S Vhgw== Received: from [192.168.1.145] ([88.98.203.54]) by smtp.gmail.com with ESMTPSA id o4-v6sm13369928wrj.45.2018.10.18.16.24.43 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Oct 2018 16:24:44 -0700 (PDT) From: Jozef Lawrynowicz Subject: [PATCH] [MSP430] Extend MSP430 data attribute handler To: GCC Patches Cc: Nick Clifton Message-ID: Date: Fri, 19 Oct 2018 00:24:43 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 X-IsSubscribed: yes The "persistent" and "noinit" data attributes for MSP430 assign the variable they are applied to to the .persistent and .noinit sections, respectively. The following patch extends the handler for these attributes to check for misuse. The testsuite updates add tests for these attributes, and some other MSP430 attributes, which previously did not have tests. Ok for trunk? From 5c56509cfa0f285549eceefb457dc59134d30d0e Mon Sep 17 00:00:00 2001 From: Jozef Lawrynowicz Date: Thu, 18 Oct 2018 22:16:22 +0100 Subject: [PATCH] Extend MSP430 data attribute handling 2018-10-19 Jozef Lawrynowicz * 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