Port legacy chisel code to chisel3 for amba and tilelink #3059

tianrui-wei · 2022-09-15T18:43:19Z

Signed-off-by: Tianrui Wei tianrui@tianruiwei.com
Related issue:

Type of change: bug report | feature request | other enhancement

Impact: no functional change | API addition (no impact on existing code) | API modification

Development Phase: proposal | implementation

Release Notes

Signed-off-by: Tianrui Wei <tianrui@tianruiwei.com>

1. remove chisel compatibility 2. remove tieoff() functions Signed-off-by: Tianrui Wei <tianrui@tianruiwei.com>

This addresses issues with the original PR

tianrui-wei · 2022-09-15T23:14:16Z

Supercedes #3057. Basically the original PR with the tilelink changes stripped. Tilelink will be re-done separately in another PR

sequencer · 2022-09-17T03:05:28Z

opps, plz make the following PRs to be small ;p

tianrui-wei · 2022-09-17T05:26:18Z

Hi @sequencer I agree having small PRs would be a better practice in general, and I try to keep PRs as small as possible (one per directory of change). However, there're several reasons why I think bigger PRs like this one are necessary in the case of porting rocket to chisel3.

Most of the changes are trivial and require no manual intervention. I use a regular expression to convert BOOL(Input()) to Input(Bool()), and things like this. One might argue there are not significant benefit to split these changes over a dozen PRs. It pollutes the commit history and makes it more difficult to bisect problems in the face of an issue
Some defines are used in tilelink modules and amba modules as well. These need to be changed as well in order to pass CI.
I have performed some sequential equivalence checking on some typical configurations like SmallConfig or DefaultConfig or WithJtagDTMSystem,DefaultConfig with jaspergold, and no functional changes are reflected. The top level module for equivalance checking is ExampleRocketSystem. In other words, I'm more confident in that the changes are not disruptive based on formal equivalence checking than depending on ci runs or manual reasoning (although not passing in ci would be worrying)

This leads me to believe it might be better to do several PRs over different directories rather than doing many small PRs, but of course we could also discuss it in the next rocket chip meeting.

cc @jerryz123

tianrui-wei · 2022-09-22T17:27:04Z

@sequencer Per our disucssion yesterday, could you create a staging branch for all the chisel3 changes?

sequencer · 2022-09-24T07:32:41Z

sorry for the delay, I was out of office these todays. Will do it 8pm cst today.

tianrui-wei · 2022-09-26T02:25:02Z

Thanks Jiuyang, let me know if you want any changes in the branch

sequencer · 2022-09-17T03:16:09Z

src/main/scala/amba/ahb/Bundles.scala

-  def tieoff(): Unit = {
-    hrdata.dir match {
-      case INPUT =>
-        hreadyout := Bool(false)
-        hduser    :<= BundleMap()
-        hresp     := AHBParameters.RESP_OKAY
-        hrdata    := UInt(0)
-      case OUTPUT => 
-        hmastlock := Bool(false)
-        hsel      := Bool(false)
-        hready    := Bool(false)
-        htrans    := AHBParameters.TRANS_IDLE
-        hsize     := UInt(0)
-        hburst    := AHBParameters.BURST_SINGLE
-        hwrite    := Bool(false)
-        hprot     := AHBParameters.PROT_DEFAULT
-        haddr     := UInt(0)
-        hauser    :<= BundleMap()
-        hwdata    := UInt(0)
-      case _ =>
-    }
-  }


why tieoff() is removed?

This is from a discussion between Jerry and me, he thinks tieoff is not good practice, is rarely used and doesn't map cleanly to chisel3 concepts. You can see later in the code that we manually tied off 2 places where it was different

src/main/scala/amba/ahb/Bundles.scala

sequencer

Some nitpicks

sequencer · 2022-10-02T17:13:18Z

src/main/scala/amba/apb/Bundles.scala

