chipsalliance / chipsalliance/chisel

Enabling Chisel plugin side-effects & questions

Open
#1,890 4 comments 0 reactions 0 assignees View on GitHub
Dominant language
Scala
Stars
4.8k
Forks
658
Avg merge
18h 59m
Merged PRs (30d)
14

Description

**Type of issue**: bug report & other enhancement
**Impact**: unknown
**Development Phase**: request

**Other information**

After upgrading to chisel 3.4.3, we now encounter this message (line breaks added manually) about chisel plugin, instead of only its last part we were used to (often easy to fix):
```
[error] (run-main-0) chisel3.AutoClonetypeException: Unable to automatically infer cloneType on class MyClass.
cloneType is now implemented by the Chisel compiler plugin so please ensure you are using it in your build.
If you cannot use the compiler plugin or you are using it and you still see this message, please file an issue and let us know.
For those not using the plugin, here is the 'runtime reflection' cloneType error message:
constructor parameters (data) have values that are hardware types, which is likely to cause subtle errors.
Use chisel types instead: use the value before it is turned to a hardware type (with Wire(...), Reg(...), etc) or use chiselTypeOf(...) to extract the chisel type.
```
This is perfectly fine & understandable so I decided to upgrade our repos (maybe missing a small pointer to the build.sbt example I finally found in the chisel-template repo --I was missing the scala option)
For anyone reading this for this reason before 3.5:
https://github.com/freechipsproject/chisel-template/blob/main/build.sbt#L21-L23

*Edit: sorry @jackkoenig 😬 this was detailed in the release note https://github.com/chipsalliance/chisel3/releases/tag/v3.4.3*

### Enabling chisel plugin side-effect

But when compiling, our code crashed elsewhere with a not so cryptic error, but **without file:line annotation**, so I had basically no clue about the actual issue (even running `last Compile / compileIncremental` for further informations)

```
[error] (Compile / compileIncremental) scala.reflect.internal.Types$TypeError:
inferred type arguments [Nothing,S] do not conform to class MyBundle's type parameter bounds [D <: chisel3.Data,S <: ClassNotExtendingDataButBehavingAsMetaHardware[D]]
```

As the culprit class types looked weird anyway, I fixed it that way:
before:
```scala
class MyBundle[D <: Data, S <: ClassNotExtendingDataButBehavingAsMetaHardware[D]](val fakeBundle: S) extends Bundle {
val sigA = fakeBundle.sigA
val sigB = fakeBundle.sigB

override def cloneType = new MyBundle[D, S](fakeBundle).asInstanceOf[this.type]
}
```
after:
```scala
class MyBundle[D <: Data](val fakeBundle: ClassNotExtendingDataButBehavingAsMetaHardware[D]) extends Bundle {
val sigA = fakeBundle.sigA
val sigB = fakeBundle.sigB

override def cloneType = new MyBundle[D](fakeBundle).asInstanceOf[this.type]
}
// and fixing all MyBundle[D,S] into MyBundle[D] elsewhere in the code
```
None of these 2 implementations caused any issues at elaboration time in our test-cases (note: this section of our codebase is quite experimental, not used in production and hence poorly tested)

note: `ClassNotExtendingDataButBehavingAsMetaHardware` defines a `cloneType` method but renaming it didn't change anything

### Back to our original AutoClonetypeException
The original culprit was:
```scala
class ExtendedData[D <: Data](val data: D) extends Bundle {
val field = Bool()
}
```
originally fixed that way:
```scala
class ExtendedData[D <: Data](private val d: D) extends Bundle {
val field = Bool()
val data = d.cloneType
}
```

But with chisel-plugin enabled, there is no more exception raised here... is this a feature or a bug?
I think it is a feature as it means less code and no more runtime AutoClonetypeException which is a quite frightening error for beginners :)
However the second form forces the `cloneType` on the argument which I like because it isolates & never trusts some user-provided `Data` as fresh, clean & unbound which I personally prefer to the mantra "never pass bound `Data`".
Indeed the later is not applicable as soon as you deal with advanced usage where bound data is sometimes encapsulated and hence passed as arguments of larger classes.
(Note that we do NOT use `chiselTypeOf` in place of `cloneType` as recommended because it voluntary requires bound data https://github.com/chipsalliance/chisel3/blob/v3.4.2/core/src/main/scala/chisel3/Data.scala#L240, hence killing the idea of always providing safety...)

### Summary
- No file/line pointer when debugging a chisel-plugin related type error, is there a way to improve this?
- Enabling chisel plugin raised a weird side-effect, easily fixed in our case
- Is it now always safe to pass bound-hardware as parameter of a class extending `Bundle`?

This is not a big deal anyway, thanks in advance for taking the time to explain me in further details what changed :)

Contributor guide

Open the contributing guide

Research direction

Start with the chisel-template build.sbt example and the Chisel compiler-plugin behavior described in the report. Read the referenced Data.scala cloneType implementation and compare the ExtendedData and MyBundle examples. Done would require an answer or documented fix for the missing file/line information and the safety of passing bound Data into Bundle parameters.

Written by the indexing model from the issue text.

Assessment

Tech stack
scala
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.