## Simple C code inefficient and working

12 posts / 0 new
Author
Message

I have the simplest code to average two 8bit signed numbers, but it is not working, temp1 and temp3 are declared as uin8_t:

```temp1=(uint8_t)((temp1-0x80)>>1)+(((uint8_t)temp3-0x80)>>1);
temp1=temp1-0x80;```

which translates to:

```369:temp1=(uint8_t)((temp1-0x80)>>1)+(((uint8_t)temp3-0x80)>>1);
MOV       R24,R7         Copy register
SUBI      R24,0x80       Subtract immediate
SBCI      R25,0x00       Subtract immediate with carry
ASR       R25            Arithmetic shift right
ROR       R24            Rotate right through carry
MOV       R7,R27         Copy register
370:temp1=temp1-0x80;
MOVW      R30,R12        Copy register pair
LDD       R24,Z+0        Load indirect with displacement
SUBI      R24,0x80       Subtract immediate
SBCI      R25,0x00       Subtract immediate with carry
ASR       R25            Arithmetic shift right
ROR       R24            Rotate right through carry

But what I really want the compiler to do is this:

```subi R24, 0x80  ; Transform [0x80, 0x7f] -> [0x00, 0xff]
lsr  R24        ; Divide by 2
subi R22, 0x80  ; Transform [0x80, 0x7f] -> [0x00, 0xff]
lsr  r22        ; Divide by 2
subi R24, 0x80  ; Transform [0x00, 0xff] -> [0x80, 0x7f]```

Besides generating inefficient code, the C code also gives bad results with certain values, like adding -1 and 0.
About the averaging method itself, I know I am discarding the LSB from both operands, but this is not important.

Quote:
How can you have an number like -1 if both temp1 and temp3 are defined as unsigned integers?

Regards,
Steve A.

The Board helps those that help themselves.

Koshchi wrote:
Quote:
How can you have an number like -1 if both temp1 and temp3 are defined as unsigned integers?
The temp variables get their values from signed variables, but internally, the values are still represented the same: -1 = 0xFF, 0=0x00

But the math on them will NOT be the same. For the example you gave of -1, (-1 - 0x80) >> 1 will give a result of 0x3F (63). If temp1 was a signed number, the result would be 0xBF (-65). The compiler is obligated to extend the values to 16 bit before doing the math, so 0xFF becomes 0xFFFF when signed and 0x00FF when unsigned.

Regards,
Steve A.

The Board helps those that help themselves.

Post a complete compilable example exhibiting the problem, together with the command line used to compile it and avr-gcc version.

JW

Quote:

The compiler is obligated to extend the values to 16 bit before doing the math,

Perhaps that could be sidestepped?

```uint8_t frog;
...
frog = 0x80;

temp1 -= frog;
temp1 >>= 1;
temp1 += (temp3 - frog)>>1;

temp1 -= frog;
...
```

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Koshchi wrote:
The compiler is obligated to extend the values to 16 bit before doing the math.
But wouldn't this be taken care off by using explicit casts? (The very thing I am trying to do).
And also, why would the second operation be also extended to 16bit?
`temp1=temp1-0x80;`

Quote:

But wouldn't this be taken care off by using explicit casts? (The very thing I am trying to do).
And also, why would the second operation be also extended to 16bit?

I'm pretty sure it is the constant that is extended to "int" by rule.

Thus my suggestion to have a dummy variable with the 8-bit constant. I'd speculate that the compiler will never even actually create a "frog" address, but that may depend on the rest of the code around the sequence.

Lee

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

`(uint8_t)((temp1-0x80)>>1)`

The only cast you are doing here is AFTER the math. And as Lee pointed out, 0x80 is signed, so you would need to explicitly cast that. But if these are signed values, then why on Earth are you using unsigned variables to hold them? And what is the purpose of subtracting 0x80 in the first place?

Quote:
And also, why would the second operation be also extended to 16bit?
Because that is what the C standard says must happen.

Regards,
Steve A.

The Board helps those that help themselves.

Thanks for helping me find the solution!
By moving the cast around I got the correct result:

```temp1=((uint8_t)(temp1-0x80)>>1)+((uint8_t)(temp3-0x80)>>1);
temp1=temp1-0x80;```

But I just realized that the best way to achieve the desired result is this:

`temp1=((int8_t)(temp1)>>1)+((int8_t)(temp3)>>1); `

Which translates directly into two arithmetic shifts, and an addition. (I was under the wrong impression that a shift was always a logical shift).

Quote:
But I just realized that the best way to achieve the desired result is this:
But again, wouldn't it be so much easier to use signed ints in the first place?

Regards,
Steve A.

The Board helps those that help themselves.