YoWASP / yosys

Unofficial Yosys WebAssembly packages
https://yowasp.org
ISC License
67 stars 2 forks source link

Broken abc9 support for `synth_ecp5` #29

Open thoughtpolice opened 11 months ago

thoughtpolice commented 11 months ago

When using synth_ecp5, it seems -abc9 has been turned on by default at some point. However, it looks like something got miscompiled, or abc support is missing somehow, meaning synth_ecp5 just immediately fails unless you specify -noabc9:

...
3.42.18.3. Executing XAIGER backend.
<suppressed ~11 debug messages>
Extracted 231 AND gates and 1034 wires from module `top' to a netlist network with 115 inputs and 196 outputs.

3.42.18.4. Executing ABC9_EXE pass (technology mapping using ABC9).

3.42.18.5. Executing ABC9.
Running ABC command: "/yosys-abc" -s -f <abc-temp-dir>/abc.script 2>&1

I haven't taken the time to diagnose if this is an obviously known issue or not. I thought abc would be compiled into yosys in this case, but I can't remember all the details OTTOMH...

Version: @yowasp/yosys/0.37.58-dev.635

thoughtpolice commented 11 months ago

That aside, I'm still very much enjoying this package! As a new years present, here's a simple Deno program that can just synthesize an entire directory of Verilog code, assuming a top.v file, and create a bitstream for ecp5 (assuming a pinout.lcf file) with the new electrostatic nextpnr placer:

import { parse } from "https://deno.land/std@0.211.0/flags/mod.ts";
import { walk } from "https://deno.land/std@0.211.0/fs/walk.ts";

import { Tree } from "npm:@yowasp/runtime";
import { runYosys } from "npm:@yowasp/yosys";
import { runEcppack, runNextpnrEcp5 } from "npm:@yowasp/nextpnr-ecp5";

async function walkToTree(path: string): Promise<Tree> {
  const files: Tree = {};
  const dirpath = path + (path.endsWith("/") ? "" : "/");

  // first, walk all the normal files and add them to the tree
  for await (const entry of walk(path, { maxDepth: 1 })) {
    if (entry.path === path) {
      continue;
    }

    const relpath = entry.path.slice(dirpath.length);

    if (entry.isDirectory) {
      files[relpath] = await walkToTree(entry.path);
    } else {
      files[relpath] = await Deno.readFile(entry.path);
    }
  }

  return files;
}

const flags = parse(Deno.args, {
  boolean: ["help"],
});

if (flags.help) {
  console.log("FIXME");
  Deno.exit(1);
}

console.log("Performing RTL synthesis...");
const rtlTree = await walkToTree(String(flags._[0]));
const synthTree = await runYosys(
  [
    "top.v",
    "-p",
    "hierarchy -libdir . -auto-top",
    "-p",
    "synth_ecp5 -noabc9 -json top.json",
  ],
  rtlTree,
  {
    printLine: (line: string) => {
      console.log(line);
      return;
    },
  },
);

console.log("Performing place and route...");
const pnrTree = await runNextpnrEcp5(
  [
    "--json",
    "top.json",
    "--textcfg",
    "top.config",
    "--lpf",
    "pinout.lpf",
    "--85k",
    "--package",
    "CABGA381",
    "--placer",
    "static",
  ],
  synthTree,
  {
    printLine: (line: string) => {
      console.log(line);
      return;
    },
  },
);

console.log("Packing bitstream...");
const packTree = await runEcppack(
  [
    "--idcode",
    "0x41113043",
    "top.config",
    "top.bit",
  ],
  pnrTree,
  {
    printLine: (line: string) => {
      console.log(line);
      return;
    },
  },
);

await Deno.writeFile("top.bit", packTree["top.bit"] as Uint8Array);
console.log("Wrote bitstream to top.bit");

Nothing more than a single file needed to automate the whole flow. I find this remarkable and I have a bunch of ideas that can build off of this. I'm really looking forward to future improvements (e.g. hostcalls for connect_rpc to work!)

whitequark commented 11 months ago

3.42.18.5. Executing ABC9. Running ABC command: "/yosys-abc" -s -f /abc.script 2>&1

The log up to this point is perfectly normal. How does it fail? abc is compiled into Yosys, and I just checked that there is code in abc9_exe.cc to handle this case.

whitequark commented 11 months ago
    printLine: (line: string) => {
      console.log(line);
      return;
    },

I think that's the default behavior. Isn't it?

whitequark commented 11 months ago

Nothing more than a single file needed to automate the whole flow.

Yep! How do you feel about the "tree" data structure, by the way? It's a bit stringly typed but it seems simple and TypeScript can handle it. The idea there is to make each executable a normal, pure JavaScript function.

I find this remarkable and I have a bunch of ideas that can build off of this.

Nice!

I'm really looking forward to future improvements (e.g. hostcalls for connect_rpc to work!)

This is in scope in principle but I haven't thought about how to do this. Certainly I would not object to having callbacks that e.g. spawn a subprocess and communicate, or open a socket.

The primitives for this operation need to be well thought out. I do not currently run Asyncify on normal YoWASP binaries (except for openFPGALoader, where this is a part of the ABI contract, essentially) so stdin or socket reads can only be synchronous. This is why right now the only thing you get is printLine/print. (The latter is accepted by @yowasp/vscode but is only used by openFPGALoader right now; I have print to priority over printLine if specified so that you can be compatible with both in the extension. I should probably change the API to only use print and provide a higher order function wrapping a printLine function to make a print function out of it.)

I think running Asyncify would complicate the build process substantially: it will no longer be possible to use the same .wasm file for both Python and JavaScript deployment. Because of this, I am very hesitant to use Asyncify. I do not think I have the bandwidth to maintain, debug, and support two divergent copies of the build pipeline for Yosys and nextpnr.

It looks like the only way to synchronously obtain data from another web worker is by using SharedArrayBuffer and Atomic.wait. That is a fairly complex addition but I think it is probably manageable. I think the SharedArrayBuffer manipulation (as well as the Node threads implementation) should be abstracted by runtime-js.

There is no support for Unix domain sockets in WASI so that's out. It seems that our API then must be based around calling subprocesses, where stdin, stdout, and stderr are connected via pipes, and each pipe is implemented using something like a bipartite buffer based on SAB. (We should probably make the stdio interface more principled at this point, separating stdin/stdout/stderr into their own individual options.)

The main remaining problem is the filesystem. It seems completely impractical, and nightmarish in any case, to have any notion of shared filesystem. We could make a copy of the filesystem at the time of spawning but that's it. We could maybe add an option to return not a full tree but a diff of the tree, which will save space too, though you can already do this in the caller. This will never be remotely POSIX-compliant in terms of shared semantics.

I hope this braindump helps.

thoughtpolice commented 11 months ago

I think this is maybe some kind of bug in Yosys, rather than the build. The code is just a copy of the example in https://github.com/YosysHQ/prjtrellis/tree/master/examples/ulx3s, and I think that script above can basically reproduce it (if you rename hello.v to top.v).

However, I found that if I changed the Yosys invocation from this:

  return await runYosys(
    [
      "top.v",
      "-p",
      "hierarchy -libdir . -auto-top",
      "-p",
      "synth_ecp5 -abc9 -json top.json",
    ],
    inputTree,
    toolPrintLine,
  );

To this:

  return await runYosys(
    [
      "synth.ys"
    ],
    inputTree,
    toolPrintLine,
  );

With this synth.ys script next to top.v:

read_verilog top.v
hierarchy -libdir . -auto-top
synth_ecp5 -abc9 -json design.synth.json

Then it works!

    printLine: (line: string) => {
      console.log(line);
      return;
    },

I think that's the default behavior. Isn't it?

Yeah, it was just leftover debugging cruft that I hadn't cleaned up before copying out of my editor.

thoughtpolice commented 11 months ago

Here's a log of the two different runs with those arguments; bad.txt crashes while good.txt succeeds: https://gist.github.com/thoughtpolice/086577e0fc72cee70af7134e600de5cb

Also I think the asyncify point makes sense, yeah. For this Deno experiment I was just going to do web workers with SharedArrayBuffer to communicate results and run multiple design sweeps. My actual goal in the long run is to have a portable command line tool I can integrate with monorepo build systems (Buck2), so the loss of connect_rpc isn't too bad for me, since I can often write Buck rules to generate the necessary modules ahead of time, etc. Maybe if wasm effect handlers ever get implemented...

EDIT: I also quite like the Tree design on that note. The pure function approach you mention is particularly important; it's really important that in a system like Buck2 or Bazel that rules are deterministic; implementing this by putting the synthesis engine inside wasm is attractive not only for the determinism but the cross platform aspect, too.

whitequark commented 11 months ago

Then it works!

sigh So. I know that abc executes undefined behavior when built for Wasm and, possibly likely almost certainly, when built for any other platform. If you run it with the smallest Wasm heap (64k) it overflows it almost instantly on the most trivial netlists, iff it is built separately from Yosys. I have actually prototyped that mode, no less than three separate times over the years. I have scrapped it each time and deleted the sources because it was impossible to get to work or to debug. It was infuriating.

I was just going to do web workers with SharedArrayBuffer to communicate results and run multiple design sweeps

Happy to integrate that upstream if we can work out a nice and lightweight solution for the pipe and popen API. (I am pretty sure we can).

The pure function approach you mention is particularly important; it's really important that in a system like Buck2 or Bazel that rules are deterministic; implementing this by putting the synthesis engine inside wasm is attractive not only for the determinism but the cross platform aspect, too.

Yes! When I started using Yosys within Amaranth and ran into the need to deploy it, I found it conceptually intolerable that I have to build a pure function for a huge matrix of platforms and OSes just because it happens to be written in C. So I fixed it o:3

whitequark commented 11 months ago

I've now uploaded all of the tools with stdout/stderr support.

whitequark commented 9 months ago

~Is action still necessary on this issue?~ Yeah, I guess abc9 is still broken.