View previous topic :: View next topic |
Author |
Message |
AlastairM
Joined: 28 Apr 2008 Posts: 28
|
efficient bit shifting |
Posted: Mon Sep 24, 2012 5:07 am |
|
|
Hi,
I have several 16 bit numbers where the top 14 bits are 2s complement numbers and the lower 2 bits are zeros.
I need to shift the useful bits to the correct place while maintaining 2s complement.
At the moment I do two 'right rotates' while stuffing the old msb into new msb.
i.e.
Code: | gyroZ=make16(i2c_read(1),i2c_read(0));
msb=bit_test(gyroZ,15);
shift_right(&gyroZ,2,msb);
shift_right(&gyroZ,2,msb); |
This looks messy to me!
Is there a better way? (I was thinking structs / unions...)
thanks
Al |
|
![](templates/subSilver/images/spacer.gif) |
Mike Walne
Joined: 19 Feb 2004 Posts: 1785 Location: Boston Spa UK
|
|
Posted: Mon Sep 24, 2012 6:15 am |
|
|
Could you use? Code: | gyroZ=make16(i2c_read(1),i2c_read(0));
gyroZ/=4;
| or even
Code: | gyroZ=(make16(i2c_read(1),i2c_read(0)))/4; |
Or does the compiler use several lines where one would do?
Mike |
|
![](templates/subSilver/images/spacer.gif) |
AlastairM
Joined: 28 Apr 2008 Posts: 28
|
|
Posted: Mon Sep 24, 2012 6:56 am |
|
|
That does it (with the addition of an explicit conversion)
Code: | accX=(signed int16)make16(i2c_read(1),i2c_read(1))/4; |
I got too tangled up in some existing code to see the obvious!
Thanks Mike - it's been a few years... |
|
![](templates/subSilver/images/spacer.gif) |
Ttelmah
Joined: 11 Mar 2010 Posts: 19359
|
|
Posted: Mon Sep 24, 2012 8:09 am |
|
|
Yes, the need to cast would be expected. The default output from make16, is an unsigned int16, so the /4 would lose the sign bit.
Best Wishes |
|
![](templates/subSilver/images/spacer.gif) |
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Mon Sep 24, 2012 11:30 am |
|
|
Quote: | gyroZ=make16(i2c_read(1), i2c_read(0) );
|
This is unsafe code. The two i2c_read()'s must be in the correct sequence
to properly read the i2c slave chip. But the C language doesn't specify
the order of evaluation of function parameters. It can be left-to-right or
right-to-left.
The correct way to do it would be to load two intermediate variables such
as 'msb' and 'lsb' with the values from i2c_read(). Then do a make16
on those variables. |
|
![](templates/subSilver/images/spacer.gif) |
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Tue Sep 25, 2012 12:20 am |
|
|
Quote: | This is unsafe code. |
Yes. But it's neither portable code due to make16 usage.
So if you are sure about the evaluation order with CCS C, you may use the construct though. |
|
![](templates/subSilver/images/spacer.gif) |
AlastairM
Joined: 28 Apr 2008 Posts: 28
|
|
Posted: Tue Sep 25, 2012 7:53 am |
|
|
It works as intended - but I'll change it to have some intermediates in case CCS changes it in the future.
I hadn't appreciated the function evaluation order wasn't necessarily left to right!
Thanks for your help.
Al |
|
![](templates/subSilver/images/spacer.gif) |
Ttelmah
Joined: 11 Mar 2010 Posts: 19359
|
|
Posted: Tue Sep 25, 2012 8:20 am |
|
|
It is one of the hidden 'caveats' of C.....
"The order of evaluation of expressions is, with certain exceptions undefined,
even if the subexpressions involve side effects. That is, unless the definition of an operator guarantees that its operands are evaluated in a particular order,
the implementation is free to evaluate operands in any order, or even
interleave their evaluation."
From K&R second edition.
Inside a function line, you can force order using brackets, and there are rules for the order of things like maths operators, but none for functions used as variables to another function. It is particularly fun, when running multi-threaded with functions whose results depend on the order they are called.
Best Wishes |
|
![](templates/subSilver/images/spacer.gif) |
asmboy
Joined: 20 Nov 2007 Posts: 2128 Location: albany ny
|
|
Posted: Wed Sep 26, 2012 1:41 pm |
|
|
I guess I'm a lucky guy, to have learned this early on my own dime.
I got snagged by being to a bit too clever in an early project, while
sending a 16 bit quantity to an 8 bit port requiring a select bit for high and low. Could not figure out what I was doing wrong.
I really scrambled my eggs until I explicitly used two intermediate variables to do the deed. |
|
![](templates/subSilver/images/spacer.gif) |
|