-  def tieoff(): Unit = {
-    pready.dir match {
-      case INPUT =>
-        pready  := Bool(false)
-        pslverr := Bool(false)
-        prdata  := UInt(0)
-        pduser :<= BundleMap()
-      case OUTPUT =>
-        pwrite  := Bool(false)
-        paddr   := UInt(0)
-        pprot   := APBParameters.PROT_DEFAULT
-        pwdata  := UInt(0)
-        pstrb   := UInt(0)
-        pauser :<= BundleMap()
-      case _ =>


why removing this.

Same as above

src/main/scala/amba/axi4/Fragmenter.scala

src/main/scala/amba/axi4/IdIndexer.scala

src/main/scala/amba/axi4/Parameters.scala

src/main/scala/tilelink/ToAHB.scala

src/main/scala/tilelink/ToAPB.scala

src/main/scala/tilelink/ToAXI4.scala

1. group different imports for chisel3.util to the same line 2. use WireDefault instead of WireInit 3. use chiselTypeOf instead of cloneType Signed-off-by: Tianrui Wei <tianrui@tianruiwei.com>

src/main/scala/amba/axi4/Xbar.scala

…ce#3059) 1. remove chisel compatibility 2. group different imports for chisel3.util to the same line 3. use WireDefault instead of WireInit 4. fuse chiselTypeOf instead of cloneType 5. remove tieoff() functions Signed-off-by: Tianrui Wei <tianrui@tianruiwei.com> Co-authored-by: Jiajie Chen <c@jia.je> Co-authored-by: Jiuyang Liu <liu@jiuyang.me>

1. remove chisel compatibility 2. group different imports for chisel3.util to the same line 3. use WireDefault instead of WireInit 4. fuse chiselTypeOf instead of cloneType 5. remove tieoff() functions Signed-off-by: Tianrui Wei <tianrui@tianruiwei.com> Co-authored-by: Jiajie Chen <c@jia.je> Co-authored-by: Jiuyang Liu <liu@jiuyang.me>

+ for chipsalliance/rocket-chip#3059

In legacy code it is ok to clone a Wire using Wire() now the requirement is more strict and WireDefault is needed Related to #3059 The Output is added for code style consistency Closes #3324

In legacy code it is ok to clone a Wire using Wire() now the requirement is more strict and WireDefault is needed Related to #3059 The Output is added for code style consistency Closes #3324 (cherry picked from commit fb5d7d0)

In legacy code it is ok to clone a Wire using Wire() now the requirement is more strict and WireDefault is needed Related to #3059 The Output is added for code style consistency Closes #3324 (cherry picked from commit fb5d7d0) Co-authored-by: Zenithal <i@zenithal.me>

* bump to Chisel 3.5.6 (#3222) * Remove deprecated code for BarrelShifter Cherry-picked chipsalliance/chisel@7372c9e Should use BarrelShifter from chisel3.std, but it is not published, see chipsalliance/chisel#2997 * Fix scala reflect error scala.reflect.internal.Types: constructor RecordMap in class RecordMap cannot be accessed in package util from package util in package rocketchip * explicit add legacy connect operators * replace all cde dependencies. * change api-config to cde in build.sbt * bump cde submodule * fix Makefile * IDecode: Fix aes64ks1i imm decode It is not rs2, it is imm Related to #3255 * CryptoNIST: refactor rnum This removes a redundant port As rnum is encoded in rs2 as imm now, we can get it just from rs2 * Remove redundant TLBExceptions V bit This was introduced by "(185cac8) Add hypervisor extension (#2841)" This is a dead code, as no circuit is producing and consuming this bit. This was discovered when migrating Core.scala to chisel3, where strict checking was applied for IO and firrtl found this is not connected. In the context of hypervisor extension, V bit, or _virtualization mode_, indicates whether the hart is currently executing in a guest. For TLBReq, the V bit is needed as it affects the PTW thus TLB behavior on whether to do _Two-Stage Address Translation_. However, the bit is not needed for TLBException. The exceptions (pf, gf, ae, ma) have no V=1/V=0 variants. Also, there is no point to add V bit for gf (guest page fault). The io.resp.gf added below in the original patch also does not connect io.resp.gf.v. If this V bit should be added for this specific exception, it should be connected. I assume this was added accidentally, as the original "extends CoreBundle()(p)" modification seems irrelevant. * update build system for cde bump * Fix unconnected io.start for TLRAMXbarTest and TLMulticlientXbarTest (cherry picked from commit 34d7309) * Fold HasPeripheryDebugModuleImp into HasPeripheryDebug (cherry picked from commit a2682ca) * Move HasDebugModule out of TileContextType (cherry picked from commit 0e4af6d) * Pinning nix to 2.13.3 in github workflows Nix 2.14 released with incompatibilities with cachix/install-nix-action. This PR pins nix to 2.13.3 to avoid CI fails See: cachix/install-nix-action#161 (cherry picked from commit 84533ae) * L1TLB: VS-mode SFENCE misses tera/giga-page entries fragmented superpages (#3297) (cherry picked from commit 8b52a6f) * feat: port Chisel2 to Chisel3 devices/ (cherry picked from commit 1ff0db3) * fix: add `chiselTypeOf` when inst Wire(in.d) (cherry picked from commit e645d94) * feat: port Chisel2 to Chisel3 rocket/ (cherry picked from commit d6a982b) * feat: port Chisel2 to Chisel3 amba/ (cherry picked from commit b2fd991) * Fix CharCount RoCC example bug Reset recv_beat (cherry picked from commit f19a90a) * Change CharCountRoCC Example to use dcacheParams RoCC accesses D$, not I$ (cherry picked from commit 61ea81c) * mill: fix empty cross arg for riscv-tests.Suite riscv-tests.suite[] wont compile (cherry picked from commit d86c011) * Hypervisor: encodeVirtualAddress extra MSB to canonicalize VS-disabled (#3314) (cherry picked from commit 0504a9b) * Support devOverride for diplomatic SRAMs (cherry picked from commit 12e21a6) * Support overriding the DTS node for diplomatic SRAMs (cherry picked from commit c8edec3) * Fix no-debug-node designs (cherry picked from commit 3b8d3c1) * all isaDTS strings to lowercase (#3333) (#3334) (cherry picked from commit 58c8249) Co-authored-by: Yangyu Chen <cyy@cyyself.name> * Deprecate old BusWrapper methods (#3337) (#3340) (cherry picked from commit 2570db7) Co-authored-by: Jerry Zhao <jerryz123@berkeley.edu> * Fix AXI4 RegisterRouter on Wire Clone (#3341) In legacy code it is ok to clone a Wire using Wire() now the requirement is more strict and WireDefault is needed Related to #3059 The Output is added for code style consistency Closes #3324 (cherry picked from commit fb5d7d0) Co-authored-by: Zenithal <i@zenithal.me> * Support TLFilter.mSubtract with multiple AddressSets (#3339) (#3343) Enables filtering out multiple ranges at once. (cherry picked from commit 4110563) Co-authored-by: Jerry Zhao <jerryz123@berkeley.edu> * Improve boundaryBuffers API | move boundaryBuffers to within-Tile (#3342) (#3344) * Add API to force TLBuffers into RocketTile as boundaryBuffers * Generate boundaryBuffers within the Tile, not the TilePRCIDomain (cherry picked from commit 3f74d79) Co-authored-by: Jerry Zhao <jerryz123@berkeley.edu> * Implement Zicond extension (#3329) * Instructions: sync from riscv-opcodes (add zicond) * Zicond: implement czero.eqz, czero.nez * Zicond: fix implementation * Zicond: reduce code duplication * Add ISA extension when using Zicond * Fix ISA extensions ordering (cherry picked from commit 62162c5) * feat: port Chisel2 to Chisel3 tilelink/ (cherry picked from commit 6c23100) * fix: not fully initialized wires in Edge.scala + using `DontCare` (cherry picked from commit 3902497) * fix: add chiselTypeOf when inst Wire + in.d and out.a; (cherry picked from commit e52773e) * fix: not fully initialized wires in Edge.scala + using `DontCare` (cherry picked from commit 5f7278b) * Revert to old := connects instead of :#= * Support removing the nonstandard CEASE from rocket (cherry picked from commit 956a1ff) * fix: Chisel3 #2944 Move SourceInfo to package experimental * Update debug_rom.S (cherry picked from commit a9ae0b9) * Update debug_rom_nonzero.S (cherry picked from commit 6ba9437) * Remove cloneType Ref to #2889 * Bump hardfloat * Bump to chisel3.6 * Switch to json4s-native * Update HeterogeneousBag to chisel3.6 * Bump mill flow to chisel 3.6.0 * Switch HeterogeneousBag to VectorMap * Update src/main/scala/util/HeterogeneousBag.scala Co-authored-by: Jack Koenig <koenig@sifive.com> * support circt and bump to 3.6 * Generalize Vec[TracedInstruction] to a TraceBundle (cherry picked from commit 85aca71) * Add time to TraceBundle (cherry picked from commit 57af718) * Supporting adding custom stuff to TraceBundle (cherry picked from commit f6f59c1) * Fix BlockableTraceBundle (cherry picked from commit efa8337) * Bump nix toolchain version * Fix TLSourceShrinker (cherry picked from commit d503368) * Support RoCC accels which define CSRs (#3358) (cherry picked from commit 7ddf02a) * Add support for an unsynthesizable ROB to produce a TracedInstruction stream from Rocket with wdata (cherry picked from commit d6c09c9) # Conflicts: # src/main/scala/rocket/RocketCore.scala # src/main/scala/subsystem/Configs.scala * Fix memory leak in debug_rob (cherry picked from commit b3f391c) * Fix unittests (cherry picked from commit a806851) * Fix TL unittests (cherry picked from commit ecf08f5) * Support blockable credited interfaces (cherry picked from commit c8cf935) * Make AsyncQueue use Rawmodule (cherry picked from commit 8db7364) * Support dynamic credit count in senders for CreditedIO (cherry picked from commit 5a5c127) * Fix TLJbarTest (cherry picked from commit e76a4ea) * Add [m/s/h]envcfg CSRs to satisfy spec requirements (#3373) (cherry picked from commit 9967142) * fix: Werror match may not be exhaustive (#3268) (#3384) + add `case _` to make match be exhaustive (cherry picked from commit 05d9db7) Co-authored-by: SingularityKChen <chency_singularity@163.com> * fix: empty argument list (#3262) (#3386) + add `()` to those empty argument list function call; Auto-application to `()` is deprecated. Supply the empty argument list `()` explicitly to invoke method (cherry picked from commit 9b383c5) Co-authored-by: SingularityKChen <chency_singularity@163.com> * fix: bit extraction use .U.extract(i) (#3263) (#3385) + replace `.U(i)` with `.U.extract(i)` (cherry picked from commit 9a1dc2d) Co-authored-by: SingularityKChen <chency_singularity@163.com> * TLB: must_alloc swapped AMO Logical/Arithmetic (#3390) (cherry picked from commit dc275c4) Co-authored-by: John Ingalls <43973001+ingallsj@users.noreply.github.com> * TLB: vsstatus.SUM check should not apply to Stage-2 fault before Stage-1 response (#3393) (#3399) (cherry picked from commit 43e0af1) Co-authored-by: John Ingalls <43973001+ingallsj@users.noreply.github.com> * CSR: optionally set delegable hypervisor exceptions (#3401) Given that usingHypervisor is used to distinguish whether the hardware supports hypervisor extensions, we should use it for the delegable exceptions as well. (cherry picked from commit 026f4c9) * Enable WARL custom CSRs, long-latency CSR accesses (#3388) * Support setting custom CSRs from datapath * Support CSR stalls (cherry picked from commit 005c6db) * PTW: set ae_ptw for out-of-range non-leaf PTEs (#3407) (#3410) For PTEs whose physical address is out-of-range, we need to set `ae_ptw` instead of `ae_final` to raise access-fault. Because non-leaf PTEs will not have R or X bits set, `ae_final` will be overrided by page-fault exceptions. (cherry picked from commit b8dad7f) Co-authored-by: Yinan Xu <xuyinan@ict.ac.cn> --------- Co-authored-by: Jiuyang Liu <liu@jiuyang.me> Co-authored-by: Zenithal <i@zenithal.me> Co-authored-by: Hansung Kim <hansung_kim@berkeley.edu> Co-authored-by: Liu Xiaoyi <circuitcoder0@gmail.com> Co-authored-by: John Ingalls <43973001+ingallsj@users.noreply.github.com> Co-authored-by: singularity <chency_singularity@163.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Yangyu Chen <cyy@cyyself.name> Co-authored-by: rewired <39949564+rewired-gh@users.noreply.github.com> Co-authored-by: Jack Koenig <koenig@sifive.com> Co-authored-by: Yinan Xu <xuyinan@ict.ac.cn>

jiegec and others added 6 commits September 13, 2022 16:54

Port legacy chisel code to chisel3 for amba

67f86d6

Signed-off-by: Tianrui Wei <tianrui@tianruiwei.com>

Port legacy chisel code to chisel3 for tilelink

1a23011

Signed-off-by: Tianrui Wei <tianrui@tianruiwei.com>

feat: port amba subfolder to chisel3

8e5ca99

1. remove chisel compatibility 2. remove tieoff() functions Signed-off-by: Tianrui Wei <tianrui@tianruiwei.com>

feat: axi lintclean

fa1a6d7

fix: fix issues with apb not tied and lint error

a9cddd3

fix: fix issues addressed in Jerry's review

2158214

This addresses issues with the original PR

tianrui-wei mentioned this pull request Sep 15, 2022

Port legacy chisel code to chisel3 for amba #3057

Closed

tianrui-wei mentioned this pull request Sep 15, 2022

feat: convert devices to use chisel3 #3058

Closed

sequencer changed the base branch from master to chisel3_port September 24, 2022 13:39

sequencer requested changes Oct 2, 2022

View reviewed changes

fix: address @sequencer's review comments

dd3703c

1. group different imports for chisel3.util to the same line 2. use WireDefault instead of WireInit 3. use chiselTypeOf instead of cloneType Signed-off-by: Tianrui Wei <tianrui@tianruiwei.com>

tianrui-wei requested a review from sequencer October 4, 2022 02:51

fix: typo

da4b5a2

sequencer deleted the branch chipsalliance:chisel3_port October 4, 2022 05:03

sequencer closed this Oct 4, 2022

sequencer reopened this Oct 4, 2022

sequencer force-pushed the chisel3_port branch from c008d85 to 78468c8 Compare October 7, 2022 04:00

sequencer mentioned this pull request Oct 7, 2022

Port ahb to Chisel 3 #3116

Closed

sequencer reviewed Oct 7, 2022

View reviewed changes

src/main/scala/amba/axi4/Xbar.scala Outdated Show resolved Hide resolved

Update src/main/scala/amba/axi4/Xbar.scala

e166feb

sequencer approved these changes Oct 7, 2022

View reviewed changes

sequencer merged commit 223c4c2 into chipsalliance:chisel3_port Oct 7, 2022

sequencer changed the title ~~Port legacy chisel code to chisel3 for amba~~ Port legacy chisel code to chisel3 for amba and tilelink Oct 7, 2022

SingularityKChen mentioned this pull request Jan 7, 2023

Remove tieoff() firesim/firesim#1375

Closed

12 tasks

SingularityKChen added a commit to SingularityKChen/chipyard that referenced this pull request Jan 7, 2023

fix: remove tieoff()

dff760d

+ for chipsalliance/rocket-chip#3059

SingularityKChen mentioned this pull request Jan 7, 2023

Remove tieoff() ucb-bar/chipyard#1308

Closed

16 tasks

ZenithalHourlyRate mentioned this pull request May 2, 2023

Fix AXI4 RegisterRouter on Wire Clone #3338

Merged

Kevin99214 mentioned this pull request May 22, 2024

Update APBtoTL scala to not flip apb address when doing conversion #3634

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port legacy chisel code to chisel3 for amba and tilelink #3059

Port legacy chisel code to chisel3 for amba and tilelink #3059

tianrui-wei commented Sep 15, 2022

tianrui-wei commented Sep 15, 2022

sequencer commented Sep 17, 2022

tianrui-wei commented Sep 17, 2022

tianrui-wei commented Sep 22, 2022

sequencer commented Sep 24, 2022

tianrui-wei commented Sep 26, 2022

sequencer Sep 17, 2022

tianrui-wei Oct 4, 2022

sequencer left a comment

sequencer Oct 2, 2022

tianrui-wei Oct 4, 2022

Port legacy chisel code to chisel3 for amba and tilelink #3059

Port legacy chisel code to chisel3 for amba and tilelink #3059

Conversation

tianrui-wei commented Sep 15, 2022

tianrui-wei commented Sep 15, 2022

sequencer commented Sep 17, 2022

tianrui-wei commented Sep 17, 2022

tianrui-wei commented Sep 22, 2022

sequencer commented Sep 24, 2022

tianrui-wei commented Sep 26, 2022

sequencer Sep 17, 2022

Choose a reason for hiding this comment

tianrui-wei Oct 4, 2022

Choose a reason for hiding this comment

sequencer left a comment

Choose a reason for hiding this comment

sequencer Oct 2, 2022

Choose a reason for hiding this comment

tianrui-wei Oct 4, 2022

Choose a reason for hiding this comment