avr-gcc 4.3.2 emitting obsolete instructions...

Go To Last Post
14 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi folks,

In the process of adding rounding to divisions performed using right-shifting, I've looked into the generated assembler code.. I've found something strange:

	uint16_t t16 = prvMonitoring.Pressure;
	int32_t pressure = t16;
	pressure -= prvPressureConfigCurrent.Config.Zero;
	return pressure >> 16;

generates the following assembler code:

int16_t xPressureGet()
{
	uint16_t t16 = prvMonitoring.Pressure;
     9be:	20 91 ec 01 	lds	r18, 0x01EC
     9c2:	30 91 ed 01 	lds	r19, 0x01ED
	int32_t pressure = t16;
     9c6:	40 e0       	ldi	r20, 0x00	; 0
     9c8:	50 e0       	ldi	r21, 0x00	; 0
	pressure -= prvPressureConfigCurrent.Config.Zero;
     9ca:	80 91 f4 01 	lds	r24, 0x01F4
     9ce:	90 91 f5 01 	lds	r25, 0x01F5
     9d2:	a0 e0       	ldi	r26, 0x00	; 0
     9d4:	b0 e0       	ldi	r27, 0x00	; 0
     9d6:	28 1b       	sub	r18, r24
     9d8:	39 0b       	sbc	r19, r25
     9da:	4a 0b       	sbc	r20, r26
     9dc:	5b 0b       	sbc	r21, r27
     9de:	9a 01       	movw	r18, r20
     9e0:	55 27       	eor	r21, r21
     9e2:	37 fd       	sbrc	r19, 7
     9e4:	50 95       	com	r21
     9e6:	45 2f       	mov	r20, r21
	return pressure >> 16;
     9e8:	c9 01       	movw	r24, r18
     9ea:	08 95       	ret
}

The instructions between 9e0 and 9e8 have no effect on the returned value in r18/19 whatsoever, so why are they generated? they change the value of r20/21 to 0xffff when pressure < 0, but for what reason?
If I remove ">>16", they are not generated.

cheers..
hOOver

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Because pressure is a 32 bit value, four bytes will be needed to hold it. If a four byte value is negative then the high bit(s) are 1 (two's complement) so if you right shift such a value 16 positions then thwe two upper bytes will be all ones. In the code snippet you posted this is the last computation bfore returning the number, so the setting of the two upper registers to all ones is ot needed as you only return the two lower bytes. The optimizer does not spot this, obviously.

If you remove the right shift, then there is no need to set the upper bits to all ones.

Not sure of your knowledge of binary representation of numeric values. If the above does not make sense, then some light will b shed if you read about "two's complement" on Wikipedia.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm completely aware of how 2's complement works :-)

Now that you enlightened me, I see what's going on, I just didn't realize it before because I never in a million years would have imagined that the compiler would emit code for that, when it's not needed.
If I had written

pressure >>= 16;
return pressure;

it might have been ok, but this I never suspected.. it's 2010 for cryin' out loud :-)

anyway, since I'm doing the rounding in inline assembler anyway (not included in above example), is there a way to also set the return value (r24/25) using inline assembler as well? I suspect not, since the compiler will require me to have a "return" statement in the function...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

hooverphonique wrote:
is there a way to also set the return value (r24/25) using inline assembler as well? I suspect not, since the compiler will require me to have a "return" statement in the function...

Specify the output of the inline assembler as a 16-bit variable which you than pass to the return statement.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Is it any better if the variable is an uint32_t instead of int32_t? Does it then return the high word only?

Also you could set up a union which has both uint32_t and two uint16_t variables so you can address x.long=something and then return x.high

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

TimothyEBaldwin wrote:
hooverphonique wrote:
is there a way to also set the return value (r24/25) using inline assembler as well? I suspect not, since the compiler will require me to have a "return" statement in the function...

Specify the output of the inline assembler as a 16-bit variable which you than pass to the return statement.

That's a way to do it, of course, but it will almost surely cause some movw's to be generated by the compiler at the end of the function. Think I may do it that way though, thanks..

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Jepael wrote:
Is it any better if the variable is an uint32_t instead of int32_t? Does it then return the high word only?

that's actually worth a try.. the int32 value is multiplied by a uint16 before shifting/rounding, but it shouldn't matter if one multiplicand data type is signed or not, as long as the other is unsigned.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

it's 2010 for cryin' out loud

Then don't mix signed and unsigned. If it is not signed work going on, then don't use a signed var and >>force the compiler by rule of law<< to do the sign propagation.

Didja try a simple cast to give the compiler an alibi?

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.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Jepael wrote:
Is it any better if the variable is an uint32_t instead of int32_t? Does it then return the high word only?

using an uint32_t makes it slightly better, but code is still emitted for clearing the (later discarded) high bytes.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:
Quote:

it's 2010 for cryin' out loud

Then don't mix signed and unsigned. If it is not signed work going on, then don't use a signed var and >>force the compiler by rule of law<< to do the sign propagation.

Didja try a simple cast to give the compiler an alibi?

hmm.. well, the 32 bit number *is* signed, but it should work with an unsigned datatype as well. No matter which way round I do it, the compiler still makes it a full fledged 32 bit number before truncating at the return statement..

a cast makes no difference.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Then the easiest way is the union trick so you can directly return the high part without >>16.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You must really call this function a lot if a couple of instructions in a corner of C are causing a problem.

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.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

hehe... no, not really.. just curious.. initially it was actually because of general space concerns, not speed.

I guess being an ex 68k assembler demo coder (Amiga) must have an effect here :wink:

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

hooverphonique wrote:
That's a way to do it, of course, but it will almost surely cause some movw's to be generated by the compiler at the end of the function. Think I may do it that way though, thanks..

That's surprising, workaround is to use an explicit register variable, for example:

int test() {
  register int out asm("r24");
  asm ("ldi %A0, 1\n\tldi %B0, 2" : "=r"(out));
  return out;
}