Combining 2 bytes into an int in C

[Zener]

I found this code online which looks good to me but doesn't work:

Code: Select all

array[0] = 0xFB;
array[1] = 0x99;
int nValue = array[0] + (array[1] << 8);

However, if I do it this way it works fine:

Code: Select all

array[0] = 0xFB;
array[1] = 0x99;
int nValue = array[0] + (array[1] * 256);

Any ideas why?

I would think it would compile the same. No I didn't look at the disassembled code yet...

Thanks

[franklin97355]

What happens if you do

Code: Select all

(array[1] << 8) + array[0]

?

[adafruit_support_bill]

It is an operator precedence issue. '+' has higher precedence than '<<'. But '*' has higher precedence than '+'. You can use parentheses to enforce the order of execution.
http://en.cppreference.com/w/cpp/language/operator_precedence

[Zener]

Since I used parentheses, why does precedence matter?

[adafruit_support_bill]

Since I used parentheses, why does precedence matter?

Hmmm. So you did. Maybe a sign-bit issue? Shouldn't be. But I have never seen a C or C++ compiler without some sign-bit problems.

What results are you getting from each?

[philba]

Yes, it's a sign bit/typing problem. array was probably declared char. If you change it to unsigned char, the problems go away. It looks like a GCC bug to me but since the behavior is probably defined to be architecture dependent, the authors of GCC can claim it's a feature.

By the way, the more accepted way of doing that is with a bitwise or (which did not fix the problem without declaring array as unsigned).

Code: Select all

unsigned char array[2];
array[0] = 0xFB;
array[1] = 0x99;
int nValue = array[0] | (array[1] << 8);

[bearmos]

You can also use a union to provide access to the individual bytes, as well as the entire variable in a single shot:

Code: Select all

typedef union
{
   unsigned short us;
   struct
   {
      unsigned char hi;
      unsigned char lo;
   }bytes;
}  USHORT;

void main()
{
   USHORT myVar;
   unsigned char low, high;
   myVar.us = 0xff11;

   low = myVar.bytes.lo; //low contains 0x11
   high = myVar.bytes.hi; //high contains 0xFF

}


This provides really easy access to the entire variable as well as the individual bytes, but can cause headaches since it is endian dependent.

[philba]

Yeah, I posted something similar in an earlier version of that question but people seem to prefer shifting and oring.

[bearmos]

Yeah, I posted something similar in an earlier version of that question but people seem to prefer shifting and oring.

Oops. . .I must have missed the other topic - either that or I didn't read this one closely enough .

I imagine people are more comfortable with shifting and or'ing since it's common practice when accessing SFR's. Coupled with that, unions can be a little tricky to get used to for the uninitiated. Oh well - now there are multiple solutions on the thread. . .

[westfw]

Shifting and adding/oring is endian-explicit. Unions are implementation dependent.

There was a bug in some recent versions of avr-gcc WRT the normal byte assembly techniques...
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46779

[Zener]

Sorry for my slow response. My boss wants me to do actual work..

And I apologize for giving half the info to start with.

1st of all, "array" WAS declared as unsigned char, not char, so, so much for that theory.

I tried

Code: Select all

nValue = (array[1] << 8) + array[0];


Same result. And what I get either way is nValue = 0x007F. Go figure. (I should be getting 0x99FB)

As an experiment I changed array[0] and array[1] both to 0xFF. The result then is 0x0083. ????

I am using mplabc18 v3.38

Here is the disassembled code:

Code: Select all

208:               array[0] = 0xFF;
000A2    0100     MOVLB 0
000A4    6989     SETF 0x89, BANKED
209:               array[1] = 0xFF;
000A6    698A     SETF 0x8a, BANKED
210:               
211:               //nValue = array[0] + (array[1] << 8);
212:               nValue = (array[1] << 8) + array[0];
000A8    518A     MOVF 0x8a, W, BANKED
000AA    0E00     MOVLW 0
000AC    50F3     MOVF 0xff3, W, ACCESS
000AE    2589     ADDWF 0x89, W, BANKED
000B0    6F8B     MOVWF 0x8b, BANKED
000B2    6B8C     CLRF 0x8c, BANKED


Code: Select all

nValue = array[0] + (array[1] * 256);

gives this:

Code: Select all

208:               array[0] = 0xFF;
000A2    0100     MOVLB 0
000A4    6989     SETF 0x89, BANKED
209:               array[1] = 0xFF;
000A6    698A     SETF 0x8a, BANKED
210:               
211:               //nValue = array[0] + (array[1] << 8);
212:               //nValue = (array[1] << 8) + array[0];
213:                nValue = array[0] + (array[1] * 256);
000A8    518A     MOVF 0x8a, W, BANKED
000AA    6E00     MOVWF 0, ACCESS
000AC    6A01     CLRF 0x1, ACCESS
000AE    C000     MOVFF 0, 0x1
000B0    F001     NOP
000B2    6A00     CLRF 0, ACCESS
000B4    5189     MOVF 0x89, W, BANKED
000B6    2400     ADDWF 0, W, ACCESS
000B8    6F8B     MOVWF 0x8b, BANKED
000BA    0E00     MOVLW 0
000BC    2001     ADDWFC 0x1, W, ACCESS
000BE    6F8C     MOVWF 0x8c, BANKED

Also, using or instead of add didn't help either...

Thanks

[franklin97355]

Is it possible, c being as low level as it is, that shifting array[1] left 8 bits is actually shifting the data in the array variable and thus causing the error?

[Zener]

That is an interesting idea. I haven't tried it on non-array data. I will try, but it may be another week... I would like to understand the assembly but I can't really see how the assembly relates to my source code. I know some assembly but I can't really follow what is going on there. Thanks

[westfw]

Which CPU are you compiling for?

Try this. The theory is that the compiler is assuming that an 8-bit quantity left-shifted by 8 bits is always 0. (this is wrong, AFAIK. Values in expressions are always supposed to be promoted to "int" before calculations, if not otherwise specified. But that frequently results in bad code for 8bit quantities, so it's somewhat understandable that it could be done wrong, especially in a compiler with "optimizations disabled" (I assume you're using the free version of C18?))

Code: Select all

nValue = (((unsigned int)(array[1]) << 8) + array[0];

I tried this in MPLAB X and it seems to produced more correct-looking code (awful code, but more correct looking.)

Personally, I'd look for a different "free" or cheap compiler. The limitations that the Microchip-provided compilers insist upon are too ... limiting. I'd much rather have a 32k (or smaller) code limit than a "produces awful and obscure code"

BTW, unless you're aiming for specific endianness that is different from the default (and your example looks like the default, the usual hack is something like:

Code: Select all

nValue = *(int*)(&array[0]);

That says: even though this is a character array, I'm telling you that there are ints there, and you should go and get them!