Discussion:
[avr-libc-dev] Missed optimizations
Raul Sanchez
2017-04-16 17:30:43 UTC
Permalink
Just wanted to report the following observations in generated code.
Target: __AVR_ATtiny85__Version: AVR gcc 4.6.4Optimization level: -Os
-------- C source ----------  uint16_t sum_adc;  uint16_t analog;  uint16_t adc_buffer[64];  uint8_t adc_index;...  sum_adc += ADC;  sum_adc -= adc_buffer[adc_index];  analog = (uint16_t) (sum_adc >> 3);...------- end C source ------------
------- assembly code (with corresponding C statements) --------//  sum_adc += ADC;        lds r18,sum_adc        lds r19,sum_adc+1        in r24,36-0x20        in r25,36+1-0x20        add r24,r18        adc r25,r19        sts sum_adc+1,r25        sts sum_adc,r24// sum_adc -= adc_buffer[adc_index];        lds r18,sum_adc              ; why not "mov r18,r24" "mov r19,r25" ?        lds r19,sum_adc+1        lds r30,adc_index        ldi r31,lo8(0)                    ;why not "clr r31"?        ldi r24,lo8(adc_buffer)        ldi r25,hi8(adc_buffer)        lsl r30        rol r31        add r30,r24        adc r31,r25        ld r20,Z        ldd r21,Z+1        sub r18,r20        sbc r19,r21        sts sum_adc+1,r19            sts sum_adc,r18// analog = (uint16_t) (sum_adc >> 3);        lds r18,sum_adc               ; loading into r18, r19 the same values they alredy hold!        lds r19,sum_adc+1        lsr r19        ror r18        lsr r19        ror r18        lsr r19        ror r18        sts analog+1,r19        sts analog,r18
Joerg Wunsch
2017-04-17 20:34:59 UTC
Permalink
Post by Raul Sanchez
Just wanted to report the following observations in generated code.
Target: __AVR_ATtiny85__Version: AVR gcc 4.6.4Optimization level: -Os
Besides your code missing linefeeds so it's barely readable: sorry,
GCC code optimization is nothing we could do anything about. We are
*only* dealing with the standard library that is frequently used
together with AVR-GCC.

About AVR-GCC, you'd have to talk to the GCC developers. However, AVR
is a minor target only there. The most competent person to talk to
about AVR-GCC's optimizations is probably stil Georg-Johann Lay
though. He implemented a lot of improvements in that area in the past
years.
--
cheers, Joerg .-.-. --... ...-- -.. . DL8DTL

http://www.sax.de/~joerg/
Never trust an operating system you don't have sources for. ;-)
Senthil Kumar Selvaraj
2017-04-18 06:08:24 UTC
Permalink
Post by Joerg Wunsch
Post by Raul Sanchez
Just wanted to report the following observations in generated code.
Target:__AVR_ATtiny85__Version: AVR gcc 4.6.4Optimization level: -Os
Besides your code missing linefeeds so it's barely readable: sorry,
GCC code optimization is nothing we could do anything about. We are
*only* dealing with the standard library that is frequently used
together with AVR-GCC.
About AVR-GCC, you'd have to talk to the GCC developers. However, AVR
is a minor target only there. The most competent person to talk to
about AVR-GCC's optimizations is probably stil Georg-Johann Lay
though. He implemented a lot of improvements in that area in the past
years.
I tried it out with the latest trunk build - it fixes all the missed
optimizations you pointed out, except for clr. According to the
instruction set manual, both ldi reg,0 and clr reg are 16 bit
instructions and both execute in one clock cycle, so I don't see any
missed optimization there.

Compiler Explorer link with your source - http://avr-gcc.senthilthecoder.com/#g:!((g:!((g:!((h:codeEditor,i:(j:1,options:(colouriseAsm:'0',compileOnChange:'0'),source:'%23include+%3Cavr/io.h%3E%0A%23include+%3Cstdint.h%3E%0Auint16_t+sum_adc%3B%0Auint16_t+analog%3B%0Auint16_t+adc_buffer%5B64%5D%3B%0Auint8_t+adc_index%3B%0A%0Aint+main()+%7B%0A++sum_adc+%2B%3D+ADC%3B%0A++sum_adc+-%3D+adc_buffer%5Badc_index%5D%3B++%0A++analog+%3D+(uint16_t)+(sum_adc+%3E%3E+3)%3B%0A++return+analog%3B%0A%7D'),l:'5',n:'1',o:'C+source+%231',t:'0')),k:37.96344647519582,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:compiler,i:(compiler:trunk,filters:(b:'0',commentOnly:'0',directives:'0',intel:'0'),options:'-v+-mmcu%3Dattiny85+-Os'),l:'5',n:'0',o:'%231+with+trunk',t:'0')),k:62.03655352480417,l:'4',n:'0',o:'',s:0,t:'0')),l:'2',n:'0',o:'',t:'0')),version:4
Georg-Johann Lay
2017-04-18 08:38:15 UTC
Permalink
On 16.04.2017 19:30, Raul Sanchez wrote:

Your email is badly broken, something it terribly wrong with your line
breaks.
Post by Raul Sanchez
Just wanted to report the following observations in generated code.
Target: __AVR_ATtiny85__
Version: AVR gcc 4.6.4
Quite old a version, dates 6 years back which is 6 major GCC versions.
Post by Raul Sanchez
Optimization level: -Os
-------- C source ----------
uint16_t sum_adc;
uint16_t analog;
uint16_t adc_buffer[64];
uint8_t adc_index;
...
This is not valid C. "..." may only occur as varargs function parameter
Post by Raul Sanchez
sum_adc += ADC;
This is not valid C: Assignments may only occur inside functions.
Post by Raul Sanchez
sum_adc -= adc_buffer[adc_index];
analog = (uint16_t) (sum_adc >> 3);
...
------- end C source ------------
------- assembly code (with corresponding C statements) --------
// sum_adc += ADC;
lds r18,sum_adc
lds r19,sum_adc+1
in r24,36-0x20
in r25,36+1-0x20
add r24,r18
adc r25,r19
sts sum_adc+1,r25
sts sum_adc,r24
// sum_adc -= adc_buffer[adc_index];
lds r18,sum_adc ; why not "mov r18,r24" "mov r19,r25" ?
Presumably, because you are using an outdated version of avr-gcc. The
oldest version I have handy is avr-gcc 4.7, and with that, the generated
code is different. But very likely, your "missed optimization" roots
somewhere in the extra obfuscation "...".
Post by Raul Sanchez
lds r19,sum_adc+1
lds r30,adc_index
ldi r31,lo8(0) ;why not "clr r31"?
Because LDI *,0 performs as well as CLR * but with the advantage that
the former does not clobber SREG.
Post by Raul Sanchez
ldi r24,lo8(adc_buffer)
ldi r25,hi8(adc_buffer)
lsl r30
rol r31
add r30,r24
adc r31,r25
ld r20,Z
ldd r21,Z+1
sub r18,r20
sbc r19,r21
sts sum_adc+1,r19
sts sum_adc,r18
// analog = (uint16_t) (sum_adc >> 3);
lds r18,sum_adc ; loading into r18, r19 the same values they alredy hold!
lds r19,sum_adc+1
lsr r19
ror r18
lsr r19
ror r18
lsr r19
ror r18
This shift appears to be invoked by -O2 or -O3, not by -Os as stated above.
Post by Raul Sanchez
sts analog+1,r19
sts analog,r18
Just for the recored, I compiled the following C code

#include <avr/io.h>

uint16_t sum_adc;
uint16_t analog;
uint16_t adc_buffer[64];
uint8_t adc_index;

void func (void)
{
sum_adc += ADC;
sum_adc -= adc_buffer[adc_index];
analog = (uint16_t) (sum_adc >> 3);
}

with

$ avr-gcc-4.7 code.c -mmcu=attiny85 -S -O2

and the resulting code.s reads:

func:
in r24,0x4
in r25,0x4+1
lds r18,sum_adc
lds r19,sum_adc+1
add r24,r18
adc r25,r19
lds r30,adc_index
ldi r31,0
lsl r30
rol r31
subi r30,lo8(-(adc_buffer))
sbci r31,hi8(-(adc_buffer))
ld r18,Z
ldd r19,Z+1
sub r24,r18
sbc r25,r19
sts sum_adc+1,r25
sts sum_adc,r24
lsr r25
ror r24
lsr r25
ror r24
lsr r25
ror r24
sts analog+1,r25
sts analog,r24
ret


Johann

Loading...