WinAVR 20081205 optimization behavior

Log in or register to post comments
Go To Last Post
10 posts / 0 new
Author
Message
Mike B
Mike B's picture
Level: Raving Lunatic
Joined: Tue. Jun 22, 2004
Posts: 3839 View posts
Location: South West Utah, USA
#1
Posted by Mike B: Thu. Feb 5, 2009 - 10:38 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

WinAVR 20081205, AVRStudio 4.15.623, Optimization level -Os

from avr/iocanxx.h:
/* RegDef: CAN MOb Status Register*/
#define CANSTMOB _SFR_MEM8(0xEE)

The optimizer fails on all of these in ISR(CANIT_vect):

        if ((CANSTMOB & ((1 << BERR) | (1 << SERR) | (1 << CERR) | (1 << FERR) | (1 << AERR))) != 0) {
        if (CANSTMOB & ((1 << BERR) | (1 << SERR) | (1 << CERR) | (1 << FERR) | (1 << AERR))) {
        if (CANSTMOB & (0x1F)) {
        if (CANSTMOB & 0x1F) {
 ca2:	80 91 ee 00 	lds	r24, 0x00EE
 ca6:	90 e0       	ldi	r25, 0x00	; 0
 ca8:	8f 71       	andi	r24, 0x1F	; 31
 caa:	90 70       	andi	r25, 0x00	; 0
 cac:	89 2b       	or	r24, r25
 cae:	a1 f1       	breq	.+104    	; 0xd18 <__vector_18+0xe0>

Tags:

Tools, Compilers and General Programming
Top
Mike B
Mike B's picture
Level: Raving Lunatic
Joined: Tue. Jun 22, 2004
Posts: 3839 View posts
Location: South West Utah, USA
Posted by Mike B: Thu. Feb 5, 2009 - 10:52 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Now these optimize correctly:

         if (CANSTMOB & ((1 << BERR) | (1 << SERR) | (1 << CERR) | (1 << FERR) | (1 << TXOK))) {
 ca2:	80 91 ee 00 	lds	r24, 0x00EE
 ca6:	8e 75       	andi	r24, 0x5E	; 94
 ca8:	a1 f1       	breq	.+104    	; 0xd12 <__vector_18+0xda>

        if ((CANSTMOB & ((1 << BERR) | (1 << SERR) | (1 << CERR) | (1 << AERR))) != 0) {
 ca2:	80 91 ee 00 	lds	r24, 0x00EE
 ca6:	8d 71       	andi	r24, 0x1D	; 29
 ca8:	a1 f1       	breq	.+104    	; 0xd12 <__vector_18+0xda>

I checked and if (CANSTMOB & 0x0F) also failed, but if (CANSTMOB & 0xF1) worked. What is going on here, what does the optimizer not like about all the lower nibble bits being set in a constant?

Sorry, AVRfreaks refused to take this post in one piece. I was forced to split it up.

Top
curtvm
curtvm's picture
Level: Raving Lunatic
Joined: Wed. Sep 21, 2005
Posts: 2958 View posts
Posted by curtvm: Thu. Feb 5, 2009 - 11:38 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

0x03,0x07,0x0F,0x1F,0x3F,0x7F

You may have to dig into the source code to find out.

Top
Mike B
Mike B's picture
Level: Raving Lunatic
Joined: Tue. Jun 22, 2004
Posts: 3839 View posts
Location: South West Utah, USA
Posted by Mike B: Sat. Feb 7, 2009 - 04:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I don't even have the WinAVR source downloaded. I am curious if anyone else can confirm this as a problem.

The code produced works in all cases, even when the optimization doesn't work. When the optimization fails it only wastes 3 AVR execution cycles. So, if it is an optimizer bug, its a very, very low priority one. The only reason I even found it was while examining some execution speed critical ISR code for wasted CPU cycles.

Top
curtvm
curtvm's picture
Level: Raving Lunatic
Joined: Wed. Sep 21, 2005
Posts: 2958 View posts
Posted by curtvm: Sat. Feb 7, 2009 - 09:39 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
I don't even have the WinAVR source downloaded
Gcc source is what you would want (there really is no 'WinAVR' source). Even if you think its not needed/wanted, I would download the gcc, binutils, and libc source code. The libc source code is readable (understandable) and there is things one can learn by looking at the source, gcc/binutils is on another level- it seems I'm about 20 years behind in the language they use. Even though it looks like a foreign language (to me), there still is some info in there that turns on a few light bulbs in the brain (well, more like tiny Christmas lights).

The numbers I posted above also duplicated the 'problem' in several WinAVR versions (I think they were all using gcc 4.* if I remember correctly). It does seem like an obvious pattern (the numbers where it doesn't optimize 'correctly'), but I would have no idea where to look in the source code.

You are right, in the big scheme of things its not a high priority problem. Just put it on your list, and if you ever need a few bytes to make your code fit, you know where to get them. There is a way to persuade the compiler to think 'clearly' in this case-

uint8_t temp=0x1F; //or whatever your bits wanted
if (CANSTMOB & temp){

in this case on those 'weird' numbers, it seems the compiler now 'knows' the number is only a byte, where the original code the compiler somehow gets 'confused' and turns it into a 2 byte number.

Top
Mike B
Mike B's picture
Level: Raving Lunatic
Joined: Tue. Jun 22, 2004
Posts: 3839 View posts
Location: South West Utah, USA
Posted by Mike B: Sun. Feb 8, 2009 - 06:57 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Typecasting the constant to (uint8_t) didn't work. Ironically the optimizer did an excellent job using the temp variable to replace the constant. It didn't allocate any other/new register storage, so the ISR code didn't push/pop another register. Unexpectedly, the temp variable turns out to be a direct replacement for what was supposed to happen with just a constant. Thanks for the tip.

Top
curtvm
curtvm's picture
Level: Raving Lunatic
Joined: Wed. Sep 21, 2005
Posts: 2958 View posts
Posted by curtvm: Mon. Feb 9, 2009 - 06:38 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Anybody know where in the gcc source code one would start looking for this 'problem'?-

#include 
//gcc 4.3.2
//-O1 or greater
int main(){

    //constant treated as an (unsigned?) int
    //(or PINB was promoted)
    //0x03,0x07,0x0F,0x1F,0x3F,0x7F
    if(PINB&0x07){
        PORTB=0;
    }

    //workaround
    uint8_t temp=0x07;
    if(PINB&temp){
        PORTB=0;
    }

    //'anything else'optimizes correctly
    if(PINB&0x81){
        PORTB=0;
    }

    //PINB promoted to an (unsigned?) int
    //0x?01,0x?02,0x?03*,0x?04,0x?07*,0x?08,0x?0F*,0x?10,
    //0x?1F*,0x?20,0x?3F*,0x?40,0x?7F*,0x?80
    //* same lower bits as above (including 0x01 since it is
    //not a single bit now), and that number +1
    if(PINB&0x017F){
        PORTB=0;
    }
    //also same for 32bits (PINB promoted)
    if(PINB&0x0001007F){
        PORTB=0;
    }

    //here it doesn't (andi 0x7E)
    //(all the rest seem to optimize corectly)
    if(PINB&0x17E){
        PORTB=0;
    }
    if(PINB&0x0001007E){
        PORTB=0;
    }

}

It seems for some reason, depending on the constants, there is some promoting of types going on. It would seem that PINB is the one getting promoted to a larger type in these cases (I deduced that only because the bitwise ops for the 16/32bit constants still optimize to a single andi). The optimization is -O1, so its somewhere in those options, I guess. I looked around a bit, but I am lost. I can't even find where these constants are being checked for single bits (sbis/sbic), but it does seem to have something to do with the checking of trailing zeros or something. Its not terribly important, but it would still be nice to point to a piece of code and say 'aha! there it is!'.

(I also tried turning on some debug options, like -dP,-save-temps, but all the stuff I see in the .s file is after the wrong type promoting has already been made)

Top
curtvm
curtvm's picture
Level: Raving Lunatic
Joined: Wed. Sep 21, 2005
Posts: 2958 View posts
Posted by curtvm: Wed. Feb 11, 2009 - 04:24 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I think I found the file where the 'problem' is- combine.c, but after looking at it I think I'm still no closer (looks all Greek/Japanese to me). Nothing like finding a needle in the haystack, then realizing there is another haystack inside that needle.

They have a lot of type promoting/restoring going on (or whatever they're doing), and somehow on specific constants the PINB in the example stays 'promoted'.

Top
EW
EW's picture
Level: Raving Lunatic
Joined: Thu. Mar 1, 2001
Posts: 4934 View posts
Location: Rocky Mountains
Posted by EW: Wed. Feb 11, 2009 - 05:47 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

curtvm wrote:
I think I found the file where the 'problem' is- combine.c, but after looking at it I think I'm still no closer (looks all Greek/Japanese to me). Nothing like finding a needle in the haystack, then realizing there is another haystack inside that needle.

I like that comparison. :) Welcome to the wonderful world of GCC. Now you begin to understand why it can be difficult to change things within GCC.

Eric Weddington
www.linkedin.com/in/ericweddington/

Top
dmitrik
dmitrik's picture
Level: New Member
Joined: Sat. Feb 14, 2009
Posts: 1 View posts
Posted by dmitrik: Sat. Feb 14, 2009 - 08:40 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Looks like the optimizer is just too smart for its own good.

#include 
uint8_t foo()
{
  uint8_t val = 0;
  if (bit_is_set(PINA,PA5)) val |=1;
  if (bit_is_set(PINA,PA0)) val |=2;
  return val;
}

I would expect something like that:

foo:
  clr  r24
  sbic 0x19,5
  ori  r24,1
  sbic 0x19,0
  ori  r24,2
  ret

Compiled with WinAVR-20081205 -Os -mmcu=atmega32

00000000 :
   0:	89 b3       	in	r24, 0x19	; 25
   2:	90 e0       	ldi	r25, 0x00	; 0
   4:	25 e0       	ldi	r18, 0x05	; 5
   6:	96 95       	lsr	r25
   8:	87 95       	ror	r24
   a:	2a 95       	dec	r18
   c:	01 f4       	brne	.+0      	; 0xe 
			c: R_AVR_7_PCREL	.text+0x6
   e:	81 70       	andi	r24, 0x01	; 1
  10:	c8 99       	sbic	0x19, 0	; 25
  12:	82 60       	ori	r24, 0x02	; 2
  14:	08 95       	ret

A loop! A freaking loop!
Funnily enough it uses sbic for the second if() but not for the first.

Copmiled with -O3 -mmcu=atmega32

00000000 :
   0:	89 b3       	in	r24, 0x19	; 25
   2:	90 e0       	ldi	r25, 0x00	; 0
   4:	96 95       	lsr	r25
   6:	87 95       	ror	r24
   8:	92 95       	swap	r25
   a:	82 95       	swap	r24
   c:	8f 70       	andi	r24, 0x0F	; 15
   e:	89 27       	eor	r24, r25
  10:	9f 70       	andi	r25, 0x0F	; 15
  12:	89 27       	eor	r24, r25
  14:	81 70       	andi	r24, 0x01	; 1
  16:	c8 99       	sbic	0x19, 0	; 25
  18:	82 60       	ori	r24, 0x02	; 2
  1a:	08 95       	ret

If I swap the order of the 2 if's around it gets even more bizzare:

With -Os:

00000000 :
   0:	c8 9b       	sbis	0x19, 0	; 25
   2:	00 c0       	rjmp	.+0      	; 0x4 
			2: R_AVR_13_PCREL	.text+0x8
   4:	82 e0       	ldi	r24, 0x02	; 2
   6:	00 c0       	rjmp	.+0      	; 0x8 
			6: R_AVR_13_PCREL	.text+0xa
   8:	80 e0       	ldi	r24, 0x00	; 0
   a:	cd 99       	sbic	0x19, 5	; 25
   c:	81 60       	ori	r24, 0x01	; 1
   e:	08 95       	ret

With -03:

00000000 :
   0:	c8 9b       	sbis	0x19, 0	; 25
   2:	00 c0       	rjmp	.+0      	; 0x4 
			2: R_AVR_13_PCREL	.text+0xe
   4:	82 e0       	ldi	r24, 0x02	; 2
   6:	93 e0       	ldi	r25, 0x03	; 3
   8:	cd 99       	sbic	0x19, 5	; 25
   a:	89 2f       	mov	r24, r25
   c:	08 95       	ret
   e:	80 e0       	ldi	r24, 0x00	; 0
  10:	91 e0       	ldi	r25, 0x01	; 1
  12:	00 c0       	rjmp	.+0      	; 0x14 <__zero_reg__+0x13>
			12: R_AVR_13_PCREL	.text+0x8

Go figure...

Top