View previous topic :: View next topic |
Author |
Message |
evsource
Joined: 21 Nov 2006 Posts: 129
|
Bit shifting by 12 produces error |
Posted: Thu Oct 09, 2008 10:25 am |
|
|
Hi everyone,
I'm making a long (16 bits) out of 4 nibbles (4 bits each). I receive 4 bytes in a communications packet, and only the lower 4 bits of each byte contains useful information. I assemble these 4 nibbles into a long. Here's how I was initially trying to do it:
Code: | command_on_line = ( (input_buffer[2] << 12) | (input_buffer[3] << 8) | (input_buffer[4] << 4) | input_buffer[5] ); |
But the upper byte was just 0x00. I was suspecting that shifting by 12 was shifting it into no-mans land. So I tried this approach:
Code: | command_on_line = make16((input_buffer[2] << 4) | input_buffer[3], (input_buffer[4] << 4) | input_buffer[5]); |
The first approach seems more elegant, and I'm just wondering how to make that work. I did try casting like this, but it didn't work either:
Code: | command_on_line = ( (long)(input_buffer[2] << 12) | (long)(input_buffer[3] << 8) | (input_buffer[4] << 4) | input_buffer[5] ); |
Thanks for any suggestions. |
|
|
future
Joined: 14 May 2004 Posts: 330
|
|
Posted: Thu Oct 09, 2008 10:27 am |
|
|
try casting your buffer bytes to long before the shift
(long)input_buffer[2]<<12 |
|
|
evsource
Joined: 21 Nov 2006 Posts: 129
|
|
Posted: Thu Oct 09, 2008 11:06 am |
|
|
future wrote: | try casting your buffer bytes to long before the shift
(long)input_buffer[2]<<12 |
That was too easy! Why didn't I think of that?!
Okay, so which one is more efficient?
Code: | command_on_line = make16((input_buffer[2] << 4) | input_buffer[3], (input_buffer[4] << 4) | input_buffer[5]); |
or ...
Code: | command_on_line = ( ((long)input_buffer[2] << 12) | ((long)input_buffer[3] << 8) | (input_buffer[4] << 4) | input_buffer[5] ); |
|
|
|
ckielstra
Joined: 18 Mar 2004 Posts: 3680 Location: The Netherlands
|
|
Posted: Thu Oct 09, 2008 2:53 pm |
|
|
Quote: | Okay, so which one is more efficient? | Have a look at the generated code in the list file for both versions and you will know.
Sorry, but this is too easy. You can find the answer yourself quicker than the time it took to post the question. Even when you don't understand assembly code it is just a matter of counting lines. |
|
|
evsource
Joined: 21 Nov 2006 Posts: 129
|
|
Posted: Thu Oct 09, 2008 3:03 pm |
|
|
ckielstra wrote: | Quote: | Okay, so which one is more efficient? | Have a look at the generated code in the list file for both versions and you will know.
Sorry, but this is too easy. You can find the answer yourself quicker than the time it took to post the question. Even when you don't understand assembly code it is just a matter of counting lines. |
Hey, thanks ckielstra, appreciate the help.
You know, I didn't have to post this topic at all, you see I had a workaround figured out. But I did post it since I figured someone else might bump into the same problem I had, and would benefit from finding it in the forum.
I hate it when people respond with something to the effect "you dummy, why don't you just go figure it out for yourself" ... hope that boosts your ego.
What would have been more helpful is if you could give some insight into *why* one is more efficient than the other, which could be applied to other coding situations. That's worth a lot more than looking at the list file every time you want to know what coding method is better. |
|
|
RayJones
Joined: 20 Aug 2008 Posts: 30 Location: Melbourne, Australia
|
|
Posted: Thu Oct 09, 2008 3:15 pm |
|
|
To increase efficiency, forget about performing bit shifts more than 8 bits, especially if you don't need to preserve the carrys.
The PIC can only ever read/write 8 bit values.
16 bit values are a logical construction by the compiler so there is a high 8 bits and a low 8 bits in consecutive memory locations.
For your intended purpose, you do not need to worry about rolling the carry bits through.
You could instead do something like the following using a union to emulate 2 8 bit bytes overlaying a 16 bit word:
Code: |
typedef union {
struct Bytes {
int8 lsb;
int8 msb;
}
int16 word;
} Val16;
Val16 Value;
Value.Bytes.msb = (input_buffer[2] << 4) | (input_buffer[3] & 0x0f);
Value.Bytes.lsb = (input_buffer[4] << 4) | (input_buffer[5] & 0x0f);
|
Your desired answer will then be in Value.word
Note that despite the apparent complexity of using structures/unions, the compiler will optimise these operations far better than any attempt at performing 16 bit shifts.
The compiler does a really good job actually.
It is also always worthwhile to perform the bit mask of the low bits to ensure no rogue bits upset your bitwise OR operation.
Ray |
|
|
|