• State Resolved
  • Locked Locked
  • Replies 8 replies
  • Subscribers 81 subscribers
  • Views 664 views
  • Users 0 members are here
Support feedback
Options
Options
Related

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.

TI CCS compiles MSP430 C code with incorrect constant conversion: 0xFF -> #127

Steve Kraus
Prodigy 50 points
Other Parts Discussed in Thread: MSP430F2618

I'm using  Version: 5.3.0.00090 to compile a small function to disable the watchdog.  I needed to implement the same function in a JavaScript function.  As a time saver I looked at the generated ASM file thinking I'll just copy the constants from the assmebly so I don't have to look up all the constants.  But I found a puzzling discrepancy in one line of the code, highlighted below.

./**
 * Disables the watchdog while still retaining the settings.
 *
 */
void wdDisable(void)
{
 //Set the Hold Bit - Retain Current Setting | Password | Turn Off
 WDTCTL = (WDTCTL & 0x00FF) | (WDTPW | WDTHOLD);
}


The following are defined in "msp430f2618.h" and the load map
WDTCTL = 0x120   WDTPW = 0x5A00    WDTHOLD = 0x0080

Here is the assembly code generated:

wdDisable:
;* --------------------------------------------------------------------------*
 .dwcfi cfa_offset, 4
 .dwcfi save_reg_to_mem, 16, -4
;** 52 -----------------------    WDTCTL = WDTCTL&0x7fu|0x5a80u;
 .dwpsn file "../DeviceDriverLayer/Watchdog/Watchdog.c",line 52,column 2,is_stmt
        MOV.W     #127,r15              ; [] |52|
        AND.W     &WDTCTL+0,r15         ; [] |52|
        OR.W      #23168,r15            ; [] |52|
        MOV.W     r15,&WDTCTL+0         ; [] |52|
;**   -----------------------    return;

The #127 constant should be #255.  And the 0x7fu in the constant above should be 0xFFu..  The #23168 constant is correct.

The wdDisable function works.  And the code generated sets bit 8 properly due to WDTHOLD. 
But what about the rest of our code, where we frequently use hex constants?  Is there are compiler option?

thanks for your help,

Steven Kraus
QiG group
skraus@qig.com

  • 0
    Guru 58965 points

    Interesting! Please try modify that statement and check the compiler generated assembly code.

    WDTCTL = (WDTCTL & 0x00FF) | (WDTPW);

  • 0 in reply to old_cow_yellow
    Guru 58965 points

    Also try:

    WDTCTL = (WDTCTL & 0x5AFF) | (WDTPW | WDTHOLD);

  • 0
    Guru 26430 points

    Perhaps your assembler file is out of sync with the C source. At some time in the past, was 0x7Fu ever used? The "return" in the assembler is missing from the "C' source. Maybe try deleting the assembler output to make sure you get a freshly generated assembler file.

  • 0
    Expert 2505 points

    OCY's answers are somewhat opaque, but the point  is that the instructions correctly implement the C code.  That the compiler didn't use the mask  you expected is an issue with your expectations rather than with the compiler.

  • 0
    Guru 227245 points

    Since you OR WDTHOLD (which is 0x80) into the result later, it makes no difference whether the AND is performed with 0x7f or 0xff. That's what you can even see in the comment:

    Steve Kraus said:
    WDTCTL = WDTCTL&0x7fu|0x5a80u;
    The result is therefore correct.

    However, the MOV.W #127,r15 could be instead performed as mov.b #-1, R15, which is shorter and faster, but your use of 0x00ff as constant prevents this.

    I agree that it is surprising, but it isn't wrong.

  • 0 in reply to old_cow_yellow
    Prodigy 50 points

    Hi OCY,

    Thanks for your help.  Here is the C and asm file. 

    void wdDisable(void)
    {
    //Set the Hold Bit - Retain Current Setting | Password | Turn Off
        WDTCTL = (WDTCTL & 0x00FF) | (WDTPW | WDTHOLD); // line 52
            WDTCTL = (WDTCTL & 0x00FF) | (WDTPW); // line 53
            WDTCTL = (WDTCTL & 0x5AFF) | (WDTPW | WDTHOLD); // line 54
    }

    Here's the assembly generated for each of the statements.  Sorry, I didn't paste in the RETA portion.
    In each  case, I checked the date & time of the "Watchdog.asm" as well as the source "Watchdog.c" file to verify the compile generated the asm file.

    wdDisable:
    ;* --------------------------------------------------------------------------*
     .dwcfi cfa_offset, 4
     .dwcfi save_reg_to_mem, 16, -4
    ;** 52 -----------------------    WDTCTL = WDTCTL&0x7fu|0x5a80u;
     .dwpsn file "../DeviceDriverLayer/Watchdog/Watchdog.c",line 52,column 2,is_stmt
            MOV.W     #127,r15              ; [] |52|
            AND.W     &WDTCTL+0,r15         ; [] |52|
            OR.W      #23168,r15            ; [] |52|
            MOV.W     r15,&WDTCTL+0         ; [] |52|
    ;** 53 -----------------------    WDTCTL = WDTCTL&0xffu|0x5a00u;
     .dwpsn file "../DeviceDriverLayer/Watchdog/Watchdog.c",line 53,column 2,is_stmt
            MOV.W     &WDTCTL+0,r15         ; [] |53|
            BIC.B     #0,r15                ; [] |53|
            OR.W      #23040,r15            ; [] |53|
            MOV.W     r15,&WDTCTL+0         ; [] |53|
    ;** 54 -----------------------    WDTCTL = WDTCTL&0x7fu|0x5a80u;
     .dwpsn file "../DeviceDriverLayer/Watchdog/Watchdog.c",line 54,column 2,is_stmt
            MOV.W     #127,r15              ; [] |54|
            AND.W     &WDTCTL+0,r15         ; [] |54|
            OR.W      #23168,r15            ; [] |54|
            MOV.W     r15,&WDTCTL+0         ; [] |54|
    ;**   -----------------------    return;
            RETA      ; []

     thanks,
    Steve

  • 0 in reply to Peter Bigot
    Prodigy 50 points

    Hi Peter,

    You are right, it does correctly implement the C code, and it is my expectations that are wrong.  I am an old hand using C and assemblers, although I have not used the MSP430 nor TI compiler before.  I have spent many hours with load maps, reference manuals, and a hex calculator tracing through code in a debugger trying to figure out questions like"  "what generated this code?" and "how did we get here?" and "maybe we should turn off optimization for this function".  My expectations are that it would implement a straightforward C statement without refactoring the constants.  Since the operations are in registers before writing to an I/O port, there is no problem.  I can see this function is correct and I have seen it work.  I'm just worried about refactoring in other functions.  We're trying to test and get this product out the door!

    thanks for your help,
    Steve

  • 0 in reply to Steve Kraus
    Guru 227245 points

    Steve Kraus said:
    Since the operations are in registers before writing to an I/O port, there is no problem.

    All MSP hardware registers are declared volatile. So the compiler ensures that every RMW operation does exactly one read and one write (and other things).
    If the 'register' were a normal non-volatile variable, then intermediate values might occur (but less register usage, maybe none, thus perhaps more effective code). in this case, an ISR that also accesses this variable might pick up an intermediate value. Ergo: variables that are used in main code and in ISRs should be declared volatile.

**Attention** This is a public forum