Skip to content

Conversation

jerryz123
Copy link
Contributor

Related PRs / Issues:

Type of change:

  • Bug fix
  • New feature
  • Other enhancement

Impact:

  • RTL change
  • Software change (RISC-V software)
  • Build system change
  • Other

Contributor Checklist:

  • Did you set main as the base branch?
  • Is this PR's title suitable for inclusion in the changelog and have you added a changelog:<topic> label?
  • Did you state the type-of-change/impact?
  • Did you delete any extraneous prints/debugging code?
  • Did you mark the PR with a changelog: label?
  • (If applicable) Did you update the conda .conda-lock.yml file if you updated the conda requirements file?
  • (If applicable) Did you add documentation for the feature?
  • (If applicable) Did you add a test demonstrating the PR?
  • (If applicable) Did you mark the PR as Please Backport?

CI Help:
Add the following labels to modify the CI for a set of features.
Generally, a label added only affect subsequent changes to the PR (i.e. new commits, force pushing, closing/reopening).
See ci:* for full list of labels:

  • ci:fpga-deploy - Run FPGA-based E2E testing
  • ci:local-fpga-buildbitstream-deploy - Build local FPGA bitstreams for platforms that are released
  • ci:disable - Disable CI

@jerryz123 jerryz123 changed the base branch from main to modular-more September 1, 2025 18:27
Copy link
Contributor

@iansseijelly iansseijelly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I guess this is good enough for now, although fragmentation is increasing

.settings(commonSettings)
.settings(scalaTestSettings)
if (!useChisel7) {
hf = hf.dependsOn(midas_target_utils)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh... how does this work? regardless of useChisel7 or not, wouldn't hf require midas as a dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was our hacky way to make everything depend on midas-target-utils in the pre-chisel7 flow

@jerryz123
Copy link
Contributor Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex Review: Here are some suggestions.

About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".

@jerryz123 jerryz123 force-pushed the chisel7 branch 2 times, most recently from d8e653f to 5986b99 Compare September 8, 2025 07:34
@jerryz123 jerryz123 merged commit feef6e0 into main Sep 11, 2025
71 of 72 checks passed
@tianrui-wei
Copy link
Member

This currently breaks Boom RTL generation in chisel7, even with chisel version set to 7.0.0, and can be reproduced by running

make verilog USE_CHISEL7=1 CONFIG=MediumBoomV3Config

Specifically, the issue arises from after running the firtool transform, the rob_debug_inst_mem gets transformed into rob_debug_inst_mem_ext in the mems.conf file, but not the model_module_hierarchy.json. I'm not sure whether this is an issue in sfc or firtool. Any ideas?

@tianrui-wei
Copy link
Member

Tracking llvm/circt#9012, we should bump the firtool after the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants