avr-gcc 4.3.2 emitting obsolete instructions...

Log in or register to post comments
Go To Last Post
14 posts / 0 new
Author
Message
hooverphonique
hooverphonique's picture
Level: Hangaround
Joined: Fri. Aug 29, 2008
Posts: 120 View posts
Location: Fyn, Denmark
#1
Posted by hooverphonique: Thu. Feb 11, 2010 - 01:44 PM
  • 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

Tags:

Tools, Compilers and General Programming
Top
JohanEkdahl
JohanEkdahl's picture
Level: 10k+ Postman
Joined: Wed. Mar 27, 2002
Posts: 26988 View posts
Location: Lund, Sweden
Posted by JohanEkdahl: Thu. Feb 11, 2010 - 02:15 PM
  • 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]

Top
hooverphonique
hooverphonique's picture
Level: Hangaround
Joined: Fri. Aug 29, 2008
Posts: 120 View posts
Location: Fyn, Denmark
Posted by hooverphonique: Thu. Feb 11, 2010 - 08:11 PM
  • 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...

Top
TimothyEBaldwin
TimothyEBaldwin's picture
Level: Hangaround
Joined: Tue. Aug 26, 2008
Posts: 226 View posts
Posted by TimothyEBaldwin: Thu. Feb 11, 2010 - 09:16 PM
  • 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.

Top
Jepael
Jepael's picture
Level: Raving Lunatic
Joined: Mon. May 24, 2004
Posts: 6279 View posts
Location: Tampere, Finland
Posted by Jepael: Thu. Feb 11, 2010 - 09:40 PM
  • 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

Top
hooverphonique
hooverphonique's picture
Level: Hangaround
Joined: Fri. Aug 29, 2008
Posts: 120 View posts
Location: Fyn, Denmark
Posted by hooverphonique: Thu. Feb 11, 2010 - 09:50 PM
  • 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..

Top
hooverphonique
hooverphonique's picture
Level: Hangaround
Joined: Fri. Aug 29, 2008
Posts: 120 View posts
Location: Fyn, Denmark
Posted by hooverphonique: Thu. Feb 11, 2010 - 10:06 PM
  • 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.

Top
theusch
theusch's picture
Level: 10k+ Postman
Joined: Mon. Feb 19, 2001
Posts: 39770 View posts
Location: Wisconsin USA
Posted by theusch: Thu. Feb 11, 2010 - 10:31 PM
  • 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.

Top
hooverphonique
hooverphonique's picture
Level: Hangaround
Joined: Fri. Aug 29, 2008
Posts: 120 View posts
Location: Fyn, Denmark
Posted by hooverphonique: Fri. Feb 12, 2010 - 02:06 PM
  • 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.

Top
hooverphonique
hooverphonique's picture
Level: Hangaround
Joined: Fri. Aug 29, 2008
Posts: 120 View posts
Location: Fyn, Denmark
Posted by hooverphonique: Fri. Feb 12, 2010 - 02:09 PM
  • 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.

Top
Jepael
Jepael's picture
Level: Raving Lunatic
Joined: Mon. May 24, 2004
Posts: 6279 View posts
Location: Tampere, Finland
Posted by Jepael: Fri. Feb 12, 2010 - 02:11 PM
  • 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.

Top
theusch
theusch's picture
Level: 10k+ Postman
Joined: Mon. Feb 19, 2001
Posts: 39770 View posts
Location: Wisconsin USA
Posted by theusch: Fri. Feb 12, 2010 - 03:56 PM
  • 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.

Top
hooverphonique
hooverphonique's picture
Level: Hangaround
Joined: Fri. Aug 29, 2008
Posts: 120 View posts
Location: Fyn, Denmark
Posted by hooverphonique: Tue. Feb 16, 2010 - 12:53 PM
  • 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:

Top
TimothyEBaldwin
TimothyEBaldwin's picture
Level: Hangaround
Joined: Tue. Aug 26, 2008
Posts: 226 View posts
Posted by TimothyEBaldwin: Sat. Feb 20, 2010 - 09:18 AM
  • 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;
}
Top