rocket-chip icon indicating copy to clipboard operation
rocket-chip copied to clipboard

Migration to new world Chisel.

Open sequencer opened this issue 3 years ago • 9 comments

In the Chisel 3.6, the compatibility layer of Chise2 will be finally deprecated after chipsalliance/chisel3#2668 get merged, In the open-source community, RC is obviously the biggest downstream user of this compatibility layer. As the downstream of Chisel project, RC will be migrate to 3.6/3.7, and not blocking Chisel development. Something like asking Chisel to keep the compatibility layer or adding special hack to RC(see here) should be avoided. In this Issue, I'm proposing a migration procedure: 0. releasing development change:

  • use "stable" + "master" branch, "stable" will depend on the latest stable version of Chisel, "master" will depend on the SNAPSHOT version.
  • add mergify to RC.
  • new PR creating should be add tags to indicate when to backport to stable version
  • a chisel version bumping should also include all backport PRs which has the corresponding version tag.
  • only release stable version
  1. Migration step.
  • setup a migrate guideline and review guideline, basically copy&paste&review from https://github.com/chipsalliance/chisel3/blob/master/docs/src/appendix/chisel3-vs-chisel2.md and https://github.com/ucb-bar/chisel2-deprecated#chisel3
  • Set up a migrating branch, all migration work(PR&review) at there, which will always be rebased to master.
  • Move all files with import Chisel._ in to a new migrate folder, mark them as files to be migrated.
  • Find code reference to which(thanks Intellij IDEA), rewrite them in the original path and change the code reference to the new version.
  • After there is no compatibility layer files, merge migrating branch into master and finish the modern chisel porting.

Type of issue: feature request

Impact: no functional change

Development Phase: request

Other information chipsalliance/chisel3#2668 https://github.com/chipsalliance/chisel3/blob/master/docs/src/appendix/chisel3-vs-chisel2.md https://github.com/ucb-bar/chisel2-deprecated#chisel3

sequencer avatar Aug 12 '22 06:08 sequencer

On changing development procedure:

  • Do we need this "stable" Chisel branch? How frequently do we expect PRs relying on bleeding-edge chisel features? This does not seem to be frequent, most PRs will not care about chisel version.
  • Note currently master depends on some versioned chisel release. I don't see a reason to make it depend on SNAPSHOT.
  • Relying on SNAPSHOT features is an antipattern IMO, since SNAPSHOT features are subject to change before release

On Chisel2->chisel3 migration:

  • Why not just merge 1-by-1 into master? I prefer this more aggressive approach, otherwise we might have a migration branch that will frequently have merge conflicts with master.
  • I don't think I understand the motivation for a migrate folder, or for the "code reference" thing you mention. grep does a fine job at finding everything that needs to be migrated

jerryz123 avatar Aug 12 '22 06:08 jerryz123

Sorry, I overslept yesterday.

Do we need this "stable" Chisel branch? How frequently do we expect PRs relying on bleeding-edge chisel features? This does not seem to be frequent, most PRs will not care about chisel version.

In theory, yes, but there are a lot of features which can be leveraged from new chisel, such as D/I, decoder(almost wait for a half year), which means, new language features landing in RC might be slowdown by one or half an years. AFAIK, SiFive is using almost the master branch in their development. So I'm proposing a more aggressive development flow: use chisel master branch in the RC master or dev branch, after new Chisel releasing, we can backport new features to stable or master branches.

Why not just merge 1-by-1 into master? I prefer this more aggressive approach, otherwise we might have a migration branch that will frequently have merge conflicts with master.

Sure, I think it's reasonable to do this aggressive approach as well. I'll draft a doc before we starting this.

sequencer avatar Aug 25 '22 03:08 sequencer

Chisel3 Port Status

