-
Notifications
You must be signed in to change notification settings - Fork 636
[chiselsim] Stop using CIRCT filelists #4969
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
Conversation
Change ChiselSim to stop using undocumented CIRCT-generated filelists to know what files to include in simulator compile. Specifically, don't rely on the existence of `filelist.f` or `firrtl_black_box_resource_files.f`. This is needed because the latter file was removed from (the unreleased) CIRCT 1.125.0 [[1]] for reasons that it wasn't supposed to be load bearing. Removing the reliance on the former is just good to do. Note: this may result in some breakages as users may have been inadvertently relying on this behavior to exclude some generated files from their compilation. E.g., a user could generate some Chisel-time metadata file which gets put into the build area, but then is excluded from the compile. To work around this, users should dump these files in an alternative area. [1]: llvm/circt@c7e10d7 Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
I'm testing this internally to make sure nothing breaks. This is expected to fix the nightly breakage here: https://github.com/chipsalliance/chisel/actions/runs/16167653880 |
Files | ||
.walk(supportArtifactsPath) | ||
.filter(_.toFile.isFile) | ||
.filter(_.getFileName.toString.startsWith("layers-")) | ||
.filter(f => !skip_re.matches(f.getFileName.toString)) |
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.
It might be marginally cleaner and faster to do this based on the file extension rather than a regex but it ain't that deep.
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.
I actually started with an endsWith
, but backed off because I thought we would need to ignore more than just .f
. I remembered that --dump-fir
could be a problem as that dumps .fir
files in the build area.
Instead of the simpler approach, I kept the regular expression and beefed it up to also ignore .fir
. This should justify its use more as I don't know of a clean way to skip by file extension other than iterating through them all (which seems kind of jank).
There may be a bit of whack-a-mole with this file exclusion approach. It would be better if we could break up the builds into distinct Chisel elaboration outputs and distinct FIRRTL compilation outputs. I'm signing up to deal with any fallout of this, though. I ran this internally and didn't see any problems other than needing to also exclude |
Change ChiselSim to stop using undocumented CIRCT-generated filelists to
know what files to include in simulator compile. Specifically, don't rely
on the existence of
filelist.f
orfirrtl_black_box_resource_files.f
.This is needed because the latter file was removed from (the unreleased)
CIRCT 1.125.0 [1] for reasons that it wasn't supposed to be load
bearing. Removing the reliance on the former is just good to do.
Note: this may result in some breakages as users may have been
inadvertently relying on this behavior to exclude some generated files
from their compilation. E.g., a user could generate some Chisel-time
metadata file which gets put into the build area, but then is excluded
from the compile. To work around this, users should dump these files in
an alternative area.
Release Notes