• State Resolved
  • Locked Locked
  • Replies 7 replies
  • Answers 1 answer
  • Subscribers 53 subscribers
  • Views 878 views
  • Users 0 members are here
Support feedback
Options
Options

This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

CCS/MSP430FG4618: Spurious "a NOP may be needed here..." warnings using_enable_interrupts() in GCC on MSP processors

Andrew McLaren
Intellectual 580 points
Part Number: MSP430FG4618

Tool/software: Code Composer Studio

From looking through the forums, there are a couple of notes related to spurious "a NOP may be needed here" errors, but these were regarding successive enable/disable interrupt instructions. I'm also seeing these in response to standalone _enable_interrupts calls.

This has probably been around a while, but with a recent upgrade to V9.1.0 of CCS, I've been looking at any warnings generated, with a view to reducing noise. These ones I haven't been able to work around.

The code is written in C, and compiled with GNU V8.3.0.16. The logic uses _enable_interrupts and _disable_interrupts as the entry points to eint and dint. From the MSP430 GCC Users Guide (SLAU464E, June 19), section 4.9.4 (NOP Instructions Surrounding Interrupt State Changes), it states that any necessary pre & post NOP instructions associated with enabling and disabling interrupts are handled correctly in the _enable_interrupts and _disable_interrupts macros, and from having a peek at in340.h, this appears to be the case. However, the compiler continues to report warnings "a NOP may be needed here", after an interrupt enable instruction" (and the equivalent "before"). There are no warnings generated around _disable_interrupts.

I've had a play surrounding the _enable_interrupts macros with _no_operation macros, but the warnings remain. Not surprising, as I believe the NOP's are already there, but the assembler is failing to recognise them, and generating the warnings.

Its not a big issue, but less noise is always better!

