Skip to content

Conversation

seldridge
Copy link
Member

@seldridge seldridge commented Jul 9, 2025

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.

Release Notes

  • Stop relying on CIRCT-generated filelists in ChiselSim. This is needed for compatibility with CIRCT 1.125.0.

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>
@seldridge seldridge added the Feature New feature, will be included in release notes label Jul 9, 2025
@seldridge seldridge requested a review from jackkoenig July 9, 2025 22:17
@seldridge
Copy link
Member Author

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))
Copy link
Contributor

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.

Copy link
Member Author

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

@seldridge
Copy link
Member Author

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 .fir files. 🤞

@seldridge seldridge merged commit ed29848 into main Jul 9, 2025
15 checks passed
@seldridge seldridge deleted the dev/seldridge/chiselsim-stop-using-filelists branch July 9, 2025 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature, will be included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants