C++ How to combine two signed 8 Bit numbers to a 16 Bit short? Unexplainable results

Question

I need to combine two signed 8 Bit _int8 values to a signed short (16 Bit) value. It is important that the sign is not lost.

My code is:

 unsigned short lsb = -13;
 unsigned short msb = 1;
 short combined = (msb << 8 )| lsb;

The result I get is -13. However, I expect it to be 499.

For the following examples, I get the correct results with the same code:

msb = -1; lsb = -6; combined = -6;
msb = 1; lsb = 89; combined = 345; 
msb = -1; lsb = 13; combined = -243;

However, msb = 1; lsb = -84; combined = -84; where I would expect 428.

It seems that if the lsb is negative and the msb is positive, something goes wrong! What is wrong with my code? How does the computer get to these unexpected results (Win7, 64 Bit and VS2008 C++)?

Answer 1

Your lsb in this case contains 0xfff3. When you OR it with 1 << 8 nothing changes because there is already a 1 in that bit position.

Try short combined = (msb << 8 ) | (lsb & 0xff);

Answer 2

You wrote, that you need to combine two 8-bit values. Why you're using unsigned short then? As Dan already said, lsb automatically extended to 16 bits. Try the following code:

uint8_t lsb = -13;
uint8_t msb = 1;
int16_t combined = (msb << 8) | lsb;

This gives you the expected result: 499.

Answer 3

Or using a union:

#include <iostream>

union Combine
{
    short target;
    char dest[ sizeof( short ) ];
};

int main()
{
    Combine cc;
    cc.dest[0] = -13, cc.dest[1] = 1;
    std::cout << cc.target << std::endl;
}

Answer 4

It is possible that lsb is being automatically sign-extended to 16 bits. I notice you only have a problem when it is negative and msb is positive, and that is what you would expect to happen given the way you're using the or operator. Although, you're clearly doing something very strange here. What are you actually trying to do here?

Answer 5

Raisonanse C complier for STM8 (and, possibly, many other compilers) generates ugly code for classic C code when writing 16-bit variables into 8-bit hardware registers. Note - STM8 is big-endian, for little-endian CPUs code must be slightly modified. Read/Write byte order is important too.

So, standard C code piece:

 unsigned int ch1Sum;
...
     TIM5_CCR1H = ch1Sum >> 8; 
     TIM5_CCR1L = ch1Sum; 

Is being compiled to:

;TIM5_CCR1H = ch1Sum >> 8; 
         LDW   X,ch1Sum 
         CLR   A 
         RRWA  X,A 
         LD    A,XL 
         LD    TIM5_CCR1,A 
;TIM5_CCR1L = ch1Sum; 
         MOV   TIM5_CCR1+1,ch1Sum+1 

Too long, too slow.

My version:

     unsigned int ch1Sum;
...
     TIM5_CCR1H = ((u8*)&ch1Sum)[0];
     TIM5_CCR1L = ch1Sum;

That is compiled into adequate two MOVes

;TIM5_CCR1H = ((u8*)&ch1Sum)[0]; 
       MOV   TIM5_CCR1,ch1Sum 
;TIM5_CCR1L = ch1Sum;
       MOV   TIM5_CCR1+1,ch1Sum+1 

Opposite direction:

    unsigned int uSonicRange;
...
      ((unsigned char *)&uSonicRange)[0] = TIM1_CCR2H;
      ((unsigned char *)&uSonicRange)[1] = TIM1_CCR2L;

instead of

    unsigned int uSonicRange;
...
      uSonicRange = TIM1_CCR2H << 8;
      uSonicRange |= TIM1_CCR2L;