fusesoc
fusesoc copied to clipboard
olofk
Support include paths beyond is_include_file
(Forwarding this from an internal bug report.)
DV packages could have 100s of included files (these are not headers, but SV classes). For compilation, all that's needed is the path to the pkg in the .f and the required search paths for the sources included in the pkg via incdirs. I think it would be better if fusesoc gained capability to add search paths rather than having to add all sources again from pkg file to .core file. Would addition of something like this be feasible:
filesets:
files_dv:
depend:
- ...
files:
- ...
paths:
- ...
This breaks per-file dependency tracking and copying of source files if we only use paths to set up incdir. Hence we should also need to add support to track all files within the proposed paths entry as source file and copy it to the build dir as needed.
Another option could be something like
files:
- rtl/include {is_include_dir: true}
I'm against adding support for FuseSoC to automatically pick up files. If there are many files, it's still easy to just write a quick bash one-liner to get all the files and paste into the core file. Another option would be to add a generator that tries to identify file types automatically and adds files. This is something I have been meaning to write at some point.
With that said, I know there can be reasons for adding an include dir that doesn't directly contain any include files. E.g. if the verilog file tries to include some_dir/some_file.vh then we would need to have some_dir as an include path. This is something I would be willing to have a patch for. My instinct has been to add another entry to the target, but putting it as an entry within a fileset (or even as a pseudo file as in your example) might be a better choice
This is to support DV usecase where a 'reusable' verification unit is contained within a SV package. In such a case, the tool (verilator, vcs etc) only needs a .f file that lists the package for compilation. The package itself could contain several SV classes that are `included.
Example:
package mypkg;
// this resides in ./
`include "myclass1.sv"
// this resides in ./foo
`include "myclass2.sv"
...
endpackage
mypkg.f:
+incdir+.
+incdir+./foo
mypkg.sv
The current .core file does not have the ability to just add the incdir for foo. Instead, user has to add a single 'myclass2.sv' entry in the .core file and set is_included_file to true for that one to 'force' the fusesoc tool to add the incdir. I feel this is not an ideal solution for what I'm trying to achieve. Note that fusesoc is NOT picking up files automatically, it just need to have support to specify incdir where and existing file in a package the sim tool is compiling is physically located.
TL; DR; version: Yes, this is to not let fusesoc automatically pick up files (which I agree is bad), but just provide ability to specify in include path (as you indicated in your second paragraph).
As I said in the initial report, passing an arbitrary incdir to the tool without tracking the files inside this directory breaks the dependency tracking: all files which make up the simulation must be declared in a core file, and hence known to fusesoc. The dependency tracking is an essential part of a build system, as it is the basis for caching and cache invalidation of build artifacts. (For example, it allows you to rebuild the simulation only if a source file changed.)
Today, you can achieve that in fusesoc by adding all files individually to a core file. If you have 100s of files, a simple one-liner script with ls gives you a list of files you can then copy-paste into a core file.
Tracking a whole directory of source files, as proposed in this PR, could be an in-between solution which continues to enable dependency tracking, and at the same time avoids copy-pasting long lists of file names into core files. But I must say, I'm not convinced that this is actually a good thing, and I'd simply stay with what we have right now.
@sriyerg can you give it a try, and see how painful the current setup actually is?
VCS already solves the problem of recompiling a package (and re-elaborate its partition) if a single file included in the package is changed. Compilation works at package granularity, not at individual file level granularity. The .f file supplied to VCS only needs the package file (plus incdirs for other directories where the sources might exist). If VCS does not need it, fusesoc should not, either. We will always run with --no-export option, so source file tracking is not necessary.
To clarify, in the current setup, I don't even have to add all files to the .core file - I just need to pick one file at random from each unique directory and add it to the .core file and mark it as 'is_included_file' so that fusesoc adds the corresponding incdir. Beyond that, fusesoc does not any any additional processing AFAIK. I just feel that rather than do this, it might be better to just provide 'is_include_dir' option instead.
So to sum it up, we're looking for a tool-agnostic way to add include directories which doesn't require a file with is_include_file to reside in the directory. All good. Not sure if it would be best to put in the fileset or target so let's bikeshed that.
Slightly related to the discussion is that I would like to do something smarter that only updates files in the build dir that have actually changed to make it easier to do incremential rebuilds. But we can open a new issue for that
Forgot about this one, but there was a fix recently that might help in these situations by setting include_path. Would be interested in knowing if that is of use for you https://fusesoc.readthedocs.io/en/master/ref/capi2.html#file