Discussion:
[PATCH][GAS][MSP430] Improve NOP warnings/insertion behaviour around interrupt state changes
Jozef Lawrynowicz
2018-11-25 13:30:44 UTC
Permalink
The attached patch for the assembler improves the handling of NOPs around
interrupt state changes for MSP430.

Currently the assembler will insert a NOP after an interrupt state change for
both 430 and 430x ISA. Unless the interrupt state change instruction is
explicitly "EINT", then a generic message about how a NOP might be needed after
an interrupt state change is emitted.

The new behaviour tightens the constraints for warning about NOPs for the 430
ISA, so NOPs are only inserted/warned about when needed:
- 430 and 430x ISA require a NOP after DINT.
- Only the 430x ISA requires NOP before EINT
- Only the 430x ISA requires NOP after every EINT. CPU42 errata.

There are also some new messages to better describe where NOPs are being
inserted/are thought to be required.

Another change is some logic to interpret the value being written
to the SR when the constant generator is used as the source, to understand if
interrupts will be enabled or disabled as a result of the instruction.

Also added many new tests.

If the patch is acceptable, I would appreciate if someone would apply it for
me, as I don't have write access.
Nick Clifton
2018-11-27 12:30:44 UTC
Permalink
Hi Jozef,
Post by Jozef Lawrynowicz
If the patch is acceptable, I would appreciate if someone would apply it for
me, as I don't have write access.
Approved and applied.
Post by Jozef Lawrynowicz
diff --git a/gas/config/tc-msp430.c b/gas/config/tc-msp430.c
[...]
Post by Jozef Lawrynowicz
@@ -2654,37 +2733,76 @@ msp430_operands (struct msp430_opcode_s * opcode, char * line)
[...]
Post by Jozef Lawrynowicz
- if (gen_interrupt_nops)
- /* Emit a NOP between interrupt enable/disable.
- See 1.3.4.1 of the MSP430x5xx User Guide. */
- doit = TRUE;
- break;
-
This deleted the "break" statement at the end of NOP_CHECK_INTERRUPT case.
I assumed that your intent was not to fall through into the NOP_CHECK_CPU12
case, so I restored the break statement.

Cheers
Nick
Jozef Lawrynowicz
2018-11-27 15:58:30 UTC
Permalink
On Tue, 27 Nov 2018 12:30:44 +0000
Post by Nick Clifton
Approved and applied.
Hi Nick,
Thanks for the review.
Post by Nick Clifton
Post by Jozef Lawrynowicz
diff --git a/gas/config/tc-msp430.c b/gas/config/tc-msp430.c
[...]
Post by Jozef Lawrynowicz
@@ -2654,37 +2733,76 @@ msp430_operands (struct msp430_opcode_s * opcode, char * line)
[...]
Post by Jozef Lawrynowicz
- if (gen_interrupt_nops)
- /* Emit a NOP between interrupt enable/disable.
- See 1.3.4.1 of the MSP430x5xx User Guide. */
- doit = TRUE;
- break;
-
This deleted the "break" statement at the end of NOP_CHECK_INTERRUPT case.
I assumed that your intent was not to fall through into the NOP_CHECK_CPU12
case, so I restored the break statement.
Ah yes, that was a mistake, thanks for fixing it.

Jozef

Loading...