artiq-fast: build gateware from a self-contained separate source derivation #23

Merged
sb10q merged 6 commits from split-software-gateware-builds into master 2020-12-02 17:22:55 +08:00
Contributor

Addresses item 2. of Gitea issue #1.

Calls the Artiq target script with --no-compile-gateware, additionally installs the generated bitstream source code, and uses builtins.unsafeDiscardStringContext on its contents. From the dangerous look of that function I wanted some code review. I think it's pure and fine to use though...

That's what is needed to drop all dependency tracking from the
gateware source so that the gateware will be rebuilt only if the
sources have changed, not for anything unrelated in the Python
code.

Addresses item 2. of Gitea issue #1. Calls the Artiq target script with `--no-compile-gateware`, additionally installs the generated bitstream source code, and uses `builtins.unsafeDiscardStringContext` on its contents. From the dangerous look of that function I wanted some code review. I think it's pure and fine to use though... That's what is needed to drop all dependency tracking from the gateware source so that the gateware will be rebuilt only if the sources have changed, not for anything unrelated in the Python code.
Owner

Some random comments:

  • The "gateware source" terminology is confusing as it could also refer to the Python Migen sources. Using "generated gateware source" is also inappropriate as there are some files such as the mor1kx Verilog that are not generated. I propose to use consistently "Vivado inputs", "FPGA synthesis inputs", or similar.
  • Instead of using a substituteInPlace hack on the TCL file, maybe it is better to modify Migen so that it copies all input Verilog files into the build directory and makes it self-contained, with relative paths. This may also help with some path-related issues that people have had with running Migen under Cygwin, which were solved by introducing Cygwin-specific code which would be nice to remove if possible. William D. Jones (cr1901) is the person to talk to for Migen-on-Cygwin matters.
  • You may want this first derivation to output a placed-and-routed Vivado checkpoint (DCP file), which is then taken in by a second derivation that opens it again with Vivado, edits the BRAM contents to set the version tag, and writes the final bitfile. Migen may need to be modified to support generating a DCP (only) instead of a BIT.
  • Merging this before it is finished will break the gateware version tag, which is not acceptable particularly on the stable branch.
Some random comments: * The "gateware source" terminology is confusing as it could also refer to the Python Migen sources. Using "generated gateware source" is also inappropriate as there are some files such as the mor1kx Verilog that are not generated. I propose to use consistently "Vivado inputs", "FPGA synthesis inputs", or similar. * Instead of using a ``substituteInPlace`` hack on the TCL file, maybe it is better to modify Migen so that it copies all input Verilog files into the build directory and makes it self-contained, with relative paths. This may also help with some path-related issues that people have had with running Migen under Cygwin, which were solved by introducing Cygwin-specific code which would be nice to remove if possible. William D. Jones (cr1901) is the person to talk to for Migen-on-Cygwin matters. * You may want this first derivation to output a placed-and-routed Vivado checkpoint (DCP file), which is then taken in by a second derivation that opens it again with Vivado, edits the BRAM contents to set the version tag, and writes the final bitfile. Migen may need to be modified to support generating a DCP (only) instead of a BIT. * Merging this before it is finished will break the gateware version tag, which is not acceptable particularly on the stable branch.
Author
Contributor

Very useful details. Thank you!

Very useful details. Thank you!
Owner

