GCC optimization bug?

[Druzyek 36]

I have one MSP430 on a launchpad and another on a breadboard connected to it. I am running some test code to see if they can communicate. I am using an unsigned int variable as a counter. Compiling with -Os the code hangs when the value of the variable reaches 2^15. Compiling without -Os it runs as expected.

 

pt is the variable in question. I thought the code for communicating between chips was at fault but it works fine when compiled without -Os. The problem seems to be when I use pt with the command SendSlave. If I use value (always 255 or less) it works. I think it is hanging in SlaveReceive. Is there a bug in the compiler?

 

EDIT: Same behavior with the latest GCC 4.6.3 and the experimental 4.7.0. Compiled like this:

msp430-gcc -std=gnu99 -g -Os -mmcu=msp430G2553 -o temp_src.elf temp_src.c

#include <msp430.h>
#include <stdio.h>
#include <stdbool.h>
#include <string.h>
#include <stdlib.h>
#include <math.h>

#define SLAVE_DATA      BIT6  //P2.6
#define SLAVE_CLOCK     BIT7  //P2.7

#define LED             BIT6  //P1.6

enum SlaveCommands {ECHO, LISTEN};
#define SLAVE_WAIT 100
#define SLAVE_WAIT_SMALL 50

static void delay_ms(int ms);
static void SlaveSend(unsigned int value);
static unsigned int SlaveReceive();

int main(void)
{
  WDTCTL=WDTPW + WDTHOLD;

  BCSCTL1=CALBC1_16MHZ;
  DCOCTL=CALDCO_16MHZ;

  P2SEL=0;
  P2SEL2=0;

  P1OUT=0;

  P1DIR=0xFF;
  P2DIR=0xFF;

  delay_ms(100);

  unsigned int value=0,v2,pt;
  int i,j,k,x;
  pt=32000;
  for (k=0;k<1000;k++)
  {
    for (i=0;i<256;i++)
    {
      //If pt doesn't increase past 2^15, i doesn't become 3
      if (i>2) P1OUT|=LED;
      for (j=0;j<256;j++)
      {
        pt++;
        value=(value+1)&0xFF;
        SlaveSend(ECHO);
        SlaveSend(value);
        //Using this line works fine
        //SlaveSend(value);
        //Using this line instead hangs
        SlaveSend(pt);
        v2=SlaveReceive();
      }
    }
  }
  for (;;
  return 0;
}

static void delay_ms(int ms)
{
  volatile int i;
  for (i=0;i<ms;i++) __delay_cycles(16000);
}

static void SlaveSend(unsigned int value)
{
  int i;
  P2OUT|=SLAVE_CLOCK;
  for (i=0;i<16;i++)
  {
    if (value&1) P2OUT|=SLAVE_DATA;
    else P2OUT&=~SLAVE_DATA;
    value/=2;
    __delay_cycles(SLAVE_WAIT);
    P2OUT&=~SLAVE_CLOCK;
    __delay_cycles(SLAVE_WAIT);
    P2OUT|=SLAVE_CLOCK;
  }
  P2OUT&=~SLAVE_CLOCK;
}

static unsigned int SlaveReceive()
{
  __delay_cycles(SLAVE_WAIT_SMALL);
  unsigned int i,value=0;
  P2DIR&=~(SLAVE_DATA+SLAVE_CLOCK);
  while (!(P2IN&SLAVE_CLOCK));
  for (i=0;i<16;i++)
  {
    while (P2IN&SLAVE_CLOCK);
    if (P2IN&SLAVE_DATA) value+=(1<<i);
    while (!(P2IN&SLAVE_CLOCK));
  }
  P2DIR|=(SLAVE_DATA+SLAVE_CLOCK);
  __delay_cycles(SLAVE_WAIT_SMALL);
  return value;
}

[pabigot 355]

pt is the variable in question. I thought the code for communicating between chips was at fault but it works fine when compiled without -Os. The problem seems to be when I use pt with the command SendSlave. If I use value (always 255 or less) it works. I think it is hanging in SlaveReceive. Is there a bug in the compiler?

 

EDIT: Same behavior with the latest GCC 4.6.3 and the experimental 4.7.0. Compiled like this:

msp430-gcc -std=gnu99 -g -Os -mmcu=msp430G2553 -o temp_src.elf temp_src.c

static unsigned int SlaveReceive()
{
  __delay_cycles(SLAVE_WAIT_SMALL);
  unsigned int i,value=0;
  P2DIR&=~(SLAVE_DATA+SLAVE_CLOCK);
  while (!(P2IN&SLAVE_CLOCK));
  for (i=0;i<16;i++)
  {
    while (P2IN&SLAVE_CLOCK);
    if (P2IN&SLAVE_DATA) value+=(1<<i);
    while (!(P2IN&SLAVE_CLOCK));
  }
  P2DIR|=(SLAVE_DATA+SLAVE_CLOCK);
  __delay_cycles(SLAVE_WAIT_SMALL);
  return value;
}

 

1 is an expression of type (signed) int, and shifting it 15 positions left overflows the representation of a signed int on the MSP430, producing undefined behavior allowing the compiler to do whatever it wants.

 

Try

    if (P2IN&SLAVE_DATA) value+=(1U<<i);

and see if that works better for you. Also look for other cases where the same problem occurs. In most situations of left shift you want the value-to-be-shifted to be unsigned.