Conversation
| } else { | ||
| raw = fs.readFileSync(jsonPath, 'utf8'); | ||
| } | ||
| const yosysJson = JSON.parse(raw); |
There was a problem hiding this comment.
This is really the only key test we need -- we don't need the yosys.js or icons files. I need to look at the Utils files to see if anything is needed. What we want here is just to make sure the schematic JSON is healthy. We don't need a full d3 evaluation of it.
mkorbel1
left a comment
mkorbel1
left a comment
There was a problem hiding this comment.
Still WIP reviewing this, but posting current checkpoint
There was a problem hiding this comment.
Are these files from d3 copied exactly or modified?
| return; | ||
| } | ||
|
|
||
| final topModule = tops.first; |
| final topModule = tops.first; | ||
| for (final name in globalPortNames) { | ||
| // Check inputs, outputs, and inOuts | ||
| final port = topModule.inputs[name] ?? |
| } | ||
| } | ||
|
|
||
| // ModuleMap-based helpers removed — Schematic synthesis prefers |
There was a problem hiding this comment.
Why not follow the same pattern as we did with SystemVerilog mixins where property information about how it synthesizes is stored within the class instead of registered separately? This makes it easier to (a) introspect on the object (rather than regex), (b) for users to expand capabilities without modifying ROHD, (c) override things
| SynthesisResult synthesize( | ||
| Module module, String Function(Module module) getInstanceTypeOfModule); | ||
| Module module, String Function(Module module) getInstanceTypeOfModule, | ||
| {SynthesisResult? Function(Module module)? lookupExistingResult, |
There was a problem hiding this comment.
I'm confused why you need this lookupExistingResult argument in various places, why you need it in addition to existingResults, and how it relates to something called "ModuleMap" that I see referenced in various places but don't see any class or variable with that name. Is this legacy stuff from a prior iteration?
| }); | ||
|
|
||
| @override | ||
| void prepare(List<Module> tops) { |
There was a problem hiding this comment.
Why does this have to happen in a separate prepare step instead of just upon synthesizing?
| }); | ||
|
|
||
| @override | ||
| void prepare(List<Module> tops) { |
There was a problem hiding this comment.
Why does it need to know all the tops? Can't it just match any port with those names on any module or just look for specific Logic instances?
| } | ||
|
|
||
| /// Deep list equality (compare contents, not identity). | ||
| static bool listEquals<T>(List<T> a, List<T> b) { |
There was a problem hiding this comment.
why not use the available ListEquality utilities for collections instead of building your own?
| continue; | ||
| } | ||
| reachableFromGlobals.add(cur); | ||
| for (final dst in cur.dstConnections) { |
There was a problem hiding this comment.
what if a global signal is an output? or is that not supported?
what if a global signal is an inOut? then we need to search in both directions, or is that not supported?
| import 'package:rohd/src/synthesizers/schematic/schematic_mixins.dart'; | ||
| import 'package:rohd/src/synthesizers/schematic/schematic_synthesis_result.dart'; | ||
|
|
||
| bool _listEquals<T>(List<T> a, List<T> b) { |
There was a problem hiding this comment.
Another list equality check custom implementation?
| final List<int> _ids; | ||
|
|
||
| /// The constant value being driven. | ||
| final BigInt _value; |
There was a problem hiding this comment.
why is this BigInt instead of LogicValue?
| extension ModuleUtils on Module { | ||
| /// Returns a map of all declared ports (inputs, outputs, inOuts) | ||
| /// keyed by port name. | ||
| Map<String, Logic> get ports => {...inputs, ...outputs, ...inOuts}; |
There was a problem hiding this comment.
question: should this just be added to Module instead of an extension? any reason not to?
| import 'package:rohd/rohd.dart'; | ||
| import 'package:rohd/src/synthesizers/schematic/schematic_synthesis_result.dart'; | ||
|
|
||
| /// Result of pass-through collection. |
2 participants
Description & Motivation
This PR adds a
Synthesizerfor D3 schematics that can be loaded using an ELK-JSON loader.This produces the Yosys JSON format.
Related Issue(s)
This fixes intel/rohd-hcl#162, but moves it into the rohd repository as a general purpose capability for ROHD.
Testing
We added a basic yosys JSON loader borrowed and documented from d3-hwschematics (https://github.com/Nic30/d3-hwschematic. We ran a few complex examples with multiple instantiation.
Backwards-compatibility
No. We added a prepare() routine for
SynthBuilderwhich is optional and helpful for this and futureSynthesizers.Documentation
Just a simple one-liner to parallel the
SystemVerilogSynthesizer. We probably need to make a concerted effort to document more of the utilities in ROHD.