drivers: fix overflow in ClockDevice.sleep_ms

[matpala]

ClockDevice.sleep_ms computed time_ms * 1000 in u32, which overflows from 4,294,968 ms (about 71.6 minutes): a panic with runtime safety enabled, illegal behavior otherwise. This multiplies in u64 instead.
Adds a test for 0, 1, the last value that fit, the first that overflowed, and maxInt(u32); it fails against main's implementation with integer overflow.

[mattnite]

Did you try enabling compiler_rt?

[matpala]

Yes. With .bundle_compiler_rt = true on the ATmega328P, compiler_rt itself fails to build with Zig 0.17.0 (address space 'flash' cannot cast into address space 'generic', ziglang/zig#37090, still open), which is why the AVR ports turn it off.

[Grazfather]

1. This adds avr-specific comments in a generic driver. I would just say that it avoid using mult instructions. Please test against multiple targets (e.g. wch, esp32c3, rp2040, and atmega328) + disassemble and provide evidence
2. Do you see evidence of the ms wasting time storing to the stack and then copying to an arg. I wonder if using a bunch of @as(u64... might avoid this. Again, please test against those targets.

[matpala]

Thanks for the review. I went back to a plain multiply, @as(u64, time_ms) * 1000, with no comment.

I built a small probe with ReleaseSmall (Zig 0.17.0) for avr5, rp2040, esp32c3, ch32v003 and ch32v203 and looked at the disassembly. With optimizations LLVM turns the shifts back into * 1000, so they compile to the same code as the multiply on every target:

• esp32c3 / ch32v203: mul + mulhu
• rp2040: __aeabi_lmul
• ch32v003 / atmega328: __muldi3

At first the shifts looked better on AVR, because they linked in the example firmware and the multiply didn't. The reason was something else: in the default build the drivers module is compiled in Debug even when the firmware is ReleaseSmall (build.zig calls dependency("drivers", .{}) without optimize, and zig build --verbose shows -Odebug ... -Mdrivers=...). In Debug the shifts aren't folded. With drivers forced to ReleaseSmall, both versions need __muldi3 on AVR. So the shifts only helped because of the Debug build, and I removed them. I'll send the optimize fix as a separate PR.

On AVR this is the same as main: with a constant like sleep_ms(100) it links, with a runtime value it doesn't (main needed __mulsi3, now __muldi3).

About ms: in the probe functions it stays in registers on every target. What they copy to the stack is the ClockDevice passed to sleep_us, and the old code does the same. With the plain multiply there's no local anymore.

This is compile and disassembly only, I didn't run it on hardware.

Probe and results

drivers/src/probe_sleep.zig on the PR branch:

const ClockDevice = @import("base/ClockDevice.zig");

export fn old_u32_mul(td: *const ClockDevice, time_ms: u32) void {
    td.sleep_us(time_ms * 1000);
}

export fn u64_mul(td: *const ClockDevice, time_ms: u32) void {
    td.sleep_us(@as(u64, time_ms) * 1000);
}

export fn shifts(td: *const ClockDevice, time_ms: u32) void {
    const ms: u64 = time_ms;
    td.sleep_us((ms << 10) - (ms << 4) - (ms << 3));
}

cd drivers/src
zig build-obj probe_sleep.zig -OReleaseSmall -ffunction-sections -target avr-freestanding -mcpu=avr5 -femit-bin=avr.o
zig build-obj probe_sleep.zig -OReleaseSmall -ffunction-sections -target thumb-freestanding-eabi -mcpu=cortex_m0plus -femit-bin=rp2040.o
zig build-obj probe_sleep.zig -OReleaseSmall -ffunction-sections -target riscv32-freestanding-eabi -mcpu=generic_rv32+c+m+zicsr+zifencei -femit-bin=esp32c3.o
zig build-obj probe_sleep.zig -OReleaseSmall -ffunction-sections -target riscv32-freestanding-eabi -mcpu=generic_rv32+e+c+xwchc+zicsr+zifencei-i -femit-bin=ch32v003.o
zig build-obj probe_sleep.zig -OReleaseSmall -ffunction-sections -target riscv32-freestanding-eabi -mcpu=generic_rv32+m+a+c+xwchc+zicsr+zifencei -femit-bin=ch32v203.o
avr-objdump -dr avr.o
llvm-objdump -dr rp2040.o   # same for the other three

Instruction counts exclude callees; u64_mul and shifts compile to identical code:

Target	old_u32_mul	u64_mul = shifts
avr5	53, __mulsi3	64, __muldi3
rp2040	12, muls	16, __aeabi_lmul
esp32c3	15, mul	16, mul + mulhu
ch32v003	19, __mulsi3	22, __muldi3
ch32v203	15, mul	16, mul + mulhu

AVR example firmware (Nano, ReleaseSmall, sleep_ms with a runtime u32):

drivers module	shifts	u64 * 1000	main (u32 * 1000)
Debug (default today)	links	__muldi3	__mulsi3
ReleaseSmall	__muldi3	__muldi3	not tested

With sleep_ms(100) both versions link in both modes, and main links with Debug drivers.

[matpala]

This was merged before I pushed the change I described above, so I opened #1090 with it.