chipsalliance / chisel

Chisel: A Modern Hardware Design Language
https://www.chisel-lang.org/
Apache License 2.0
3.92k stars 589 forks source link

Error Message Unhelpful When Annotating Literals #2145

Open alvintung-sifive opened 2 years ago

alvintung-sifive commented 2 years ago

Type of issue: bug report

Impact: no functional change

Development Phase: request

Other information

firrtl.annotations.AnnotationException: Illegal component name: UInt<2>("h03")
    at firrtl.annotations.ComponentName.<init>(Target.scala:839)
    at chisel3.internal.NamedComponent.toNamed(Builder.scala:300)
    at chisel3.internal.NamedComponent.toNamed$(Builder.scala:299)
    at chisel3.Data.toNamed(Data.scala:339)
    at chisel3.dontTouch$$anon$1.toFirrtl(dontTouch.scala:39)
    at chisel3.dontTouch$$anon$1.toFirrtl(dontTouch.scala:39)
    at chisel3.internal.firrtl.Circuit.$anonfun$firrtlAnnotations$1(IR.scala:806)
    at scala.collection.immutable.List.flatMap(List.scala:293)
    at scala.collection.immutable.List.flatMap(List.scala:79)
    at chisel3.internal.firrtl.Circuit.firrtlAnnotations(IR.scala:806)
    at chisel3.stage.phases.Convert.$anonfun$transform$1(Convert.scala:30)
    at scala.collection.immutable.List.flatMap(List.scala:293)
    at scala.collection.immutable.List.flatMap(List.scala:79)
    at chisel3.stage.phases.Convert.transform(Convert.scala:24)
    at chisel3.stage.phases.Convert.transform(Convert.scala:17)
    at firrtl.options.DependencyManager.$anonfun$transform$5(DependencyManager.scala:280)
    at firrtl.Utils$.time(Utils.scala:181)
    at firrtl.options.DependencyManager.$anonfun$transform$3(DependencyManager.scala:280)
    at scala.collection.LinearSeqOps.foldLeft(LinearSeq.scala:169)
    at scala.collection.LinearSeqOps.foldLeft$(LinearSeq.scala:165)
    at scala.collection.immutable.List.foldLeft(List.scala:79)
    at firrtl.options.DependencyManager.transform(DependencyManager.scala:269)
    at firrtl.options.DependencyManager.transform$(DependencyManager.scala:255)
    at firrtl.options.PhaseManager.transform(DependencyManager.scala:443)
    at chisel3.stage.ChiselStage$.emitVerilog(ChiselStage.scala:240)
    at chisel3.getVerilogString$.apply(verilog.scala:7)
    at Playground$.delayedEndpoint$Playground$1(main.scala:15)
    at Playground$delayedInit$body.apply(main.scala:2)
    at scala.Function0.apply$mcV$sp(Function0.scala:39)
    at scala.Function0.apply$mcV$sp$(Function0.scala:39)
    at scala.runtime.AbstractFunction0.apply$mcV$sp(AbstractFunction0.scala:17)
    at scala.App.$anonfun$main$1(App.scala:76)
    at scala.App.$anonfun$main$1$adapted(App.scala:76)
    at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:563)
    at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:561)
    at scala.collection.AbstractIterable.foreach(Iterable.scala:919)
    at scala.App.main(App.scala:76)
    at scala.App.main$(App.scala:74)
    at Playground$.main(main.scala:2)
    at Main$.main(main.scala:21)
    at Main.main(main.scala)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at sbt.Run.invokeMain(Run.scala:133)
    at sbt.Run.execute$1(Run.scala:82)
    at sbt.Run.$anonfun$runWithLoader$5(Run.scala:110)
    at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
    at sbt.util.InterfaceUtil$$anon$1.get(InterfaceUtil.scala:17)
    at sbt.TrapExit$App.run(TrapExit.scala:258)
    at java.lang.Thread.run(Thread.java:748)

If the current behavior is a bug, please provide the steps to reproduce the problem: https://scastie.scala-lang.org/Yp73iZvSSaaQiViJAHyAZQ

What is the current behavior? Annotating a literal value does not provide a helpful enough error message to pinpoint the offending code.

What is the expected behavior? The error message should give the file and line number of the offending code.

Please tell us about your environment:

What is the use case for changing the behavior?

sequencer commented 2 years ago

I thought this should be guarded by: https://github.com/chipsalliance/chisel3/blob/f1e37900170124254f3cf4599a45e7a485c17a91/core/src/main/scala/chisel3/dontTouch.scala#L36-L38

mwachs5 commented 2 years ago

i mean, the dontTouch is just an example. In general I don't think we can annotate a literal value, right?

jackkoenig commented 2 years ago

@sequencer I thought this should be guarded by:

Literals are synthesizable, they're just not nameable.

@mwachs5 i mean, the dontTouch is just an example. In general I don't think we can annotate a literal value, right?

You are correct, this is something of a fundamental issue.

We could make chisel3.experimental.annotate (I'm glad it's still experimental!) take a SourceInfo so that we could at least provide a source locator for where annotate is called. The onus would then be on library writers to propagate that source locator. We could also encourage library writers to guard against literals (and we need to do that in dontTouch).

sequencer commented 2 years ago

Literals are synthesizable, they're just not nameable.

I see, I understand it now. I personal think a better solution might be: DataMirror providing more APIs for a binding to reflect a hardware type?(just pull more information from the Builder), and user can guard it with those information? annotate API can also check nameable in this case as well.