Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add native Yosys/Nextpnr support #14

Merged
merged 22 commits into from
Nov 26, 2023
Merged

Add native Yosys/Nextpnr support #14

merged 22 commits into from
Nov 26, 2023

Conversation

malmeloo
Copy link
Member

@malmeloo malmeloo commented Jul 12, 2023

resolves #6

In the "old" implementation, Yosys/Nextpnr tasks were subclasses of WorkerTaskTerminal. Since this class was rather biased towards the webassembly implementation, I've refactored most of this code to decouple the tasks and webasm code from each other.

Changes to existing code

  • WorkerTaskTerminal is now called TaskTerminal, as it only handles terminal-related tasks and does not necessarily spawn workers anymore. Instead, it accepts a TerminalTask in the constructor which it will set up, run and monitor.
  • NextpnrTaskTerminal and friends now subclass a new class: TerminalTask. The implementations are largely the same, but this allows us to pull task-related code such as finding the command/args and i/o files into the task itself, decoupling it from the TaskTerminal. They store these arguments in a RunnerContext data structure, which can be passed on to a Runner for actual execution.
  • The WebAssembly-specific code (+ web worker creation) has been pulled out of the TaskTerminal and into a WebAssemblyRunner. It will set up, start and monitor the worker using the existing callbacks.
  • Both Runners and TerminalTasks allow setting a callback for TerminalMessages (println, error, done). TerminalTasks proxy messages from their Runner straight to the TaskTerminal by re-emitting the message, but they can send messages themselves as well.

TODO

  • Refactor code (see above)
  • Write output files from the webasm runner to disk somewhere
  • Add Native runner using child_process
    • Write generated file to disk for the native program to load
    • Create process
    • Forward stdout/stderr from process to runner
    • Improve error handling
  • Fix WebAssembly runner
    • Currently broken since we need to use the worker_threads API which is incompatible with the web workers one
  • Add config option + code to switch between runners
  • Better terminal formatting (?)
    • Fix weird terminal spacing when an error is raised
    • Make stderr/errors red
    • make success message green
    • Add more details to log file (stream, time)

@malmeloo
Copy link
Member Author

I managed to resolve the Yosys WebAssembly runner issue; the hard nodefs dependency appears to be emitted directly by their icestorm project (here and here) and only gets executed when being compiled by emscripten and running in a nodejs environment. I wrote a small WebPack plugin to patch those nodefs mount calls out of the final bundle and removed the directories it tries to create before starting up yosys, which appears to resolve the issue.

I think a long-term solution for this would be to submit a patch to their repository and add some kind of compilation flag so we can build Yosys without that dependency, as the current fix is definitely not great (it's a simple regex find-and-replace).

I'm going to un-draft this PR as I don't think it will change much anymore, if at all, although it does still need more testing for all the different environments the extension will run in. I'll try to test it a bit more in the coming days.

@malmeloo malmeloo marked this pull request as ready for review August 25, 2023 13:11
The import promise only rejects once, so any successive attempts would throw a different error
tsconfig.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@DanielleHuisman DanielleHuisman merged commit 054b8a9 into main Nov 26, 2023
@DanielleHuisman DanielleHuisman deleted the add-native-workers branch November 26, 2023 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for using native Yosys/nextpnr
2 participants