[committed,MSP430] Warn that the critical function attribute has no effect on interrupt functions
diff mbox series

Message ID 20181229202634.17bf98ee@jozef-Aspire-VN7-793G
State New
Headers show
Series
  • [committed,MSP430] Warn that the critical function attribute has no effect on interrupt functions
Related show

Commit Message

Jozef Lawrynowicz Dec. 29, 2018, 8:26 p.m. UTC
The attached patch fixes an issue for MSP430 where the "critical" and
"interrupt" function attributes could be used together.

The MSP430 hardware already performs the same tasks as the "critical"
attribute when entering/exiting an ISR i.e. disabling interrupts on entry, and
restoring the interrupt state on exit. Using the "critical" attribute with
"interrupt" is therefore redundant and adds extra unnecessary code.

A warning is now emitted, and the "critical" attribute is ignored, when using
the two attributes together.

Committed to trunk.

Patch
diff mbox series

From 18eb3191fee5f7cc8288cfd88a0b76259b8b547c Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Sat, 29 Dec 2018 16:27:49 +0000
Subject: [PATCH] [MSP430] Warn that the critical function attribute has no
 effect on interrupt functions

2018-12-29  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	gcc/ChangeLog:

	* config/msp430/msp430.c (msp430_attr): Warn when the critical and
	interrupt function attributes are used together.
	* gcc/doc/extend.texi: Update documentation on the critical attribute.

	gcc/testsuite/ChangeLog:

	* gcc.target/msp430/critical-interrupt.c: New test.

---
 gcc/config/msp430/msp430.c                           |  9 +++++++++
 gcc/doc/extend.texi                                  |  8 ++++++--
 gcc/testsuite/gcc.target/msp430/critical-interrupt.c | 12 ++++++++++++
 3 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/msp430/critical-interrupt.c

diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index 3a41cc0..21b5819 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -1946,6 +1946,13 @@  msp430_attr (tree * node,
 	  TREE_USED (* node) = 1;
 	  DECL_PRESERVE_P (* node) = 1;
 	}
+      if (is_critical_func (* node))
+	{
+	  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))
     {
@@ -1960,6 +1967,8 @@  msp430_attr (tree * 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))
+	message = "critical attribute has no effect on interrupt functions";
     }
   else if (TREE_NAME_EQ (name, ATTR_NAKED))
     {
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 1849120..27724e4 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -4978,8 +4978,12 @@  These function attributes are supported by the MSP430 back end:
 @cindex @code{critical} function attribute, MSP430
 Critical functions disable interrupts upon entry and restore the
 previous interrupt state upon exit.  Critical functions cannot also
-have the @code{naked} or @code{reentrant} attributes.  They can have
-the @code{interrupt} attribute.
+have the @code{naked}, @code{reentrant} or @code{interrupt} attributes.
+
+The MSP430 hardware ensures that interrupts are disabled on entry to
+@code{interrupt} functions, and restores the previous interrupt state
+on exit. The @code{critical} attribute is therefore redundant on
+@code{interrupt} functions.
 
 @item interrupt
 @cindex @code{interrupt} function attribute, MSP430
diff --git a/gcc/testsuite/gcc.target/msp430/critical-interrupt.c b/gcc/testsuite/gcc.target/msp430/critical-interrupt.c
new file mode 100644
index 0000000..3ef7a12
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/critical-interrupt.c
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-final { scan-assembler-not "attributes.*critical" } } */
+
+void __attribute__((interrupt,critical))
+fn1 (void)
+{ /* { dg-warning "critical attribute has no effect on interrupt functions" } */
+}
+
+void __attribute__((critical,interrupt))
+fn2 (void)
+{ /* { dg-warning "critical attribute has no effect on interrupt functions" } */
+}
-- 
2.7.4