What thought has been put into the design of nmigen.build? Where are the design notes? What problems with migen.build does it fix? I don't see any of this...

Codecov Report

Merging #46 into master will decrease coverage by 0.03%.
The diff coverage is 60%.

@@            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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a2a702...c0475fa. Read the comment docs.

FYI - @olofk

The main difference lies in the use of Edalize to invoke vendor tools instead of doing so ourselves.

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).
What problem does edalize solve? From what I can see it's only sourcing Xilinx's settingsXX.sh and running vivado -mode tcl -batch file.tcl - the xdc etc. have to be generated for it.

@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 make on the target system. I did a quick PoC for the icestorm backend some time ago (https://github.com/olofk/edalize/commits/sh) but haven't found time to finish this work

Why is it better than subprocess.Popen(["vivado", "-mode", ...], ...)?

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

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.

The existing solution in Migen also allows this. And Nix can automatically send stuff to one or many build servers...

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.

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.

If this refers to rebuilding an EDA project without requiring python, then it seems you had the answer to your question already.

It refers to building without python. That has been used several times to report problems to Xilinx.

If it fits nmigen, great. More users is generally a good thing.

Well it fits in the sense that it can do the equivalent of a few subprocess.Popen and other calls, but I don't see it solving a difficult problem. And I see the future synthesis flows mostly using Nix to call the EDA programs and not being based on all-in-one Python scripts. The latter are fine for simple projects though.

to quickly gain support for many EDA tools

The difficult part here is writing the tcl and xdc files (or their equivalents for other tools)...

Design goals

1. Equivalent functionality to migen.build
2. Easy migration of Migen platforms to nMigen
3. Maintain portability between Linux/macOS/Windows

Issues with migen.build

• Interfacing with vendor toolchains requires a lot of OS-specific code to format paths, file extensions, build scripts, etc. See lattice/diamond.py, lattice/icestorm.py.

Externalizing toolchain invocation to another tool/library could alleviate the need for this code.

Issues with Edalize

• Requiring Make to build designs is a concern for portability.
• Constraints file formatting must be done by nMigen anyway.

Current approach and limitations

So 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 toolchain.additional_commands, which can be found in platforms/arty_a7.py.

This is a case were we need to add commands to be executed after write_bitstream, and the Edalize template is currently not extensible.
This is also an example of a platform which requires toolchain configuration outside the constraints file.

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.

Issues with migen.build
* Interfacing with vendor toolchains requires a lot of OS-specific code to format paths,

I don't think that's the worst issue with migen.build. Here are other ones:

• error reporting
• support for automatic insertion of differential buffers (a signal can be either defined as differential or single-ended in the platform file, and the appropriate buffer, if needed, would be inserted automatically and transparently to the core using that signal. There should be an option to disable this feature in special case).
• tristate signals and their interaction with the above.
• some constraint names are Xilinx-centric (e.g. IOStandard). Do we need any constraint translation layer anyway or just put everything into something like the current Misc?
• code cleanliness in the pin management system, which is worse than in the "run the EDA tool" part

3\. Maintain portability between Linux/macOS/Windows

Except for the open source flow, I don't think anything runs on Mac.

• Equivalent functionality to migen.build

• Easy migration of Migen platforms to nMigen

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.

In the case of Edalize+Vivado, I do not know how to implement hooks such as toolchain.additional_commands, which can be found in platforms/arty_a7.py.

This is a case were we need to add commands to be executed after write_bitstream, and the Edalize template is currently not extensible.

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

This is also an example of a platform which requires toolchain configuration outside the constraints file.

Can you explain what kind of configuration you're referring too?

Can you explain what kind of configuration you're referring too?

If I'm not mistaken, in the previous example:
write_cfgmem -force -format bin -interface spix4 -size 16 -loadbit \"up 0x0 {build_name}.bit\" -file {build_name}.bin"
should end up in a tcl file separate from the xdc ?

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 migen.build where it would poorly construct ad-hoc build scripts with a lot of code duplication.

I also think it's more important to focus on problems with migen.build such as the pin management system as opposed to bikeshedding the best way to run a few subprocesses.

@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:

• the user may want to generate a Vivado DCP and not a final bitstream (which the external build system will put through additional steps before generating the bitstream itself)
• the external build system may supply itself the Verilog/VHDL files for additional cores (LM32, mor1kx, VexRiscv, Xilinx IP)
• having an external build system provides additional flexibility and features to mix in SpinalHDL or Xilinx IP generator flows.

Having a "batteries included" script that runs all common steps and invokes the EDA tools is fine for simple projects though.

and not try too hard to be a build system

I agree, which is why I think Edalize is the way to go for something that would be built into nMigen.

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.

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.

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.

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?

• Your nix scripts in effect take care of the boilerplate such as generating e.g. the EDA .tcl input files that customize the build steps. In your scenario, Migen only generates the UCF?
• You are open to having nMigen generate EDA input files in addition to a UCF, which you then run directly from your shell rather than from Python subprocesses.
• In all cases, the added functionality should not interfere with your Nix flow.

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.

You can use edalize to just generate a self contained project. See here for an example

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.

• some constraint names are Xilinx-centric (e.g. IOStandard). Do we need any constraint translation layer anyway or just put everything into something like the current Misc?

I agree to put vendor specific constraints in Misc.
What is the purpose of the PlatformInfo constraint ? I do not see it used in Migen platforms.

If IOStandard, Drive and PlatformInfo are not required, we could end up with only three types of constraint: Pins, Subsignal and Misc.

• error reporting
• code cleanliness in the pin management system

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.
As far as I understand, constraints are subject to the following rules:

1. A resource must either have one Pins constraint or at least one Subsignal constraint.
2. A Subsignal constraint must contain a Pins constraint.

These rules are implicitly validated in the _resource_type() function. However, violations are reported by an AssertionError with no further indication.

Despite being treated differently from other constraints, Pins and Subsignals can be positioned anywhere in the constraint list. This lack of distinction forces functions to do the separation themselves. See get_sig_constraints().

In order to make this distinction explicit, I suggest the following model:

An I/O resource is defined by:

• a name,
• a number,
• either one Pins constraint or a list of Subsignal constraints,
• an optional list of Misc constraints.

A Subsignal is defined by:

• a name,
• a Pins constraint,
• an optional list of Misc constraints.

By separating Pins and Subsignals from Misc constraints, resource validation and retrieval should become straightforward.

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.

No to both. Edalize basically just runs the tool; and for the purposes of things like m-labs/nix-scripts#11 I want to be able to disable the Edalize steps completely.

We can split the build in two steps:

1. Toolchain input generation: the Verilog output, constraints file, additional TCL files, etc. are generated by nMigen.
2. Build: files generated from step 1. are fed to Edalize to produce a bitstream.

Step 2. is optional in case the user wants more control over their toolchain.

In order to make this distinction explicit, I suggest the following model:

An I/O resource is defined by:
* a name,
* a number,
* either one Pins constraint or a list of Subsignal constraints,
* an optional list of Misc constraints.

A Subsignal is defined by:
* a name,
* a Pins constraint,
* an optional list of Misc constraints.

By separating Pins and Subsignals from Misc constraints, resource validation and retrieval should become straightforward.

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

        if misc and not all(isinstance(c, Misc) for c in misc):
            raise TypeError("misc has invalid type: should be a list of Misc")

What's the point of Misc then? Just make a list of strings. Also, the usual Python coding style is not to check for types unnecessarily.

Also, the usual Python coding style is not to check for types unnecessarily.

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 str interface but isn't a string are never realized here as well.

I don't think those errors are very hard to debug, and you can still have a similar issue by putting something other than a string inside the Misc object anyway. Python is simply not meant to be used without duck typing.

I don't think those errors are very hard to debug

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.

and you can still have a similar issue by putting something other than a string inside the Misc object anyway

Which is why the Misc object also needs one, yes.

Python is simply not meant to be used without duck typing.

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, ResetInserter and co should be mostly transparent proxies, ideally, which is a single method in Ruby (def method_missing) but in Python I still don't see an especially good way to do that.

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.

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

@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 write_bitstream which would be added by rendering the vivado-run.tcl.j2 template ?

Progress update:

Changes:

• improve constraint error checking
• Subsignals can now be nested

Todo:

• automatic insertion of differential buffers
• split build in two steps, as discussed previously

A working preview including #45 and an Arty A7 platform is available here.

Progress update:

Todo:

• automatic insertion of differential buffers

Automatic insertion of tristate buffers needs to be sorted out first.

Changes:

• Pins take an optional dir argument:

  • valid values are "i", "o" or "io" (default)
  • dir="io" tristates the pin

• <platform>.request() takes an optional dir argument which overrides the direction:

  • the direction can only be changed from "io" to "i" or "o"
  • in case of subsignals, dir must be a dict.
    e.g. platform.request("serial", dir={"tx": "o"})

Progress update:

• Support for differential pairs:
  • They are defined with DiffPairs instead of Pins
  • Identifiers are provided as "P N" pairs
  • Platforms must implement .get_diff_input(), .get_diff_output(), .get_diff_tristate() to return the buffer primitives.
  • Example usage:

Resource("foo", 0,
    DiffPairs(
        "N2 N1",
        "U2 V2",
        dir="io"
    ),
    misc=[Misc("IOSTANDARD=DIFF_SSTL135")]
)

• An ECP5 platform using Trellis+nextpnr has been added.

 misc=[Misc("IOSTANDARD=DIFF_SSTL135")]

i still don't see the point of encapsulating every string in a Misc instance, instead of just writing misc=["IOSTANDARD=DIFF_SSTL135"].

i still don't see the point of encapsulating every string in a Misc instance, instead of just writing misc=["IOSTANDARD=DIFF_SSTL135"].

misc=[Misc("..."), ...] is now extras=["...", "..."]

Changes:

• Overriding dir in <platform>.request() does not mutate global state anymore,
• <platform>.request_raw() can be used to request a bare resource,
• DiffInput and friends have been removed.

Merged.

@olofk By the way, you might want to investigate the solution I made for nMigen instead of Edalize:

• nmigen/nmigen/build/run.py

  Lines 1 to 62 in 3194b5c

  	from collections import OrderedDict
  	import os
  	import sys
  	import subprocess
  	import zipfile
  	__all__ = ["BuildPlan", "BuildProducts"]
  	class BuildPlan:
  	def __init__(self, script):
  	self.script = script
  	self.files = OrderedDict()
  	def add_file(self, filename, content):
  	assert isinstance(filename, str) and filename not in self.files
  	# Just to make sure we don't accidentally overwrite anything.
  	assert not os.path.normpath(filename).startswith("..")
  	self.files[filename] = content
  	def execute(self, root="build", run_script=True):
  	os.makedirs(root, exist_ok=True)
  	cwd = os.getcwd()
  	try:
  	os.chdir(root)
  	for filename, content in self.files.items():
  	dirname = os.path.dirname(filename)
  	if dirname:
  	os.makedirs(dirname, exist_ok=True)
  	mode = "wt" if isinstance(content, str) else "wb"
  	with open(filename, mode) as f:
  	f.write(content)
  	if run_script:
  	if sys.platform.startswith("win32"):
  	subprocess.run(["cmd", "/c", "{}.bat".format(self.script)], check=True)
  	else:
  	subprocess.run(["sh", "{}.sh".format(self.script)], check=True)
  	return BuildProducts(os.getcwd())
  	finally:
  	os.chdir(cwd)
  	def archive(self, file):
  	with zipfile.ZipFile(file, "w") as archive:
  	# Write archive members in deterministic order and with deterministic timestamp.
  	for filename in sorted(self.files):
  	archive.writestr(zipfile.ZipInfo(filename), self.files[filename])
  	class BuildProducts:
  	def __init__(self, root):
  	self._root = root
  	def get(self, filename, mode="b"):
  	assert mode in "bt"
  	with open(os.path.join(self._root, filename), "r" + mode) as f:
  	return f.read()

• nmigen/nmigen/build/plat.py

  Lines 179 to 268 in 3194b5c

  	class TemplatedPlatform(Platform):
  	file_templates = abstractproperty()
  	command_templates = abstractproperty()
  	build_script_templates = {
  	"build_{{name}}.sh": """
  	# {{autogenerated}}
  	set -e{{verbose("x")}}
  	{{emit_commands("sh")}}
  	""",
  	"build_{{name}}.bat": """
  	@rem {{autogenerated}}
  	{{emit_commands("bat")}}
  	""",
  	}
  	def toolchain_prepare(self, fragment, name, **kwargs):
  	# This notice serves a dual purpose: to explain that the file is autogenerated,
  	# and to incorporate
  	autogenerated = "Automatically generated by nMigen {}. Do not edit.".format(__version__)
  	def emit_design(backend):
  	return {"rtlil": rtlil, "verilog": verilog}[backend].convert(
  	fragment, name=name, platform=self, ports=list(self.iter_ports()),
  	ensure_sync_exists=False)
  	def emit_commands(format):
  	commands = []
  	for index, command_tpl in enumerate(self.command_templates):
  	command = render(command_tpl, origin="<command#{}>".format(index + 1))
  	command = re.sub(r"\s+", " ", command)
  	if format == "sh":
  	commands.append(command)
  	elif format == "bat":
  	commands.append(command + " || exit /b")
  	else:
  	assert False
  	return "\n".join(commands)
  	def get_tool(tool):
  	tool_env = tool.upper().replace("-", "_")
  	return os.environ.get(tool_env, tool)
  	def get_override(var):
  	var_env = "NMIGEN_{}".format(var)
  	if var_env in os.environ:
  	return os.environ[var_env]
  	elif var in kwargs:
  	return kwargs[var]
  	else:
  	return jinja2.Undefined(name=var)
  	def verbose(arg):
  	if "NMIGEN_verbose" in os.environ:
  	return arg
  	else:
  	return jinja2.Undefined(name="quiet")
  	def quiet(arg):
  	if "NMIGEN_verbose" in os.environ:
  	return jinja2.Undefined(name="quiet")
  	else:
  	return arg
  	def render(source, origin):
  	try:
  	source = textwrap.dedent(source).strip()
  	compiled = jinja2.Template(source, trim_blocks=True, lstrip_blocks=True)
  	except jinja2.TemplateSyntaxError as e:
  	e.args = ("{} (at {}:{})".format(e.message, origin, e.lineno),)
  	raise
  	return compiled.render({
  	"name": name,
  	"platform": self,
  	"emit_design": emit_design,
  	"emit_commands": emit_commands,
  	"get_tool": get_tool,
  	"get_override": get_override,
  	"verbose": verbose,
  	"quiet": quiet,
  	"autogenerated": autogenerated,
  	})
  	plan = BuildPlan(script="build_{}".format(name))
  	for filename_tpl, content_tpl in self.file_templates.items():
  	plan.add_file(render(filename_tpl, origin=filename_tpl),
  	render(content_tpl, origin=filename_tpl))
  	for filename, content in self.extra_files.items():
  	plan.add_file(filename, content)
  	return plan

• https://github.com/m-labs/nmigen/blob/master/nmigen/vendor/fpga/lattice_ice40.py#L16-L109

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).