Skip to content
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

Add GPU project chipyard changes #2190

Draft
wants to merge 389 commits into
base: main
Choose a base branch
from
Draft

Conversation

richardyrh
Copy link

@richardyrh richardyrh commented Feb 7, 2025

  • Added Virgo/Radiance configs, memory coalescer configs, as well as a firesim config
  • Added the radiance submodule
  • Bumped rocket-chip, testchipip and gemmini with new commits
  • Minor sbt and Makefile build system additions

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

hansungk and others added 30 commits November 22, 2023 16:27
Includes GPR_RESET which uses simulation-only initial block to clear out
regfile.
@richardyrh
Copy link
Author

common.mk Outdated
@@ -233,6 +236,7 @@ $(TOP_SMEMS_CONF) $(MODEL_SMEMS_CONF) &: $(MFC_SMEMS_CONF) $(MFC_MODEL_HRCHY_JS
--model-module-name $(MODEL) \
--out-dut-smems-conf $(TOP_SMEMS_CONF) \
--out-model-smems-conf $(MODEL_SMEMS_CONF)
cat $(base_dir)/vlsi/add.mems.conf >> $(TOP_SMEMS_CONF)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this for?

Copy link
Author

Choose a reason for hiding this comment

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

ah i had a comment here, somehow missed it and didnt push. updated, it's for the srams we use inside of vortex

Copy link
Contributor

@jerryz123 jerryz123 left a comment

Choose a reason for hiding this comment

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

What is the use case for run-binaries-debug-bg? It just runs them in the background? Can't you just do make run-binaries .... & to do that?

Also, what is the change to riscv-tools-feedstock for?

@@ -46,6 +46,9 @@ dependencies:
- findutils
- lzop

# dpi module
- rust
Copy link
Contributor

Choose a reason for hiding this comment

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

🦀 . I believe you'll need to regenerate the lockfile too

Copy link
Contributor

Choose a reason for hiding this comment

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

ope I don't think we want this in here just yet

import org.chipsalliance.cde.config.Config
import radiance.subsystem.RadianceGemminiDataType

class WithRadBootROM(address: BigInt = 0x10000, size: Int = 0x10000, hang: BigInt = 0x10100) extends Config((site, here, up) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this to the right file in config/fragments.

Also maybe we can merge this one first: chipsalliance/rocket-chip#3699? That will allow SoCs with multiple ROMs (one for normal core, and one for GPU cores).

@@ -38,4 +38,5 @@ SIM_PREPROC_DEFINES = \
+define+RANDOMIZE_MEM_INIT \
+define+RANDOMIZE_REG_INIT \
+define+RANDOMIZE_GARBAGE_ASSIGN \
+define+RANDOMIZE_INVALID_ASSIGN
+define+RANDOMIZE_INVALID_ASSIGN \
$(EXTRA_SIM_PREPROC_DEFINES)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add this to HELP_COMPILATION_VARIABLES in common.mk

@@ -151,3 +151,6 @@
[submodule "generators/vexiiriscv"]
path = generators/vexiiriscv
url = https://github.com/ucb-bar/vexiiriscv-tile.git
[submodule "generators/radiance"]
Copy link
Contributor

Choose a reason for hiding this comment

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

You should add radiance to the .github/scripts/check-commit.sh script

new freechips.rocketchip.subsystem.WithNBanks(4) ++
new chipyard.config.WithSystemBusWidth(16 * 8) ++
// Small Rocket core that does nothing
new radiance.subsystem.WithNCustomSmallRocketCores(1) ++
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you just remove the cores? I think that should be supported (Look at NoCoresConfig).

@richardyrh
Copy link
Author

What is the use case for run-binaries-debug-bg? It just runs them in the background? Can't you just do make run-binaries .... & to do that?

@hansungk's changes

Also, what is the change to riscv-tools-feedstock for?

Didn't mean to change that, will revert

@hansungk
Copy link
Contributor

hansungk commented Feb 7, 2025

run-binaries-debug-bg makes sure the compile stage before sim is serialized. Was mostly for personal use though, we can leave it out

@jerryz123
Copy link
Contributor

run-binaries-debug-bg makes sure the compile stage before sim is serialized. Was mostly for personal use though, we can leave it out

Hmm, the Makefile dependencies should already be enforcing compile ahead of sim... unless there's something which the Makefile isn't aware of that is a dependency.

@hansungk
Copy link
Contributor

hansungk commented Feb 7, 2025

Yep, it's just that if you kick off many run-binaries-debug &, they might compile the same thing concurrently and might mangle some states. was mostly used for same config but different binary launches

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.

4 participants