bloated/inefficient code in init()

[nerdralph]

init() excessively uses the sbi/cbi macros, which rarely compile to the sbi/cbi instructions. For addresses over 0x3F, the sbi macro results in lds + ori + sts. I tried cleaning up the ADC setup code as follows:

uint8_t prescaler = _BV(ADPS2) | _BV(ADPS1) | _BV(ADPS2);
ADCSRA = prescaler | _BV(ADEN);

This compiles to ldi + sts, and reduced the size of the Blink sketch (built for m328p) from 924 to 890 bytes. Doing the same for the timer setup code would save even more. I'm willing to prepare a pull request if there is interest in these improvements

[MCUdude]

Thanks for pointing this out! I was not aware that sbi/cbi macros rarely compiled to a single instruction.

BTW is there any reason why you couldn't have done this instead in your example? Is the variable necessary?

ADCSRA = _BV(ADPS2) | _BV(ADPS1) | _BV(ADPS0) | _BV(ADEN);

[nerdralph]

Thanks for pointing this out! I was not aware that sbi/cbi macros rarely compiled to a single instruction.

BTW is there any reason why you couldn't have done this instead in your example? Is the variable necessary?

ADCSRA = _BV(ADPS2) | _BV(ADPS1) | _BV(ADPS0) | _BV(ADEN);

I used a prescaler variable for code readability. gcc optimizes it away, so it still compiles to the ldi + sts that I mentioned.

[MCUdude]

I used a prescaler variable for code readability. gcc optimizes it away, so it still compiles to the ldi + sts that I mentioned.

Makes sense. Is there a rule of thumb that I can follow when it comes to optimizing away variables? In other words, how do I know if GCC will optimize the variable away or not? Is it when the variable is used as a constant?

[nerdralph]

I used a prescaler variable for code readability. gcc optimizes it away, so it still compiles to the ldi + sts that I mentioned.

Makes sense. Is there a rule of thumb that I can follow when it comes to optimizing away variables? In other words, how do I know if GCC will optimize the variable away or not? Is it when the variable is used as a constant?

If it's not volatile, gcc is pretty good at optimizing away constant variables. With LTO, it does that even across functions. The easiest way to be sure is to use avr-objdump to look at the binary.

[MCUdude]

If it's not volatile, gcc is pretty good at optimizing away constant variables. With LTO, it does that even across functions. The easiest way to be sure is to use avr-objdump to look at the binary.

Thanks, I'll keep this in mind.

the definition for the sbi and cbi macros is this:

#ifndef cbi
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#endif

#ifndef sbi
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#endif

what is it that _SFR_BYTE does that makes this not compile into a single instruction? Does _SFR_BYTE bring any "features that the following code don't?

#ifndef cbi
#define cbi(sfr, bit) (sfr &= ~_BV(bit))
#endif

#ifndef sbi
#define sbi(sfr, bit) (sfr |= _BV(bit))
#endif

[SpenceKonde]

Frankly, not a clue what the point of the _SFR_BYTE in there is...

The main issue there us the fact that setting or clearing a byte can't bcompile into a single instruction except in special cases.... the SBI and CBI instructions only work in the lowest 32 locations of IO memory.... the relevant defines are in sfr_defs.h

#define _SFR_BYTE(sfr) _MMIO_BYTE(_SFR_ADDR(sfr))

#define _MMIO_BYTE(mem_addr) (*(volatile uint8_t *)(mem_addr))

#define _SFR_ADDR(sfr) _SFR_MEM_ADDR(sfr)

#define _SFR_MEM_ADDR(sfr) ((uint16_t) &(sfr))

[SpenceKonde]

@nerdralph - do you have any idea what this does?

Does the compiler actually need hints like this? Won't something like:

PORTA |= 1<<5;

already come out to an SBI or CBI if it's compiletime known?

[SpenceKonde]

(PORTA chosen just as an example, since we know it's in the low IO space)

[nerdralph]

gcc compiles it to sbi. A few of the SFR macros are still useful for inline asm, but using the cbi and sbi macros is just stupid.

On Fri, Oct 2, 2020, 23:17 Spence Konde (aka Dr. Azzy) < notifications@github.com> wrote:

@nerdralph https://github.com/nerdralph - do you have any idea what this does?

Does the compiler actually need hints like this? Won't something like:

PORTA |= 1<<5;

already come out to an SBI or CBI if it's compiletime known?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/arduino/ArduinoCore-avr/issues/357#issuecomment-703031391, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNZ6QOSCJKF6AJ74KDJYLSI2CUDANCNFSM4OXL5M7A .

[SpenceKonde]

Glad you agree. I'll excise those from my cores in a future update.

On Sat, Oct 3, 2020 at 6:12 AM Ralph Doncaster notifications@github.com wrote:

gcc compiles it to sbi. A few of the SFR macros are still useful for inline asm, but using the cbi and sbi macros is just stupid.

On Fri, Oct 2, 2020, 23:17 Spence Konde (aka Dr. Azzy) < notifications@github.com> wrote:

@nerdralph https://github.com/nerdralph - do you have any idea what this does?

Does the compiler actually need hints like this? Won't something like:

PORTA |= 1<<5;

already come out to an SBI or CBI if it's compiletime known?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/arduino/ArduinoCore-avr/issues/357#issuecomment-703031391, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNZ6QOSCJKF6AJ74KDJYLSI2CUDANCNFSM4OXL5M7A .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

--

---

Spence Konde Azzy’S Electronics

New products! Check them out at tindie.com/stores/DrAzzy GitHub: github.com/SpenceKonde ATTinyCore: Arduino support for almost every ATTiny microcontroller Contact: spencekonde@gmail.com