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

The new chisel (updated in Rocket on 02/03/2017) does not work with riscv-boom. #792

Closed
pentin-as opened this issue Mar 2, 2018 · 2 comments
Milestone

Comments

@pentin-as
Copy link

pentin-as commented Mar 2, 2018

Old version (works): e276571
New version (does not work): 9787117

When compiling the riscv-boom code, an exception occurs in this line:
https://github.com/freechipsproject/chisel3/blob/97871178cb511063965f971b768f91c289c4776f/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala#L617

error text:
...
[error] Caused by: chisel3.core.AutoClonetypeException: Unable to automatically infer cloneType on class boom.IssueSlotIO$$anon$1: Unable to determine instance of outer class class boom.IssueSlotIO, multiple possible candidates List(boom.IssueSlotIO@69a5c, boom.IssueSlotIO@69da4) assignable to outer class type
[error] 	at chisel3.core.Bundle.chisel3$core$Bundle$$autoClonetypeError$1(Aggregate.scala:576)
[error] 	at chisel3.core.Bundle$$anonfun$11.apply(Aggregate.scala:617)
[error] 	at chisel3.core.Bundle$$anonfun$11.apply(Aggregate.scala:593)
[error] 	at scala.Option.map(Option.scala:146)
[error] 	at chisel3.core.Bundle.cloneType(Aggregate.scala:593)
[error] 	at chisel3.core.Bundle.cloneType(Aggregate.scala:511)
[error] 	at chisel3.core.Output$.apply(Data.scala:180)
[error] 	at Chisel.package$AddDirectionToData$.asOutput$extension(compatibility.scala:28)
[error] 	at boom.IssueSlotIO.<init>(issue_slot.scala:42)
[error] 	at boom.IssueSlotIO.cloneType(issue_slot.scala:45)
[error] 	at boom.IssueSlotIO.cloneType(issue_slot.scala:19)
...

the riscv-boom code in which the error occurs:

...
19: class IssueSlotIO(num_wakeup_ports: Int)(implicit p: Parameters) extends BoomBundle()(p)
20: {
...
36:   val debug = {
37:     val result = new Bundle {
38:       val p1 = Bool()
39:       val p2 = Bool()
40:       val p3 = Bool()
41:    }
42:    result.asOutput
43:  }
44:
45:   override def cloneType = new IssueSlotIO(num_wakeup_ports)(p).asInstanceOf[this.type]
...

What do you think about it?

pentin-as referenced this issue in codelec/riscv-boom Mar 2, 2018
@jackkoenig
Copy link
Contributor

In case anyone else is looking at this, I was able to reproduce it with the code below. Note that the Wire construction is necessary (in that it doesn't trip on the first clone in IO), which leads me to believe this can be cloned once but not again. It also does not work with any iteration of autoclonetype. As in the error message above, it appears to have two candidates of MyCoolBundle for the outer object which is strange.

import chisel3._

class MyCoolBundle(w: Int) extends Bundle {
  val foo = {
    val bar = new Bundle {
      val a = UInt(w.W)
    }
    Output(bar)
  }
  override def cloneType = new MyCoolBundle(w).asInstanceOf[this.type]
}

class AutoCloneTypeTop extends Module {
  val io = IO(new MyCoolBundle(8))
  val wire = WireInit(io)
}

@ducky64
Copy link
Contributor

ducky64 commented Mar 2, 2018

Thanks for the minimal test case. It's potentially the outer class detection by stack frame examination, it's seeing both the cloneType call on the earlier Bundle, and the call to init inside cloneType for the new Bundle. Potentially solvable by having the stack frame examination filter only init calls, or just being more stricter about matching name/line/frame position/etc. Will give that a try and see if it solves the minimal test case.

@azidar azidar added this to the 3.1.0 milestone Mar 2, 2018
ucbjrl pushed a commit that referenced this issue Jun 15, 2018
* Change VerilogMemDelays to put new Statements at end of Module

Fixes #547

This is instead of putting them right after the modified DefMemory which could
result in use before declaration errors for things that feed into the new
logic.

* Adds tests that show VerilogMemDelays crashing. (#792)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants