Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bad error message when using experimental.IO #1109

Closed
jackkoenig opened this issue Jun 13, 2019 · 5 comments
Closed

Bad error message when using experimental.IO #1109

jackkoenig opened this issue Jun 13, 2019 · 5 comments
Labels
Milestone

Comments

@jackkoenig
Copy link
Contributor

jackkoenig commented Jun 13, 2019

When using experimental.IO as used in many diplomatic libraries, if you forget to .suggestName the port, you'll get a really unhelpful exception instead of the desired error at https://github.com/freechipsproject/chisel3/blob/410f03b9122978e43db938d7774b451f2b9111d0/chiselFrontend/src/main/scala/chisel3/RawModule.scala#L52.

It appears to be due to the improved .toString (#985) which during the above error message hits another, less helpful error message.

Type of issue: bug report

Impact: no functional change

Development Phase: proposal

Other information

If the current behavior is a bug, please provide the steps to reproduce the problem:

import chisel3._
import chisel3.experimental.{IO, RawModule}

class MyModule extends RawModule {
  def func(): Unit = {
    val port = IO(Output(UInt(8.W)))
    port := 3.U
  }
  func()
}

object MyMain extends App {
  println(chisel3.Driver.emit(() => new MyModule))
}

What is the current behavior?

This gives:

[error] Exception in thread "main" java.lang.ClassCastException: chisel3.internal.firrtl.Ref cannot be cast to chisel3.internal.firrtl.ModuleIO
[error]         at chisel3.experimental.BaseModule.$anonfun$getChiselPorts$2(Module.scala:246)
[error]         at scala.collection.TraversableLike.$anonfun$map$1(TraversableLike.scala:234)
[error]         at scala.collection.mutable.ResizableArray.foreach(ResizableArray.scala:59)
[error]         at scala.collection.mutable.ResizableArray.foreach$(ResizableArray.scala:52)
[error]         at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:48)
[error]         at scala.collection.TraversableLike.map(TraversableLike.scala:234)
[error]         at scala.collection.TraversableLike.map$(TraversableLike.scala:227)
[error]         at scala.collection.AbstractTraversable.map(Traversable.scala:104)
[error]         at chisel3.experimental.BaseModule.getChiselPorts(Module.scala:245)
[error]         at chisel3.experimental.DataMirror$.modulePorts(Data.scala:134)
[error]         at chisel3.experimental.DataMirror$.fullModulePorts(Data.scala:143)
[error]         at chisel3.Data.bindingToString(Data.scala:367)
[error]         at chisel3.UInt.toString(Bits.scala:647)
[error]         at java.lang.String.valueOf(String.java:2994)
[error]         at java.lang.StringBuilder.append(StringBuilder.java:131)
[error]         at chisel3.experimental.RawModule.$anonfun$namePorts$4(RawModule.scala:52)
[error]         at chisel3.internal.LogEntry.toString(Error.scala:151)
[error]         at java.lang.String.valueOf(String.java:2994)
[error]         at java.io.PrintStream.println(PrintStream.java:821)
[error]         at scala.Console$.println(Console.scala:267)
[error]         at scala.Predef$.println(Predef.scala:393)
[error]         at chisel3.internal.ErrorLog.$anonfun$checkpoint$2(Error.scala:85)
[error]         at chisel3.internal.ErrorLog.$anonfun$checkpoint$2$adapted(Error.scala:85)
[error]         at scala.collection.mutable.ResizableArray.foreach(ResizableArray.scala:59)
[error]         at scala.collection.mutable.ResizableArray.foreach$(ResizableArray.scala:52)
[error]         at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:48)
[error]         at chisel3.internal.ErrorLog.checkpoint(Error.scala:85)
[error]         at chisel3.internal.Builder$.$anonfun$build$2(Builder.scala:354)
[error]         at scala.util.DynamicVariable.withValue(DynamicVariable.scala:58)

What is the expected behavior?

Instead, it should error saying that it couldn't name the IO

@jackkoenig jackkoenig added the bug label Jun 13, 2019
@jackkoenig jackkoenig added this to the 3.2.1 milestone Jun 13, 2019
@jackkoenig
Copy link
Contributor Author

jackkoenig commented Nov 18, 2019

At the very least, .toString should not throw exceptions.

Possible [more robust] solutions:

  • Add name argument to experimental IO
  • Use rocket-chip's ValName
    • Possible issue: macros inspecting surrounding scope may be deprecated? Investigate.

@edwardcwang
Copy link
Contributor

Is experimental IO documented anywhere?

@jackkoenig
Copy link
Contributor Author

I don't think so, obviously should be

@sequencer
Copy link
Member

I found currently used IO is

protected def IO[T <: Data](iodef: T): T = chisel3.experimental.IO.apply(iodef) // scalastyle:ignore method.name

seems to be an alias to chisel3.experimental.IO

I tested chisel3.IO and chisel3.experimental.IO, both of which has a same behavior. I think it is the reason that reflection macro didn't find the IO name in a function.(However I didn't find where did the reflection logic)

@jackkoenig
Copy link
Contributor Author

@sequencer yeah that's right. The basic idea is that some name must be recording for all values declared as IO. There is logic in the Module class to reflectively .suggestName all class fields of type chisel3.Data, but we can't use reflection to do that to members of functions. You could use the @chiselName annotation to macro name the things.

jackkoenig added a commit that referenced this issue Feb 19, 2020
@mergify mergify bot closed this as completed in aac6e17 Feb 19, 2020
mergify bot pushed a commit that referenced this issue Feb 19, 2020
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit aac6e17)
mergify bot added a commit that referenced this issue Feb 19, 2020
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit aac6e17)

Co-authored-by: Jack Koenig <jack.koenig3@gmail.com>
jackkoenig added a commit to chipsalliance/rocket-chip that referenced this issue Feb 19, 2020
Summary of changes in chisel3:
* Patch fix to chipsalliance/chisel#1109

Summary of changes in firrtl:
* Constant propagate binary operations with matching arguments
* Revert last connect semantics support in reset inference
* Fix reset inference for Resets that are only invalidated
jackkoenig added a commit to chipsalliance/rocket-chip that referenced this issue Feb 25, 2020
Summary of changes in chisel3:
* Patch fix to chipsalliance/chisel#1109

Summary of changes in firrtl:
* Constant propagate binary operations with matching arguments
* Revert last connect semantics support in reset inference
* Fix reset inference for Resets that are only invalidated
jackkoenig pushed a commit that referenced this issue Feb 28, 2023
* add multi target annotation for advices

* use Seq[Seq[Target]] store targets

* add flat function

* doc simplify
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants