MSP420 assembler: Add -m{u,U} options to enable/disable NOP warnings for unknown interrupt state changes

gas	* config/tc-msp430.c (options): New OPTION_UNKNOWN_INTR_NOPS,
	OPTION_NO_UNKNOWN_INTR_NOPS and do_unknown_interrupt_nops.
	(md_parse_option): Handle OPTION_UNKNOWN_INTR_NOPS and
	OPTION_NO_UNKNOWN_INTR_NOPS by setting do_unknown_interrupt_nops
	accordingly.
	(md_show_usage): Likewise.
	(md_shortopts): Add "mu" for OPTION_UNKNOWN_INTR_NOPS and
	"mU" for OPTION_NO_UNKNOWN_INTR_NOPS.
	(md_longopts): Likewise.
	(warn_eint_nop): Update comment.
	(warn_unsure_interrupt): Don't warn if prev_insn_is_nop or
	prev_insn_is_dint or we are assembling for 430 ISA.
	(msp430_operands): Only call warn_unsure_interrupt if
	do_unknown_interrupt_nops == TRUE.
	* testsuite/gas/msp430/nop-unknown-intr.s: New test source file.
	* testsuite/gas/msp430/nop-unknown-intr-430.d: New test.
	* testsuite/gas/msp430/nop-unknown-intr-430x.d: New test.
	* testsuite/gas/msp430/nop-unknown-intr-430x-ignore.d: New test.
	* testsuite/gas/msp430/nop-unknown-intr-430.l: Warning output for new
	test.
	* testsuite/gas/msp430/nop-unknown-intr-430x.l: Likewise.
	* testsuite/gas/msp430/msp430.exp: Add new tests to driver.

gas/ChangeLog

@@ -1,3 +1,28 @@
+2019-04-17  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
+
+	* config/tc-msp430.c (options): New OPTION_UNKNOWN_INTR_NOPS,
+	OPTION_NO_UNKNOWN_INTR_NOPS and do_unknown_interrupt_nops.
+	(md_parse_option): Handle OPTION_UNKNOWN_INTR_NOPS and
+	OPTION_NO_UNKNOWN_INTR_NOPS by setting do_unknown_interrupt_nops
+	accordingly.
+	(md_show_usage): Likewise.
+	(md_shortopts): Add "mu" for OPTION_UNKNOWN_INTR_NOPS and
+	"mU" for OPTION_NO_UNKNOWN_INTR_NOPS.
+	(md_longopts): Likewise.
+	(warn_eint_nop): Update comment.
+	(warn_unsure_interrupt): Don't warn if prev_insn_is_nop or
+	prev_insn_is_dint or we are assembling for 430 ISA.
+	(msp430_operands): Only call warn_unsure_interrupt if
+	do_unknown_interrupt_nops == TRUE.
+	* testsuite/gas/msp430/nop-unknown-intr.s: New test source file.
+	* testsuite/gas/msp430/nop-unknown-intr-430.d: New test.
+	* testsuite/gas/msp430/nop-unknown-intr-430x.d: New test.
+	* testsuite/gas/msp430/nop-unknown-intr-430x-ignore.d: New test.
+	* testsuite/gas/msp430/nop-unknown-intr-430.l: Warning output for new
+	test.
+	* testsuite/gas/msp430/nop-unknown-intr-430x.l: Likewise.
+	* testsuite/gas/msp430/msp430.exp: Add new tests to driver.
+
 2019-04-16  Alan Modra  <amodra@gmail.com>
 
 	* testsuite/gas/all/weakref1.d: xfail nds32.

gas/config/tc-msp430.c

@@ -680,6 +680,9 @@ static bfd_boolean gen_interrupt_nops = FALSE;
 #define OPTION_WARN_INTR_NOPS 'y'
 #define OPTION_NO_WARN_INTR_NOPS 'Y'
 static bfd_boolean warn_interrupt_nops = TRUE;
+#define OPTION_UNKNOWN_INTR_NOPS 'u'
+#define OPTION_NO_UNKNOWN_INTR_NOPS 'U'
+static bfd_boolean do_unknown_interrupt_nops = TRUE;
 #define OPTION_MCPU 'c'
 #define OPTION_MOVE_DATA 'd'
 static bfd_boolean move_data = FALSE;
@@ -1454,6 +1457,13 @@ md_parse_option (int c, const char * arg)
       warn_interrupt_nops = FALSE;
       return 1;
 
+    case OPTION_UNKNOWN_INTR_NOPS:
+      do_unknown_interrupt_nops = TRUE;
+      return 1;
+    case OPTION_NO_UNKNOWN_INTR_NOPS:
+      do_unknown_interrupt_nops = FALSE;
+      return 1;
+
     case OPTION_MOVE_DATA:
       move_data = TRUE;
       return 1;
@@ -1484,13 +1494,16 @@ static void
 msp430_make_init_symbols (const char * name)
 {
   if (strncmp (name, ".bss", 4) == 0
+      || strncmp (name, ".lower.bss", 10) == 0
+      || strncmp (name, ".either.bss", 11) == 0
       || strncmp (name, ".gnu.linkonce.b.", 16) == 0)
     (void) symbol_find_or_make ("__crt0_init_bss");
 
   if (strncmp (name, ".data", 5) == 0
+      || strncmp (name, ".lower.data", 11) == 0
+      || strncmp (name, ".either.data", 12) == 0
       || strncmp (name, ".gnu.linkonce.d.", 16) == 0)
     (void) symbol_find_or_make ("__crt0_movedata");
-
   /* Note - data assigned to the .either.data section may end up being
      placed in the .upper.data section if the .lower.data section is
      full.  Hence the need to define the crt0 symbol.
@@ -1574,7 +1587,7 @@ const pseudo_typeS md_pseudo_table[] =
   {NULL, NULL, 0}
 };
 
-const char *md_shortopts = "mm:,mP,mQ,ml,mN,mn,my,mY";
+const char *md_shortopts = "mm:,mP,mQ,ml,mN,mn,my,mY,mu,mU";
 
 struct option md_longopts[] =
 {
@@ -1589,6 +1602,8 @@ struct option md_longopts[] =
   {"mn", no_argument, NULL, OPTION_INTR_NOPS},
   {"mY", no_argument, NULL, OPTION_NO_WARN_INTR_NOPS},
   {"my", no_argument, NULL, OPTION_WARN_INTR_NOPS},
+  {"mu", no_argument, NULL, OPTION_UNKNOWN_INTR_NOPS},
+  {"mU", no_argument, NULL, OPTION_NO_UNKNOWN_INTR_NOPS},
   {"md", no_argument, NULL, OPTION_MOVE_DATA},
   {"mdata-region", required_argument, NULL, OPTION_DATA_REGION},
   {NULL, no_argument, NULL, 0}
@@ -1620,6 +1635,13 @@ md_show_usage (FILE * stream)
 	   _("  -mY - do not warn about missing NOPs after changing interrupts\n"));
   fprintf (stream,
 	   _("  -my - warn about missing NOPs after changing interrupts (default)\n"));
+  fprintf (stream,
+	   _("  -mU - for an instruction which changes interrupt state, but where it is not\n"
+	     "        known how the state is changed, do not warn/insert NOPs\n"));
+  fprintf (stream,
+	   _("  -mu - for an instruction which changes interrupt state, but where it is not\n"
+	     "        known how the state is changed, warn/insert NOPs (default)\n"
+	     "        -mn and/or -my are required for this to have any effect\n"));
   fprintf (stream,
 	   _("  -md - Force copying of data from ROM to RAM at startup\n"));
   fprintf (stream,
@@ -2536,7 +2558,8 @@ static void
 warn_eint_nop (bfd_boolean prev_insn_is_nop, bfd_boolean prev_insn_is_dint)
 {
   if (prev_insn_is_nop
-      /* Prevent double warning for DINT immediately before EINT.  */
+      /* If the last insn was a DINT, we will have already warned that a NOP is
+	 required after it.  */
       || prev_insn_is_dint
       /* 430 ISA does not require a NOP before EINT.  */
       || (! target_is_430x ()))
@@ -2554,10 +2577,16 @@ warn_eint_nop (bfd_boolean prev_insn_is_nop, bfd_boolean prev_insn_is_dint)
 /* Use when unsure what effect the insn will have on the interrupt status,
    to insert/warn about adding a NOP before the current insn.  */
 static void
-warn_unsure_interrupt (void)
+warn_unsure_interrupt (bfd_boolean prev_insn_is_nop,
+		       bfd_boolean prev_insn_is_dint)
 {
-  /* Since this could enable or disable interrupts, need to add/warn about
+  if (prev_insn_is_nop
-     adding a NOP before and after this insn.  */
+      /* If the last insn was a DINT, we will have already warned that a NOP is
+	 required after it.  */
+      || prev_insn_is_dint
+      /* 430 ISA does not require a NOP before EINT or DINT.  */
+      || (! target_is_430x ()))
+    return;
   if (gen_interrupt_nops)
     {
       gen_nop ();
@@ -3646,12 +3675,12 @@ msp430_operands (struct msp430_opcode_s * opcode, char * line)
 	  else if (op1.mode == OP_REG
 		   && (op1.reg == 2 || op1.reg == 3))
 	    this_insn_is_dint = TRUE;
-	  else
+	  else if (do_unknown_interrupt_nops)
 	    {
 	      /* FIXME: Couldn't work out whether the insn is enabling or
 		 disabling interrupts, so for safety need to treat it as both
 		 a DINT and EINT.  */
-	      warn_unsure_interrupt ();
+	      warn_unsure_interrupt (prev_insn_is_nop, prev_insn_is_dint);
 	      check_for_nop |= NOP_CHECK_INTERRUPT;
 	    }
 	}

gas/testsuite/gas/msp430/either-data-bss-sym.d

@@ -0,0 +1,9 @@
+#objdump: -t
+#name: Check symbols to initialise data and bss have been defined for .either sections
+#...
+.*__crt0_movedata.*
+.*__crt0_move_highdata.*
+#...
+.*__crt0_init_bss.*
+.*__crt0_init_highbss.*
+#pass

gas/testsuite/gas/msp430/either-data-bss-sym.s

@@ -0,0 +1,24 @@
+	.file	"main.c"
+.text
+	.global	a
+	.section	.either.data,"aw"
+	.balign 2
+	.type	a, @object
+	.size	a, 2
+a:
+	.short	42
+	.global	b
+	.section	.either.bss,"aw",@nobits
+	.balign 2
+	.type	b, @object
+	.size	b, 2
+b:
+	.zero	2
+.text
+	.balign 2
+	.global	main
+	.type	main, @function
+main:
+.L2:
+	BR	#.L2
+	.size	main, .-main

gas/testsuite/gas/msp430/low-data-bss-sym.d

@@ -0,0 +1,7 @@
+#objdump: -t
+#name: Check symbols to initialise data and bss have been defined for .lower sections
+#...
+.*__crt0_movedata.*
+#...
+.*__crt0_init_bss.*
+#pass

gas/testsuite/gas/msp430/low-data-bss-sym.s

@@ -0,0 +1,24 @@
+	.file	"main.c"
+.text
+	.global	a
+	.section	.lower.data,"aw"
+	.balign 2
+	.type	a, @object
+	.size	a, 2
+a:
+	.short	42
+	.global	b
+	.section	.lower.bss,"aw",@nobits
+	.balign 2
+	.type	b, @object
+	.size	b, 2
+b:
+	.zero	2
+.text
+	.balign 2
+	.global	main
+	.type	main, @function
+main:
+.L2:
+	BR	#.L2
+	.size	main, .-main

gas/testsuite/gas/msp430/msp430.exp

