-
Notifications
You must be signed in to change notification settings - Fork 58
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
[WIP] Add nmigen.build #46
Conversation
nmigen/back/rtlil.py
Outdated
@@ -843,13 +843,17 @@ def convert_fragment(builder, fragment, name, top): | |||
# to create a cell for us in the parent module. | |||
port_map = OrderedDict() | |||
for signal in fragment.ports: | |||
port_map[compiler_state.resolve_curr(signal)] = signal | |||
signal.name = compiler_state.resolve_curr(signal) | |||
port_map[signal.name] = signal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's backwards. The constraint file should be adapted to the signal names, not the other way around.
What thought has been put into the design of |
Codecov Report
@@ Coverage Diff @@
## master #46 +/- ##
==========================================
- Coverage 85.66% 85.62% -0.04%
==========================================
Files 27 27
Lines 3927 3930 +3
Branches 767 768 +1
==========================================
+ Hits 3364 3365 +1
- Misses 475 476 +1
- Partials 88 89 +1
Continue to review full report at Codecov.
|
FYI - @olofk |
I want to invoke Vivado myself from Nix to facilitate things like https://github.com/m-labs/nix-scripts/issues/11 (or at least have an easy option to do so). |
@sbourdeauducq Edalize is an abstraction library for EDA tools to make it a bit easier to target different tools. It does not attempt to generate any RTL or constraint files as the purpose it to be a thin layer which allows python libraries (e.g. migen, litex, myhdl, apio) or stand-alone applications to use it while leaving the details up to each of these projects. For the purpose of nmigen we have identified a couple of things we would like to change to make edalize more in lin with nmigens goals. IIRC the most important of those was to generate .sh/.bat files instead of Makefiles, to avoid the need to have |
Why is it better than |
It allows rebuilding the project without having python installed. It's of course a trade-off, but I have found it very handy to export a system that I can send to clients, or to a build server, without any external dependencies. With that said, I'm not opposed to adding a possibility to launch tools from within the python code. E.g. by adding a a constructor flag to choose between generating sh/bat files, makefiles or launch from python. But I also got the feeling from your previous message that you wanted to launch vivado yourself anyway, so in that case, it would probably be best to just generate the sh file, which you can ignore and launch vivado yourself |
The existing solution in Migen also allows this. And Nix can automatically send stuff to one or many build servers... |
Not sure what this refers to, but if that was a reply to what I wrote about sending to build servers, then good you have a solution for that, because that is outside of edalize's scope. If this refers to rebuilding an EDA project without requiring python, then it seems you had the answer to your question already. All in all, I don't have any grand plans for edalize other than being a (mostly) tool-agnostic interface to EDA tools that can be reused by other projects to quickly gain support for many EDA tools. If it fits nmigen, great. More users is generally a good thing. If it needs some changes, let's look at those changes. If it's a bad fit for nmigen, then there's no use in shoehorning it in. |
It refers to building without python. That has been used several times to report problems to Xilinx.
Well it fits in the sense that it can do the equivalent of a few |
The difficult part here is writing the tcl and xdc files (or their equivalents for other tools)... |
Design goals
Issues with migen.build
Externalizing toolchain invocation to another tool/library could alleviate the need for this code. Issues with Edalize
Current approach and limitationsSo far, the approach for this PoC has been to reuse as much code from Migen as possible, except for toolchain-specific code, which is now limited to constraints file formatting. As the io/connector handling code remains unchanged, most platforms should hopefully be able to be migrated with little to no modifications. In the case of Edalize+Vivado, I do not know how to implement hooks such as This is a case were we need to add commands to be executed after I am not sure if my current approach is the right one and I am ready to turn backward and add toolchain invocation inside nMigen if need be. |
I don't think that's the worst issue with migen.build. Here are other ones:
|
Except for the open source flow, I don't think anything runs on Mac. |
If the goal is to do exactly the same as Migen, there is no need for nMigen :) The platform migration can probably be done with some script that reads the Migen platform file and generates a rough nMigen platform file, that can be polished by hand afterwards. |
All Edalize backends that support tcl (currently ise, modelsim, quartus, rivierapro, spyglass and vivado) can be extended by passing tcl files and specifying them as tclSource. The backend will then source these files as part of the project. This can be used to set extra options or install hooks. The other option is to implement this as a post_build hook
Can you explain what kind of configuration you're referring too? |
If I'm not mistaken, in the previous example: |
I don't think that requiring Nix for nMigen is the way to go. For example, Glasgow supports Windows as a 1st class platform, which means that it could not use nMigen; I see no reason nMigen should specifically eschew Windows given that it does not really gain much from this. I think Edalize generating shell or batch scripts with an appropriate abstraction is a good solution to the problem with I also think it's more important to focus on problems with |
@jfng I apologize for my tone earlier in this thread (in #46 (comment)). I see you understood me correctly, but I should not have implied that you haven't done design work. |
It's not really bikeshedding since it leads to the idea that nmigen.build (for lack of a better name) should focus on managing FPGA I/O resources and generating EDA tool inputs, and not try too hard to be a build system. The user can supply their own external build system, e.g. based on Nix, that runs the EDA tools and performs other steps. Also, the generated EDA tool inputs should not be monolithic as they are now. For example:
Having a "batteries included" script that runs all common steps and invokes the EDA tools is fine for simple projects though. |
I agree, which is why I think Edalize is the way to go for something that would be built into nMigen.
Also reasonable. |
@jfng Re: edalize being tied to make, the plan was to move away from that. @olofk even has a branch for it. Re: "external build system", is that not what edalize is already doing? It will also generate the EDA inputs for you, so nMigen doesn't have to do it. There should be a "fire and forget" way to generate all the EDA inputs into a self-contained package when using nMigen to distribute to other people and remote machines. But I'm not convinced nMigen itself needs to generate those files. NMigen can "just" focus on pin and resource management. |
No to both. Edalize basically just runs the tool; and for the purposes of things like https://github.com/m-labs/nix-scripts/issues/11 I want to be able to disable the Edalize steps completely. |
@sbourdeauducq You can use edalize to just generate a self contained project. See here for an example. Edalize does not invoke a toolchain in this demo; I do it manually. Anyway is below an accurate summary of what you're discussing?
The advantage of migen.build for me was mostly reducing boilerplate (TCL, CDC, paths and sourcing scripts (!), etc) for every project I wrote. Yes there was a lot of duplication internally in migen.build, but over the years migen.build saved me a lot of time. |
It still generates a monolithic output, doesn't allow an external build system to supply additional VHDL/Verilog files, doesn't allow generating a DCP instead of a bitfile, etc. Again, having the option to do that, as long as it can be disabled, is fine and good for simple projects. |
I agree to put vendor specific constraints in If
I think these two issues are related. In migen.build, an I/O resource is defined by a name, a number and a list of constraints.
These rules are implicitly validated in the _resource_type() function. However, violations are reported by an Despite being treated differently from other constraints, In order to make this distinction explicit, I suggest the following model: An I/O resource is defined by:
A
By separating
We can split the build in two steps:
Step 2. is optional in case the user wants more control over their toolchain. |
For example: Before: _io = [
("clk100", 0, Pins("E3"), IOStandard("LVCMOS33")),
("serial", 0,
Subsignal("tx", Pins("D10")),
Subsignal("rx", Pins("A10")),
IOStandard("LVCMOS33")
),
] After: _io = [
Resource("clk100", 0, Pins("E3"), misc=[Misc("IOSTANDARD=LVCMOS33")]),
Resource("serial", 0,
Subsignal("tx", Pins("D10")),
Subsignal("rx", Pins("A10"))
misc=[Misc("IOSTANDARD=LVCMOS33")]
)
]
class Resource:
def __init__(self, name, number, *io, misc=[]):
self.name = name
self.number = number
if (len(io) == 1) and isinstance(io[0], Pins):
self.io = io[0]
elif all(isinstance(c, Subsignal) for c in io):
self.io = io
else:
raise TypeError("io has invalid type: should be either Pins or a list of Subsignals")
if misc and not all(isinstance(c, Misc) for c in misc):
raise TypeError("misc has invalid type: should be a list of Misc")
self.misc = misc |
What's the point of |
That style is garbage because all it results in is crashes some time later where you no longer have any context. The hypothetical advantages of being able to pass an object that implements the |
I do, sufficiently to add type and range checks everywhere else in nMigen. Incidentally, almost everyone who gave me feedback on nMigen appreciates these checks.
Which is why the
I completely disagree. Python doesn't even support duck typing, in the sense that it gives you essentially no tools to actually make use of that feature. For example, Also, Linux is simply not meant to be used on desktops, and yet here we are. Of course, I would rather not use Python here at all, or ever again. But so long as this is a requirement, I'm going to make it work reliably whether it's meant for that or not. |
@olofk Still with the Vivado backend, if I wanted to do the following: synth_design -top {top} -part {device}
# ask for reports about utilization, timing, etc here
opt_design
place_design
# more reports
route_design
# more reports, checkpoint
write_bitstream {top}.bit Can I get away with sourcing these commands in an additional file (or a "pre_build" hook), except |
Progress update:
Automatic insertion of tristate buffers needs to be sorted out first. Changes:
|
Progress update:
Resource("foo", 0,
DiffPairs(
"N2 N1",
"U2 V2",
dir="io"
),
misc=[Misc("IOSTANDARD=DIFF_SSTL135")]
)
|
i still don't see the point of encapsulating every string in a |
|
nmigen/lib/io.py
Outdated
self.i = i | ||
|
||
def elaborate(self, platform): | ||
m = Module() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entire body of this function can be replaced with platform.get_diff_input(self.p, self.n, self.i)
, it is a no-op as written.
But also I think we don't actually need any of these classes. You either:
- request a prepared pad (SDR, DDR, whatever) from the platform, in which case it gives you a Record already connected to an Instance, or
- request a raw pad from the platform, in which case it gives you a bare inout.
In both cases we don't need DiffInput
, etc, only pad_layout
.
nmigen/build/generic_platform.py
Outdated
assert (resource.name, resource.number) not in self.matched | ||
|
||
if dir is not None: | ||
resource.update_dir(dir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are resources supposed to be added? If this is done the same way as in oMigen, i.e. you have a global array of _io
somewhere, then this is completely wrong, because trying to build two different designs in one Python invocation will fail.
In general, nMigen avoids mutating anything, which is slightly slower but eliminates hard-to-find bugs like this.
Changes:
|
@@ -0,0 +1,383 @@ | |||
import edalize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do a lazy import so it's not a dependency for flows not using edalize?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've agreed (privately on IRC) to postpone edalize integration as edalize does not yet do what we want. I'm not sure why the PR isn't updated.
Merged. |
@olofk By the way, you might want to investigate the solution I made for nMigen instead of Edalize:
The way it works is it uses Jinja2 templates to generate a completely standalone archive of build precursors, and scripts for all supported OSes, which you can then freely move around (as an archive or in any other way) and build by running one command. As far as I can tell Edalize cannot do this, and this means it is not possible to use Edalize to perform remote builds (without installing it on a remote machine, which is fragught with problems). |
This is a proof-of-concept for a nMigen equivalent of Migen's build system.
The main difference lies in the use of Edalize to invoke vendor tools instead of doing so ourselves.
However, as far as I understand, constraint file (XDC, PCF, etc.) formatting must still be done on our side, which prevents nMigen from being completely vendor agnostic.