ws2812: write inline assembly using C instead of Go #401
Conversation
This is better for various reasons:
* I don't like the inline assembly implementation in TinyGo. It kinda
works, but it's hard to use correctly.
* The Go version had a subtle bug: the i and value variables weren't
marked as modified. The compiler produced correct code by chance. In
GCC-style inline assembly, it's possible to correctly mark it as an
input+output operand.
* LLVM 14 has some changes to inline assembly that change how memory
operands can be created. Instead of supporting that, I'd like to get
rid of memory operands entirely (and possibly inline assembly in
general).
I did some light testing: the ARM assembly changed a bit but not in any
way that should have a practical effect, and the RISC-V assembly didn't
change at all.
See #401 for details. I haven't converted it to autogenerated assembly because AVR is different from many other architectures (8-bit, among others) and it didn't seem worth the effort as many chips run at 16MHz anyway. I ran the two AVR smoke tests for the ws2812 driver and the resulting binary is exactly the same.
See #401 for details. I haven't converted it to autogenerated assembly because AVR is different from many other architectures (8-bit, among others) and it didn't seem worth the effort as many chips run at 16MHz anyway.
|
I tried testing this on my Digispark. It compiled but did not work. Then I discovered that TinyGo v0.22 does not work either. v0.20 does, but I did not get a chance to bisect further. |
|
It works only halfway for me (the lights work but have the wrong color etc). I'll bisect this a bit. |
|
Bisected to tinygo-org/tinygo@92150bd. Not sure why that has an effect, because interrupts are disabled while updating the LEDs. |
|
The weird thing is that I've used an Arduino Nano in a project with ws2812 LEDs (192 LEDs!) and it worked just fine. |
|
tinygo-org/tinygo#2791 fixes the bug for the Arduino Uno (I didn't test the DigiSpark). |
|
Digispark works somewhat better, but now seems like all the commands to the LEDs in the strip are only changing the color of the first LED. The others are unlit.
|
|
Thanks for testing! I'm afraid I need to investigate this more thoroughly. |
|
I think the Digispark issue being unrelated should not delay merging this PR, which does work as advertised on the other boards I have tested. Thanks @aykevl now merging. |
For details, see: * tinygo-org/drivers#401 * tinygo-org/tinygo#2790 Essentially, this updates the inline assembly to the current best practice.
For details, see: * tinygo-org/drivers#401 * tinygo-org/tinygo#2790 Essentially, this updates the inline assembly to the current best practice.
For details, see: * tinygo-org/drivers#401 * tinygo-org/tinygo#2790 Essentially, this updates the inline assembly to the current best practice.
See #401 for details. I haven't converted it to autogenerated assembly because AVR is different from many other architectures (8-bit, among others) and it didn't seem worth the effort as many chips run at 16MHz anyway. I ran the two AVR smoke tests for the ws2812 driver and the resulting binary is exactly the same.
This is better for various reasons:
works, but it's hard to use correctly.
marked as modified. The compiler produced correct code by chance. In
GCC-style inline assembly, it's possible to correctly mark it as an
input+output operand.
operands can be created. Instead of supporting that, I'd like to get
rid of memory operands entirely (and possibly inline assembly in
general).
I did some light testing: the ARM assembly changed a bit but not in any
way that should have a practical effect, and the RISC-V assembly didn't
change at all.
This PR is possible thanks to ThinLTO, which enables inlining across C and Go and performs some more dead code elimination. And it might also require #391.