@@ -24,8 +24,10 @@ if [expr [istarget "msp430-*-*"]]  then {
     run_dump_test "bad"
     run_dump_test "errata_warns"
     run_dump_test "errata_fixes"
-    run_dump_test "high-data-bss-sym" { { as "-mdata-region=upper" } }
+    run_dump_test "low-data-bss-sym"
-    run_dump_test "high-data-bss-sym" { { as "-mdata-region=either" } }
+    run_dump_test "either-data-bss-sym"
+    run_dump_test "high-data-bss-sym" { { as "-ml -mdata-region=upper" } }
+    run_dump_test "high-data-bss-sym" { { as "-ml -mdata-region=either" } }
     run_dump_test "pr22133"
     run_dump_test "nop-int-430"
     run_dump_test "nop-int-430x"
@@ -38,4 +40,7 @@ if [expr [istarget "msp430-*-*"]]  then {
     run_dump_test "nop-dint-430x"
     run_dump_test "nop-dint-430x-silent"
     run_dump_test "nop-dint-430x-ignore"
+    run_dump_test "nop-unknown-intr-430"
+    run_dump_test "nop-unknown-intr-430x"
+    run_dump_test "nop-unknown-intr-430x-ignore"
 }

gas/testsuite/gas/msp430/nop-unknown-intr-430.d

@@ -0,0 +1,14 @@
+#name: Unknown Interrupt State NOP Insertions (MSP430 CPU)
+#source: nop-unknown-intr.s
+#as: -my -mu -mcpu=430
+#warning_output: nop-unknown-intr-430.l
+#objdump: -d --prefix-addresses --show-raw-insn
+
+.*: +file format .*msp.*
+
+
+Disassembly of section .text:
+0x0+0000 12 42 00 00[ 	]+mov[ 	]+&0x0000,r2[ 	]+;0x0000
+0x0+0004 1a 42 00 00[ 	]+mov[ 	]+&0x0000,r10[ 	]+;0x0000
+0x0+0008 02 47[ 	]+mov[ 	]+r7,[ 	]+r2[ 	]+;
+0x0+000a 1a 42 00 00[ 	]+mov[ 	]+&0x0000,r10[ 	]+;0x0000

gas/testsuite/gas/msp430/nop-unknown-intr-430.l

@@ -0,0 +1,3 @@
+[^:]*: Assembler messages:
+[^:]*:11: Warning: a NOP might also be needed here, after the instruction that changed interrupt state
+[^:]*:16: Warning: a NOP might also be needed here, after the instruction that changed interrupt state

gas/testsuite/gas/msp430/nop-unknown-intr-430x-ignore.d

@@ -0,0 +1,13 @@
+#name: Ignore Unknown Interrupt State NOP Insertions (MSP430X CPU)
+#source: nop-unknown-intr.s
+#as: -my -mU -mcpu=430x
+#objdump: -d --prefix-addresses --show-raw-insn
+
+.*: +file format .*msp.*
+
+
+Disassembly of section .text:
+0x0+0000 12 42 00 00[ 	]+mov[ 	]+&0x0000,r2[ 	]+;0x0000
+0x0+0004 1a 42 00 00[ 	]+mov[ 	]+&0x0000,r10[ 	]+;0x0000
+0x0+0008 02 47[ 	]+mov[ 	]+r7,[ 	]+r2[ 	]+;
+0x0+000a 1a 42 00 00[ 	]+mov[ 	]+&0x0000,r10[ 	]+;0x0000

gas/testsuite/gas/msp430/nop-unknown-intr-430x.d

@@ -0,0 +1,14 @@
+#name: Unknown Interrupt State NOP Insertions (MSP430X CPU)
+#source: nop-unknown-intr.s
+#as: -my -mu -mcpu=430x
+#warning_output: nop-unknown-intr-430x.l
+#objdump: -d --prefix-addresses --show-raw-insn
+
+.*: +file format .*msp.*
+
+
+Disassembly of section .text:
+0x0+0000 12 42 00 00[ 	]+mov[ 	]+&0x0000,r2[ 	]+;0x0000
+0x0+0004 1a 42 00 00[ 	]+mov[ 	]+&0x0000,r10[ 	]+;0x0000
+0x0+0008 02 47[ 	]+mov[ 	]+r7,[ 	]+r2[ 	]+;
+0x0+000a 1a 42 00 00[ 	]+mov[ 	]+&0x0000,r10[ 	]+;0x0000

gas/testsuite/gas/msp430/nop-unknown-intr-430x.l

@@ -0,0 +1,5 @@
+[^:]*: Assembler messages:
+[^:]*:9: Warning: a NOP might be needed here, before this interrupt state change
+[^:]*:11: Warning: a NOP might also be needed here, after the instruction that changed interrupt state
+[^:]*:14: Warning: a NOP might be needed here, before this interrupt state change
+[^:]*:16: Warning: a NOP might also be needed here, after the instruction that changed interrupt state

gas/testsuite/gas/msp430/nop-unknown-intr.s

@@ -0,0 +1,16 @@
+	.text
+
+;;; Test for warnings when an instruction might change interrupt state, but
+;;; the assembler doesn't know whether interrupts will be enabled or disabled.
+;;; "MOV &FOO,R10" is used as an artbitrary statement which isn't a NOP, to
+;;; break up the instructions being tested.
+
+;;; Moving a value in memory into SR might change interrupt state
+	MOV &FOO,R2
+
+	MOV &FOO,R10
+
+;;; Moving a value from a register into SR might change interrupt state
+	MOV R7,R2
+
+	MOV &FOO,R10