byuccl / bfasst

Tools for FPGA Assurance Flows
Apache License 2.0
12 stars 4 forks source link

Build folders created by Vivado #347

Closed jgoeders closed 7 months ago

jgoeders commented 7 months ago

Currently Vivado creates an in_context directory, and synth and impl and created as subdirectories.

I would like to remove the in_context directory, and just have the synth and impl directories, so that the directory structure is consistent regardless of what synth/impl tool is used.

yonnorc42 commented 7 months ago

When I run designs, it creates an in_context and ooc directory, both of which contain synth and impl as subdirectories, would I have to differentiate between the in and out of context synth and impl directories by naming them something like ooc_synth/impl and in_context_synth/impl?

jgoeders commented 7 months ago

@reillymck @KeenanRileyFaulkner @dallinjdahl Probably worth a larger discussion here.

I think I would prefer to just have "synth" and "impl". Making a new directory for every variation of synthesis tool seems potentially problematic. For example, suppose one flow wanted to run synthesis with DSPs, but another flow wanted synthesis without DSPs. Would we then create more "synth_*" sub-directories to distinguish between these synthesis options?

I understand the reasoning for needing separate directories since we moved to a build/design/flow structure, but I think that's going to cause us more problems than it's worth.

It seems safer to me to go back to build/flow/design to make sure variations in how different flows may invoke the tools do not cause conflicts that are missed.

Thoughts?

dallinjdahl commented 7 months ago

The way this is currently set up is that we treat normal vs ooc as separate tools, which happen to both be implemented by vivado. This choice is validated due to the significant difference in the outputs and build scripts between the two.

The reason we switched to build/design/flow is so that we can reuse results from previous runs if they are relevant. That relevance check is done by ninja, and we've taken pains to ensure that the dependency graph is complete. If it isn't that's a bug.

As an example of this completeness, the flow_args are templated into viv_synth.tcl.mustache to generate viv_synth.tcl. The synthesis step explicitly depends on viv_synth.tcl, and the templating step depends on the mustache file. If the flow_args change or the mustache file is updated, everything that depends on that is considered out of date and will be overwritten, so there should be no conflicts. If you want to keep the results for a given run, those files (under the current paradigm) should be copied elsewhere for archival purposes. This mirrors the way most command line utilities work on linux.

Another note is that we don't support running multiple flows against a given design in a single run, which means that the flow_args can't change within a run, so a run is guaranteed to be internally consistent.

In conclusion, normal vs ooc is a much larger difference than with/without DSPs, so they are treated as separate tools. Anything specified as flow_args is done within the respective version (ooc or normal), and build targets are reused when possible, and invalidated when necessary.

dallinjdahl commented 7 months ago

For additional insight into the level of completeness, the following link illustrates the relevant aspect: https://cr.yp.to/redo/honest-script.html

ninja automatically records dependencies on the rules and builds, and we specify all the pertinent files in the dependency list. That includes build scripts like the tcl file. Currently the line is drawn at utilities. Utilities are expected to be present and functional, and we do not depend on them explicitly.

The fuzzy area is in ninja_utils. We currently treat them as utilities, and they are intended to be written as such, so that they could run as a standalone tool if necessary. However, we could treat them as build scripts with respect to dependencies.

KeenanRileyFaulkner commented 7 months ago

The fuzzy area is in ninja_utils. We currently treat them as utilities, and they are intended to be written as such, so that they could run as a standalone tool if necessary. However, we could treat them as build scripts with respect to dependencies.

With regards to this, I have an issue open, #336 that will add the ninja_utils python scripts as implicit dependencies wherever they are used by ninja.

reillymck commented 7 months ago

I agree with Dallin. And at least in my use case, I generally am running vivado with the same arguments between runs, although the tools afterwards differ. The mustache file would cause the vivado run to be redone if those arguments changed, but as is I think it's useful to have the file structure work on the assumption that vivado runs will be the same vs each flow will require a new run.

jgoeders commented 7 months ago

If the flow_args change or the mustache file is updated, everything that depends on that is considered out of date and will be overwritten, so there should be no conflicts.

OK, that's good to know. Given that then I agree keeping it as build/design/flow is ideal.

I would just stick with synth and impl for the usual flow. I agree that ooc synthesis is different enough to call it synth_ooc, but also, I'm not sure the motivation of calling it something else? All of the synthesis tools thus far have just used the synth folder, versus having tool-specific naming (eg synth_vivado, synth_icecube, etc). But if a flow is going to run both ooc and regular synthesis, then I suppose having separate names would be important.

Is there such as thing as out-of-context implementation?

reillymck commented 7 months ago

There is such a thing as ooc implementation, it just needs to be ran in the same vivado session as implementation. In my experience, opening an checkpoint from a ooc design post-synthesis and then running implementation does not work. I'm not sure if I tried this with Vivado 2023.1 and I definitely have not tested it with 2023.2