Andrew

  • 0
    Expert 2410 points

    Hi,

    Are you able to provide a short snippet of C code that produces these warnings?

    Thanks,

  • 0 in reply to Jozef_Lawrynowicz
    Intellectual 580 points

    That would be too easy!

    Just gone and reduced this to some minimal code, and of course it works perfectly. What a surprise. With this minimal code I can turn the warnings off and on by switching between _enable_interrupts() and the raw eint as expected.

    The warnings all follow the following form;

    C:\Users\Andrew\AppData\Local\Temp\ccwZeFhA.s: Assembler messages:
    C:\Users\Andrew\AppData\Local\Temp\ccwZeFhA.s:48: Warning: a NOP might be needed here, before an interrupt enable instruction

    I've been trying to trap the intermediate assembler listing to have a peek in there, but it all seems to be cleaned up, either by GCC or CCS, and haven't found a way to retain it. I've been using the -S option on the compiler, which 'should' retain the assembler listing, but I suspect CCS is removing this in any case, as I've not been able to capture one. Any ideas on how to retain the generated assembler file? Any other ideas for getting a handle on what is happening? That would at least give us a indication on whether the NOP's are being inserted or not (I'm guessing not?).

    (Actually just had another thought, which is to actually run the code with an assembler window open in CCS. Haven't done this for years, and currently don't have an MSP set up to do it, but give me a couple of days...)

    Thanks and regards

    Andrew

  • 0 in reply to Andrew McLaren
    Expert 2410 points

    Andrew McLaren said:

    I've been trying to trap the intermediate assembler listing to have a peek in there, but it all seems to be cleaned up, either by GCC or CCS, and haven't found a way to retain it. I've been using the -S option on the compiler, which 'should' retain the assembler listing, but I suspect CCS is removing this in any case, as I've not been able to capture one. Any ideas on how to retain the generated assembler file? Any other ideas for getting a handle on what is happening? That would at least give us a indication on whether the NOP's are being inserted or not (I'm guessing not?).

    (Actually just had another thought, which is to actually run the code with an assembler window open in CCS. Haven't done this for years, and currently don't have an MSP set up to do it, but give me a couple of days...)

    The "--save-temps" option is what you're looking for. I added this option to "Miscellaneous C source specific flags" in Project Properties>Build>GNU Compiler>Miscellaneous. GCC will now retain some useful files in the build directory. I think it would be useful to have a specific check box for this option in the project properties dialog. I'll work with TI to see if we can add this.

    The "*.i" file is the preprocessed C source. This is useful if you want to extract a testcase without also having to provide supporting header files. All definitions and declarations needed to compile that file will be in the ".i", it's just a matter of reducing that to a smaller test case.

    The "*.s" file is the assembler code we can inspect to see where the NOP warnings are being triggered.

    Regards,

  • 0 in reply to Jozef_Lawrynowicz
    Intellectual 580 points

    Thanks for the pointer Jozef.

    I can see what is happening, but haven't a clue why. I'll put in a dump of the appropriate bits of the .c, the .i and the .s, together with the compiler command and the warnings generated.

    First the C within the library routine

    _disable_interrupts ();
    status = EVT_RaiseInterruptEvent(vector, flags, evt_context);
    _enable_interrupts ();

    And the preprocessed C (the .i)

    __asm__ __volatile__ ("dint { nop");
    status = EVT_RaiseInterruptEvent(vector, flags, evt_context);
    __asm__ __volatile__ ("eint");

    The generated assembler - not surprising without any nop's around the eint

    dint { nop
    ; Removed the assembler implementing the call
    eint

    The compiler command and output;

    Invoking: GNU Compiler

    "C:/ti/ccs910/ccs/tools/compiler/msp430-gcc-8.3.0.16_win32/bin/msp430-elf-gcc-8.3.0.exe" -c -mmcu=msp430fg4618 -mhwmult=none -I"C:/Data/Projects/Software/Micro/Common" -I"C:/ti/ccs910/ccs/tools/compiler/msp430-gcc-8.3.0.16_win32/msp430-elf/include" -I"C:/ti/ccs910/ccs/ccs_base/msp430/include_gcc" -O0 -g -gdwarf-3 -gstrict-dwarf -Wall -mlarge -save-temps -mcode-region=none -mdata-region=none -MMD -MP -MF"trunk/EVT_EventHandler.d" -MT"trunk/EVT_EventHandler.o" -o"trunk/EVT_EventHandler.o" "../trunk/EVT_EventHandler.c"

    EVT_EventHandler.s: Assembler messages:
    EVT_EventHandler.s:264: Warning: a NOP might be needed here, before an interrupt enable instruction
    EVT_EventHandler.s:267: Warning: a NOP might be needed here, after an interrupt enable instruction
    Finished building: "../trunk/EVT_EventHandler.c"

     

    Just for comparison, I did the same sequence with a bit of dummy code - this generates an executable rather than a library, but don't think that should affect anything.

    First the C - just threw a loop between the enable and disable.

    _disable_interrupts ();
    while (ctr < 10) {ctr++;}
    _enable_interrupts ();

    And the preprocessed C (the .i)

    __asm__ __volatile__ ("dint { nop");
    while (ctr < 10) {ctr++;}
    __asm__ __volatile__ ("nop { eint { nop");

    The generated assembler

    ; 17 "../trunk/NOP_Test.c" 1
    dint { nop
    ; Removed the stuff in the middle
    ; 19 "../trunk/NOP_Test.c" 1
    nop { eint { nop

    The compiler command and output;

    Invoking: GNU Compiler

    "C:/ti/ccs910/ccs/tools/compiler/msp430-gcc-8.3.0.16_win32/bin/msp430-elf-gcc-8.3.0.exe" -c -mmcu=msp430fg4618 -mhwmult=none -I"C:/Data/Projects/Software/Micro/Common/trunk" -I"C:/Data/Projects/Software/Micro/Common_Tests" -I"C:/ti/ccs910/ccs/tools/compiler/msp430-gcc-8.3.0.16_win32/msp430-elf/include" -I"C:/ti/ccs910/ccs/ccs_base/msp430/include_gcc" -O0 -g -gdwarf-3 -gstrict-dwarf -Wall -mlarge -save-temps -mcode-region=none -mdata-region=none -MMD -MP -MF"trunk/NOP_Test.d" -MT"trunk/NOP_Test.o" -o"trunk/NOP_Test.o" "../trunk/NOP_Test.c"
    Finished building: "../trunk/NOP_Test.c"

     

    Its many years since I've touched assembler, but the style of the nop { eint { nop is a bit foreign to me. Are the intervening braces some sort of statement separator?

    I did have a peek at the in430.h (where the _enable_interrupts macro is expanded), and interestingly, the nop { eint {nop sequence is only substituted for MSP430X and XV2, processors, otherwise its simply an eint. The dint { nop sequence is applied to all processors. Made me wonder if its failing to detect it''s compiling for a 430X, but nothing in the compile instruction looks wrong. Any ideas? For now I can simply add _no_operations around the _enable_interrupts, which should be safe.

    Thanks and regards

    Andrew

  • 0 in reply to Andrew McLaren
    Expert 2410 points

    Thanks for providing this extra information.

    Andrew McLaren said:
    Its many years since I've touched assembler, but the style of the nop { eint { nop is a bit foreign to me. Are the intervening braces some sort of statement separator?

    Yes you're right it's a GNU assembler construct to separate statements. There's a bit more info in this thread https://e2e.ti.com/support/tools/ccs/f/81/p/503224/1822907#1822907

    Andrew McLaren said:
    I did have a peek at the in430.h (where the _enable_interrupts macro is expanded), and interestingly, the nop { eint {nop sequence is only substituted for MSP430X and XV2, processors, otherwise its simply an eint. The dint { nop sequence is applied to all processors. Made me wonder if its failing to detect it''s compiling for a 430X, but nothing in the compile instruction looks wrong. Any ideas? For now I can simply add _no_operations around the _enable_interrupts, which should be safe.

    So as long as __MSP430_HAS_MSP430X_CPU__ is defined before in430.h is included, "_enable_interrupts" will expand to "nop { eint { nop".

    __MSP430_HAS_MSP430X_CPU__ is defined in msp430fg4618.h, which is included by msp430.h.

    I did a quick experiment and if you include "in430.h" before "msp430.h" then this assembler error is emitted, since in430.h does not know when it is included that you have a 430X CPU, but the assembler does know its a 430X CPU.

    I suspect that this is the cause of your problem. You should only need to include "msp430.h" in your program, and this will pull in any other required header files.

    We can definitely improve the behaviour of the header files here. GCC generates a builtin definition when an MSP430X CPU is in used - "__MSP430X__". This should be checked in addition to/instead of the "__MSP430_HAS_*_CPU__" macros created by the device header file.

    Regards,

  • 0 in reply to Jozef_Lawrynowicz
    Intellectual 580 points

    Hi Jozef,

    You are absolutely right! In fact, it wasn't just the order - this particular module had somehow missed out on a msp430.h include altogether! I initially thought it must have been picked up further down the include chain, but no sign of that. However, the only thing it does that gets close to the MSP specifics is the interrupt management, so I'm assuming there was nothing else in there that referenced any of the processor specific definitions.

    Just out of interest, is there anything that specifies the C structure required to comply with the CCS, GCC environment (such as the requirement for msp430.h, its sequence relative to other things such as in430.h, etc). Much of this I've picked up by osmosis, and wonder if there are other things I've missed, or been ignorant of implications. The closest I've found is the MSP430 GCC Users Guide, which does talk about the mechanism GCC uses to load the processor specific definitions (via msp430.h).

    Thanks for all your help. Another vote for not ignoring warnings!

    Andrew

  • 0 in reply to Andrew McLaren
    Expert 2410 points

    Hi Andrew,

    Andrew McLaren said:

    Just out of interest, is there anything that specifies the C structure required to comply with the CCS, GCC environment (such as the requirement for msp430.h, its sequence relative to other things such as in430.h, etc). Much of this I've picked up by osmosis, and wonder if there are other things I've missed, or been ignorant of implications. The closest I've found is the MSP430 GCC Users Guide, which does talk about the mechanism GCC uses to load the processor specific definitions (via msp430.h).

    Thanks for all your help. Another vote for not ignoring warnings!

    I think we've covered the extent of the quirks here; I wouldn't worry about anything else. Just stick with including "msp430.h" in every source file and avoid manually including any of the other header files.

    The device specific behaviour of the compiler and the msp430-gcc-support-files are set up to be treated as a bit of a black box. In the sense that you just need to specify the MCU with "-mmcu", and include "msp430.h" at the top of your source file, and the rest will be handled behind the scenes. i.e. it will pull in all the device specific definitions required, select the linker script to use and set up the hardware multiply and ISA support for the device.

    Again, this could be documented better in the user guide, so we'll look to improve that.

    Happy to help,