• State Resolved
  • Locked Locked
  • Replies 4 replies
  • Answers 1 answer
  • Subscribers 80 subscribers
  • Views 964 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.

Code Composer Studio V5 Macro Expansion bug?

Greg Greenwood
Prodigy 90 points


Working with an MSP430 design I came across unexpected behavior with some very simple C code.

the original code:

#define INPUT_BIT P1IN & Bit2  //checks input P1 bit 2

...later in the code:

If (INPUT_BIT == 0) input_low();  //CASE #1

else input_high();

The code always executes input_high().  If I change the if statement to read:

if((INPUT_BIT) == 0) input_low();    // CASE #2

then the if statement branches according to the input on bit2 of portA.

Question: Is this a compiler bug?

I would have thought that the compiler after expanding the macro in CASE  #1 would get

If ( P1IN & Bit2 == 0) input_low();

which should be evaluated correctly but in CCS V5 the parenthesizes appear to be necessary.

Am I missing something?

  • 0
    TI__Expert 7535 points

    Hi Greg,

    Have you had a chance to look at the dis-assembly of your program?  I do not believe that there is any compiler error taking place; rather, the compiler is making an assumption on order of operations that is different than your assumption.

    The examples cases below should illustrate this.

    1. First, let's look at what happens with parentheses (this is your test case that works).  I've faked out a P1 register with a volatile byte for this example.  Everything else matches your test case.  Notice in the disassembly of the if statement that the first action taken is a bitwise AND (BIT.B) between the value BIT1 and P1.  Then, if that result is zero (JNE), the if statement is entered.  This works as you intend in your application.

    Now let's look at the case where the parethesis are removed.  Here, the compiler has to assume an order of operations (bitwise AND first, or logical equals first)?  The compiler operates right to left on the parameters.  In other words, BIT1 and 0 would be tested for equality first, then that result (a 1 or a zero) would be bitwise AND-ed with P1 and that result would determine the jump.  Since BIT1 will never equal zero, the if statement parameters will always return zero.  As such, the if statement will never be entered.  The compiler recognizes this, and optimizes the statement out with a simple non-conditional jump.

    Below is a third case for illustration's sake, to prove out the right-to-left execution order.  Rather then testing with a 0 as a constant, I tested with a volatile memory location (to force the assmbler into checking it, rather than optimizing it out like the test above).  If you notice, BIT1 (which is 0x02) is compared with testVariable first.  If they are not equal, 1 is ANDed with P1.  Otherwise, 0 is ANDed with P1.  This shows that the right-most statement executes first, when you do not provide any explicit instructions regarding order of operations. 

    Best practice in C-code is to use paranthesis with multiple bitwise/logical operations that are inline.  The MSP430 compiler assumption here is an MSP430 assumption; there is no C specification for left-to-right or right-to-left in this case, (at least to my knowledge).  You can include the parenthesis in your #define, if you do not wish to include them every time you reference the port input.

    Regards,
    Walter

  • 0
    Guru 100835 points

    This is not a compiler bug,  it's not an implementation choice, and it's unambiguous.

    The C language says that "==" takes precedence over "&", so

    If ( P1IN & Bit2 == 0) input_low();

    must(!) execute as

    if ( P1IN & (Bit2 == 0)) input_low();

    Since (with high probability) "Bit2" is not 0, this expression evaluates to 0 always. As Mr. Schnoor suggests: If in doubt, parenthesize. It's even more important to parenthesize (in general) any value that gets #define-d since you never know where it will turn up and you won't see what's wrong by looking at it.

  • 0 in reply to Bruce McKenney
    Prodigy 90 points

    Thanks for the clarification.  I missed the operator precedence == before &.

    I also found it interesting that associativity in this case is left to right.

  • 0
    Guru 227245 points

    Greg Greenwood1 said:
    If (INPUT_BIT == 0) input_low();  //CASE #1

    This resolves to

    if (P1IN & Bit2 == 0) which is equivalent to if (P1IN & (Bit2==0)). Since Bit2 is != 0, this comes down to if(P1IN & 0) = if (false).

    It's not a precompiler bug, it is a coder bug. This type of macros always has to be done like this:

    #define INPUT_BIT (P1ÎN & Bit2)

    to avoid messing up any expression by operator precendences.

    Keep in mind htat macro expansion is a plain text replacement. The compiler later doesn't know which part was put there by a macro, so it cannot assume any brackets around such a block.

**Attention** This is a public forum