Another two things:

  • Let's deploy this on the beta branch only first. Copy the current artiq-board.nix to artiq-board-legacy.nix and use one or the other based on pkgs.lib.strings.versionAtLeast artiq-fast.artiq.version "6.0".
  • Using a whole BRAM to store just a few bytes of data is wasteful, maybe it would also work with primitives like ROM256X1 (see: https://www.xilinx.com/support/documentation/sw_manuals/xilinx2012_2/ug953-vivado-7series-libraries.pdf). One needs to check if their contents can be modified using the Vivado TCL commands even after synthesis, placement and routing.
    We need to support boards with Artix-7, Kintex-7, Zynq-7000 FPGAs which all have the same primitives ("7-series"), and with Kintex Ultrascale which does not. Find a suitable common primitive between KU and 7-series.
Another two things: * Let's deploy this on the beta branch only first. Copy the current ``artiq-board.nix`` to ``artiq-board-legacy.nix`` and use one or the other based on ``pkgs.lib.strings.versionAtLeast artiq-fast.artiq.version "6.0"``. * Using a whole BRAM to store just a few bytes of data is wasteful, maybe it would also work with primitives like ``ROM256X1`` (see: https://www.xilinx.com/support/documentation/sw_manuals/xilinx2012_2/ug953-vivado-7series-libraries.pdf). One needs to check if their contents can be modified using the Vivado TCL commands even after synthesis, placement and routing. We need to support boards with Artix-7, Kintex-7, Zynq-7000 FPGAs which all have the same primitives ("7-series"), and with Kintex Ultrascale which does not. Find a suitable common primitive between KU and 7-series.
Author
Contributor

Please review again.

I do not think I have access to hardware for testing.

Please review again. I do not think I have access to hardware for testing.
Owner

You do, there's a KC705 on rpi-1.m-labs.hk and a Kasli 2.0 (without Ethernet) on hestia.m-labs.hk.

You do, there's a KC705 on rpi-1.m-labs.hk and a Kasli 2.0 (without Ethernet) on hestia.m-labs.hk.
Author
Contributor

Tested on KC705:

[     0.003842s]  INFO(runtime): software ident unprogrammed
[     0.009149s]  INFO(runtime): gateware ident 6.7357.3d841358.beta;nist_clock
Tested on KC705: ```text [ 0.003842s] INFO(runtime): software ident unprogrammed [ 0.009149s] INFO(runtime): gateware ident 6.7357.3d841358.beta;nist_clock ```
Author
Contributor

Updated for --gateware-identifier-str

Updated for `--gateware-identifier-str`
Author
Contributor

Rebased

Rebased
Owner
  • Did you test that the buildEnv output from artiq-board.nix can be installed as a Python package as per the example nix-shell file in the manual, and artiq_flash will find the binaries?
  • Did you test conda package generation?
  • It seems it will still display software ident unprogrammed in the log, which is a pretty useless message. I suggest dropping this message (and all associated code) entirely, please submit a ARTIQ PR that does that.
* Did you test that the buildEnv output from artiq-board.nix can be installed as a Python package as per the example nix-shell file in the manual, and artiq_flash will find the binaries? * Did you test conda package generation? * It seems it will still display ``software ident unprogrammed`` in the log, which is a pretty useless message. I suggest dropping this message (and all associated code) entirely, please submit a ARTIQ PR that does that.
Author
Contributor

Did you test that the buildEnv output from artiq-board.nix can be installed as a Python package as per the example nix-shell file in the manual, and artiq_flash will find the binaries?

Fixed now: 81b1af7

Did you test conda package generation?

Just did: they include the firmware and the bitstream.

It seems it will still display software ident unprogrammed in the log, which is a pretty useless message. I suggest dropping this message (and all associated code) entirely, please submit a ARTIQ PR that does that.

That is strange. I just inserted a grep -r IDENTIFIER_STR into the firmware build:

artiq_kasli/tester/software/include/generated/csr.rs:  pub const CONFIG_IDENTIFIER_STR: &'static str = "6.7499.3e388330.beta;tester";
artiq_kasli/tester/software/include/generated/csr.h:#define CONFIG_IDENTIFIER_STR "6.7499.3e388330.beta;tester"

I'm going to run the firmware on a device...

> Did you test that the buildEnv output from artiq-board.nix can be installed as a Python package as per the example nix-shell file in the manual, and artiq_flash will find the binaries? Fixed now: 81b1af7 > Did you test conda package generation? Just did: they include the firmware and the bitstream. > It seems it will still display software ident unprogrammed in the log, which is a pretty useless message. I suggest dropping this message (and all associated code) entirely, please submit a ARTIQ PR that does that. That is strange. I just inserted a `grep -r IDENTIFIER_STR` into the firmware build: ``` artiq_kasli/tester/software/include/generated/csr.rs: pub const CONFIG_IDENTIFIER_STR: &'static str = "6.7499.3e388330.beta;tester"; artiq_kasli/tester/software/include/generated/csr.h:#define CONFIG_IDENTIFIER_STR "6.7499.3e388330.beta;tester" ``` I'm going to run the firmware on a device...
Owner

artiq-board-legacy.nix would need these changes
12fbe64616

artiq-board-legacy.nix would need these changes https://git.m-labs.hk/M-Labs/nix-scripts/commit/12fbe646167aecb37452b595396d7c4f0d7b94a6
Author
Contributor

artiq-board-legacy.nix would need these changes
12fbe64616

(Related to #34) Yet I can't find a artiq-board-legacy.nix anywhere in the repository and its history. Please help. I may be working on too many PRs over too long time periods. :-)

> artiq-board-legacy.nix would need these changes > 12fbe64616 ~~(Related to #34) Yet I can't find a `artiq-board-legacy.nix` anywhere in the repository and its history. Please help.~~ I may be working on too many PRs over too long time periods. :-)
Owner

You are creating it in this PR but did not integrate the changes from https://git.m-labs.hk/M-Labs/nix-scripts/commit/12fbe64616.

You are creating it in this PR but did not integrate the changes from https://git.m-labs.hk/M-Labs/nix-scripts/commit/12fbe64616.
Author
Contributor

Tested this on the kc705 on rpi-1:

[     0.003844s]  INFO(runtime): software ident 6.7499.3e388330.beta;nist_clock
[     0.010801s]  INFO(runtime): gateware ident 6.7499.3e388330.beta;nist_clock
Tested this on the kc705 on rpi-1: ``` [ 0.003844s] INFO(runtime): software ident 6.7499.3e388330.beta;nist_clock [ 0.010801s] INFO(runtime): gateware ident 6.7499.3e388330.beta;nist_clock ```
sb10q merged commit eab83a6a3f into master 2020-12-02 17:22:55 +08:00
Owner

Hmm, this seems to cause the firmware to be built during the Hydra initial evaluation step. May cause timeouts and improperly reported errors...

Hmm, this seems to cause the firmware to be built during the Hydra initial evaluation step. May cause timeouts and improperly reported errors...
astro deleted branch split-software-gateware-builds 2020-12-02 22:55:56 +08:00
Author
Contributor

Well yes, Nix needs to find out whether the Vivado input has changed.

Build the firmware in another separate step?

Well yes, Nix needs to find out whether the Vivado input has changed. Build the firmware in another separate step?
Owner

What if we create one Hydra jobset that builds the firmware and the Verilog source, which becomes the input of another? Evaluations (with all the firmware builds) are taking over an hour right now, with no feedback in the Hydra web interface.

What if we create one Hydra jobset that builds the firmware and the Verilog source, which becomes the input of another? Evaluations (with all the firmware builds) are taking over an hour right now, with no feedback in the Hydra web interface.
Author
Contributor

I'll give that a try. What's a good name for this intermediate jobset? It's going to be an additional targets jobs.

I'll give that a try. ~~What's a good name for this intermediate jobset?~~ It's going to be an additional *targets* jobs.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/nix-scripts#23
No description provided.