-
Notifications
You must be signed in to change notification settings - Fork 18
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
Create chisel3-vs-compat.md #48
base: master
Are you sure you want to change the base?
Conversation
### Utilities | ||
|
||
Many constructs originally available via `import Chisel._` are now located in | ||
`chisel3.util`. They can be imported |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there text missing? They can be imported doesn't convey much information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sorry, this is a WIP and I jumped around, sorry I should've made this a draft PR
but can be used to drive wires from bidirectional inputs or outputs to "monitor" | ||
the the full aggregate, ignoring directions. | ||
|
||
* Bulkconnect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is actually BiConnect
, yes? Or should this be referred to as Bulk Connect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Traditionally people called it "bulk connect" but I agree "biconnect" is better anyway
Apologies, to clarify this is more of a draft PR (which is what I should've opened) |
```scala | ||
val x = UInt(8.W) | ||
``` | ||
Notably, widths are now a type rather than a byname argument to the `UInt` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notably, widths are now a type rather than a byname argument to the `UInt` | |
Notably, widths are now a type rather than a by-name argument to the `UInt` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I just realized that what I really mean is "named argument", by-name is when you put =>
in front of the type 🤦♂
It makes the example more useful Co-Authored-By: edwardcwang <[email protected]>
arguments must be a port. | ||
It will determine the correct leaf-level connections based on the directions of | ||
its port arguments. | ||
It cannot be used to connect two components _inside_ of a module. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So.... how are you supposed to do that? Do you have to use two monoconnects?
a := b
b := a
4.U <> io.x // This also works but is stylistically suspect | ||
} | ||
``` | ||
In contrast to compatibility mode, every field of two connected Bundles must match. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this only true for <>
operator?
val in = Input(new BundleA) | ||
val out = Output(new BundleB) | ||
} | ||
io.out <> io.in // This is an error because io.in.bar doesn't match |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So how to do this properly assuming the user wants the same code to be generated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chisel.<>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a method defined on data so that wouldn't work, but you could define a new operator via an implicit conversion:
object CompatConnect {
import Chisel._
implicit class sketchyBulkConnectable(data: Data) {
def <*>(that: Data): Unit = data <> that
}
}
You can then import CompatConnect._
in the code that needs it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fundamentally, relying on this behavior was considered dangerous because in refactoring code if you added a field it would bulk connect anyway and lead to subtle bugs. I think there is a very strong argument that such behavior should work when used with inheritance, eg. if Foo
and Bar
are Bundles
that both extends a common interface Fizz
, <>
should be legal between them and only connect the common fields in Fizz
. Unfortunately, it's really not clear how to implement such a feature (well, probably doable with incredibly gnarly reflection), so it's been languishing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that this is the desired/intended behavior in most extant use cases
this needs to address the lack of |
WIP documentation on
import chisel3._
vs.import Chisel._
. I know there's some overlap with https://github.com/freechipsproject/www.chisel-lang.org/blob/master/docs/src/main/tut/chisel3/chisel3-vs-chisel2.md but there are other issues to address.