UEFI: add support for tasks scheduler and make it default - #5553
UEFI: add support for tasks scheduler and make it default #5553sparques wants to merge 3 commits into
Conversation
|
You mean like this? https://github.com/tinygo-org/tinygo/blob/dev/targets/avr.json#L20 |
|
I'm sorry, if there is something that's making |
|
I am wonder why you would need it conditionally included when you can just add to target file? |
|
@deadprogram Ah, I think I see what was confusing me. I thought src/internal/task/task_stack_amd64_windows.S was specific to just the tasks scheduler (and thus, should not be included for any other scheduler). But it's fine if it's included even if we're not using the tasks scheduler? |
|
Here are some further edited comments from automated review.
Since both share the same .S file too, I'd delete the new file and update the tags instead:
That drops 58 lines and, more importantly, means a future fix to the register layout can't be applied to one copy and missed in the other. Renaming the file to task_stack_amd64_winabi.go at that point would be a nice touch, but it's cosmetic.
Minor: the c.GOARCH() == "amd64" guard is redundant today (uefi-amd64 is the only uefi target and pins goarch: amd64), but harmless as future-proofing.
for ticks() < deadline { The cooperative scheduler's normal path (addSleepTask + task.Pause(), scheduler_cooperative.go:253) should already work on UEFI because ticks(), nanosecondsToTicks(), and sleepTicks() are all implemented in src/runtime/runtime_uefi.go. If the default path works, this file can go away entirely. If it does need to stay, with this implementation a sleeping goroutine spins the run queue for the whole duration, so time.Sleep never lets the scheduler go idle. That matches existing UEFI behavior (sleepTicks already spins on CpuPause), so it's not a regression but it does mean waitForEvents/SetWaitForEvents never gets a chance to run during a sleep, which may matter for anyone using that hook. Worth a comment in the file either way explaining why the standard sleep queue isn't used. Also: the //go:linkname gosched runtime.Gosched is unnecessary since sleep_custom_uefi.go is in package runtime, so it can call Gosched() directly. The linkname adds an indirection with no benefit. |
This adds support to uefi-amd64 target for the tasks scheduler and makes it default. The none-scheduler is still supported.
I did a very basic test of making sure two separate goroutines ran simultaneously--seems to work.
I'm not sure if the bit in compileopts/config.go is kosher; Is that the right approach or is there a better way to conditionally include ExtraFiles?