After #3175, there are still ~105 files to port.

  • [x] hardfloat - 27 (listed at https://github.com/ucb-bar/berkeley-hardfloat/pull/48)
  • diplomacy - 9
    • [x] Clone
    • [x] CloneModule
    • [x] Parameters
    • [x] SRAM
    • [x] Nodes
    • [x] LazyModule
    • [x] Resources
    • [x] AddressDecoder
    • [x] package
  • tile - 7
    • [x] Interrupts
    • [x] BusErrorUnit
    • [x] RocketTile
    • [x] Core
    • [x] L1Cache
    • [x] BaseTile
    • [x] FPU
  • util - 23
    • [x] CRC
    • [ ] ShiftReg
    • [x] GeneratorUtils
    • [ ] PSDTestMode
    • [ ] Frequency
    • [x] ReorderQueue
    • [x] ROMGenerator
    • [x] ECC
    • [x] Counters
    • [x] ResetCatchAndSync
    • [x] IdentityModule
    • [x] HeterogeneousBag
    • [x] GenericParameterizedBundle
    • [x] MuxLiteral
    • [x] Property
    • [x] MultiWidthFifo
    • [x] LatencyPipe
    • [x] Misc
    • [x] Arbiters
    • [x] HellaQueue
    • [x] LCG
    • [x] Annotations
    • [x] Broadcaster
  • devices/tilelink - 10
    • [x] Error
    • [x] MasterMux
    • [x] ClockBlocker
    • [x] MaskROM
    • [x] PhysicalFilter
    • [x] TestRAM
    • [x] CLINT
    • [x] Zero
    • [x] Deadlock
    • [x] Plic
  • rocket - 6. These I believe were all previously ported by @CircuitCoder
    • [x] RocketCore
    • [x] NBDcache
    • [x] IDecode
    • [x] ScratchpadSlavePort
    • [x] TLB
    • [x] Frontend
  • tilelink - 22
    • [x] BusWrapper
    • [x] Delayer
    • [x] CacheCork
    • [x] SourceShrinker
    • [x] Edges
    • [x] RationalCrossing
    • [x] ErrorEvaluator
    • [x] Fuzzer
    • [x] RegisterRouter
    • [x] Buffer
    • [x] Filter
    • [x] Metadata
    • [x] Isolation
    • [x] Parameters
    • [x] Map
    • [x] FIFOFixer
    • [x] Jbar
    • [x] Fragmenter
    • [x] RAMModel
    • [x] PatternPusher
    • [x] RegisterRouterTest
    • [x] Atomics
  • amba/axi4 - 1
    • [x] UserYanker

Chisel3 port tasks

  • Due to #3175 missing some commits, those commits need to be reintroduced.
  • Some WIP PRs such as for devices/tilelink and util were closed without merge (https://github.com/chipsalliance/rocket-chip/pull/3131, https://github.com/chipsalliance/rocket-chip/pull/3140). Files changed on these branches could be used as the basis for fresh commits git checkout <WIP branch> -- path/to/file
  • Hardfloat files also need to be ported and submodule updated (https://github.com/ucb-bar/berkeley-hardfloat/pull/48)
  • remove chisel3_port spandrel from CI (https://github.com/chipsalliance/rocket-chip/blob/master/.github/workflows/continuous-integration.yml#L16)

michael-etzkorn avatar Jan 05 '23 00:01 michael-etzkorn

Thanks for listing this out. My assessment is that the subset that was merged is fine, since @tianrui-wei ran LEC on that branch.

Unfortunately, I think the only way to recover the missing commits is through a manual audit. Your list above should be very helpful. I don't believe there's a way to reopen a merged PR.

My proposal is to create a new chisel3_port_part2 branch that will contain the remaining changes, and after everything is in we can run LEC on the designs again.

I have also found bugs due to the scala bump that were uncaught in the regressions, these are being collected in https://github.com/chipsalliance/rocket-chip/pull/3196. This should be merged independent of the remaining chisel upgrades.

jerryz123 avatar Jan 05 '23 00:01 jerryz123

Here's the original branch I ran lec on https://github.com/tianrui-wei/rocket-chip/tree/chisel3_port_vanilla I pushed it from my local repo as the upstream got overwritten

tianrui-wei avatar Jan 05 '23 00:01 tianrui-wei

Thanks. That makes this a lot easier. Can you rebase that branch against current master and open a PR?

jerryz123 avatar Jan 05 '23 00:01 jerryz123

A draft PR can be found at https://github.com/chipsalliance/rocket-chip/pull/3197

tianrui-wei avatar Jan 05 '23 00:01 tianrui-wei

It looks like this has been delayed and significant changes have happened. @jerryz123 @sequencer since chipyard is bumping, would we want to release 1.7 on chisel 3.6 with that commit (closing #3231)? I can update the changelog, but someone with developer status will need to move those into the GH release.

michael-etzkorn avatar May 23 '23 17:05 michael-etzkorn

We've made progress getting master to work with chisel3.6.

This PR will switch master to 3.6, at which point tagging master as 1.7 makes sense. https://github.com/chipsalliance/rocket-chip/pull/3354

jerryz123 avatar May 23 '23 17:05 jerryz123