wrong inline asm constraint in delayMicroseconds

[nerdralph]

Bug first discovered in MicroCore, which uses the same code.
MCUdude/MicroCore#136

[MCUdude]

What's the reason for the slightly different delayMicroseconds implementation in ArduinoShrink and picoCore?

ArduinoShrink:

ArduinoShrink/src/as_wiring.c

Lines 25 to 41 in a95c1c4

	void delayMicroseconds(uint16_t us)
	{
	// if us is a compile-time constant result is accurate to 1 cycle
	if (__builtin_constant_p(us)) {
	_delay_us(us);
	return;
	}
	// code below runs when us is not known at compile time
	// uses inline asm to guarantee exact cycle counts
	const float fMHz = (F_CPU/1000000.0);
	delay1us:
	// delay 1us per loop, less 4 cycles for overhead
	_delay_us(1.0 - (4.0 / fMHz));
	asm volatile ("sbiw %[us], 1" : [us]"+w"(us));
	asm goto( "brne %l[delay1us]" :::: delay1us);
	}

picoCore:

__attribute((always_inline))
static inline void delayMicroseconds(uint16_t us)
{
    // if us is a compile-time constant result is accurate to 1 cycle
    if (__builtin_constant_p(us)) {
        _delay_us(us);
        return;
    }

    // when us is not known at compile time, delay is accurate to +/- 2us
    // plus an overhead of 3 CPU cycles
    const float fMHz = (F_CPU/1000000.0);
    // subtract two for rounding before dividing by 4
    us -= 2;
    delay4us:
        // delay 4us per loop, less 4 cycles for overhead
        _delay_us(4.0 - (4.0 / fMHz));
        asm volatile ("sbiw %[us], 4" : [us]"+d"(us));
    asm goto( "brpl %l[delay4us]" :::: delay4us);
}

[nerdralph]

picoCore is designed to work at lower frequencies, such as 4.8Mhz or even 1.2Mhz. ArduinoShink is designed for the common frequencies of 8 and 16Mhz. If you try to build ArduinoShink with F_CPU set to 1Mhz it will fail.

…

On Wed, Jan 4, 2023, 13:47 Hans ***@***.***> wrote: What's the reason for the slightly different delayMicroseconds implementation in ArduinoShrink and picoCore? ArduinoShrink: https://github.com/nerdralph/ArduinoShrink/blob/a95c1c40721834e714f6bfe88fe0a280a7940052/src/as_wiring.c#L25-L41 picoCore: __attribute((always_inline)) static inline void delayMicroseconds(uint16_t us) { // if us is a compile-time constant result is accurate to 1 cycle if (__builtin_constant_p(us)) { _delay_us(us); return; } // when us is not known at compile time, delay is accurate to +/- 2us // plus an overhead of 3 CPU cycles const float fMHz = (F_CPU/1000000.0); // subtract two for rounding before dividing by 4 us -= 2; delay4us: // delay 4us per loop, less 4 cycles for overhead _delay_us(4.0 - (4.0 / fMHz)); asm volatile ("sbiw %[us], 4" : [us]"+d"(us)); asm goto( "brpl %l[delay4us]" :::: delay4us); } — Reply to this email directly, view it on GitHub <#12 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKNZ6TJ6BVL62MV6EEQN5LWQWZRVANCNFSM6AAAAAATP6TSQA> . You are receiving this because you authored the thread.Message ID: ***@***.***>