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

Vec literals #849

Closed
edwardcwang opened this issue Jul 8, 2018 · 13 comments
Closed

Vec literals #849

edwardcwang opened this issue Jul 8, 2018 · 13 comments
Labels
feature request Feature New feature, will be included in release notes
Milestone

Comments

@edwardcwang
Copy link
Contributor

Now that a new literal framework is in place after #820, would it make sense to create a mechanism (possibly by co-opting VecInit or by creating a VecLit constructor) to create Vec literals? The current closest equivalent, VecInit, still creates a wire and assigns the literals to it the old way we worked around lack of bundle literals.

Type of issue: feature request

Impact: no functional change | API addition (no impact on existing code)

Development Phase: request | proposal

@edwardcwang edwardcwang added feature request Feature New feature, will be included in release notes labels Jul 8, 2018
@ducky64
Copy link
Contributor

ducky64 commented Jul 8, 2018

I think unifying literal constructor and init is something that needs more thought.

VecInit is a useful shorthand for packing elements into a Vec, but can't be used in Bundle literal constructors. Bundle literal constructors also can't take non-literal types (like VecInit results), but can be unpacked into Scala-land data (used for testers2).

I think there are use case arguments for both, though what the API looks like should be up for debate. Vec(3, UInt(32.W)).Lit(..., ..., ...)? Should there also be a Bundle equivalent of init, as a shorthand for packing Bundles, like (new MyBundle(...)).Init(a=..., b=...)? Can we do something about unifying the syntax, since the equivalent VecInit would look more like Vec(3, UInt(32.W)).Init(..., ...)? But VecInit can also determine the Vec type from its arguments, which Bundle Lit (or speculatively, init) can't do.

@aswaterman
Copy link
Member

Re: API: Another option is to avoid introducing a new API and instead hide this improvement inside Chisel. This could be done when VecInit's args are all literals, and when the resulting wire is never assigned to.

This might not be practical, for implementation reasons, but it's def. worth considering.

@ducky64
Copy link
Contributor

ducky64 commented May 19, 2019

Writing up discussion from chisel-dev two weeks back:

Vector literals could have two APIs, one analogous to Seq(...) and another analogous to .toSeq()

Vec.Lit(0.U, 1.U, 2.U, 3.U, …)

val w = Wire(Vec(n, UInt(3.W)))
w := Vec.Lit(0.U, 1.U, 2.U, 3.U, …)

(0 until n).map(_.U).toVecLit

These might need to support some way to specify a base type, eg if you wanted a Vec of SInts. One proposal is, analogous to how we specify widths:

(0 until n).map(_.U).toVecLit(SInt(3.W))

But the Vec.Lit case is trickier, since there's less of an analogy. Perhaps analogous to Bundle literals, create the "prototype" object and then create a literal from it:

Vec(6, SInt(3.W)).Lit(0.U, 1.U, 2.U, 3.U, …)

though this overspecifies the Vec since the length is specified both explicitly in the Vec(...) prototype constructor, and then again implicitly in the .Lit(...) argument list.

@chick
Copy link
Contributor

chick commented May 19, 2019

I prefer the last, and the over-specification sounds like an error check to me.

@ducky64
Copy link
Contributor

ducky64 commented May 19, 2019

I'd disagree. I can't think of any language off the top of my head which requires over-specification like that (both explicitly the number of elements, and implicitly as the size of the argument list). Even C allows something like int arr[] = {0, 1, 2}; (though you could also explicitly specify the array length if you wanted to). This style also becomes a maintenance burden, as if you have a mismatch, you might not notice until you get a runtime error.

@aswaterman
Copy link
Member

aswaterman commented May 20, 2019

Vec.Lit(0.U, 1.U, 2.U, 3.U, …) seems fine for the presumably common case of not needing to force a particular base type.

Having to redundantly type the number of elements is hokey. The old Enums have that property, and it's not a feature.

@kammoh
Copy link
Contributor

kammoh commented Feb 6, 2020

Any updates on this?

@jackkoenig
Copy link
Contributor

As far as I know, no one is working on it. It would be nice though.

@azidar azidar added this to the 3.3.x milestone Mar 16, 2020
@azidar
Copy link
Contributor

azidar commented Mar 16, 2020

Notes from Meeting:

VecInit(Seq())
Resolution:
Support VecInit(Seq.empty[T]) => Vec[T] vut not for any indexing (including dynamic)

  • Must throw exception to avoid sample_element problem
  • Generates no hardware
  • Stretch goal, support Nothing so that VecInit(Seq.empty[MyBundle])(0.U(0.W)) is legal
  • Our support for zero-width/zero-index stuff is already sketchy
Vec(0, UInt(128.W))
UInt() != UInt(128.W)
VecInit(Seq.empty[T])
VecInit(Seq.empty[UInt])
Seq.fill(0){...}
def func[T <: Data](xs: Seq[T]) = VecInit(xs)

If xs is empty, this is VecInit(Seq.empty[T])

Vec(0, UInt(128.W)) =? Vec(0, UInt(1.W)) =? Vec(0, Bool())
Nil == List[Int]() == List[String]() == List[Potato]()

How to “naturally” construct Seq.empty[Int]: Seq.fill(0)(12345)

VecInit(typeTemplate=Vec(0, …), init=Seq())
VecInit(Seq.empty[UInt]) :=? Vec(0, UInt(128.W))

Relationship of Vec(0, UInt(128.W)) and VecInit(Seq())` ?

Vec(0, UInt(128.W)) evaluate to Vec[Nothing]()
Vec[Nothing] <:??? Vec[UInt]

Should Vec be (co)variant? Is it?

def func[T <: Data](xs: Vec[T], idx: UInt): T = xs(idx)

Need to materialize a concrete object of type T prevents us from materializing a concrete object without sample_element constructor!!!!
This is the Seq.empty[T] problem
What if xs is a VecInit(Seq.empty[T])
Say if T=UInt, should this UInt object that is materialized be

  • UInt(0.W)?
  • UInt(1.W)?
  • UInt(1000.W)?
    UInt() -> or return Exception instead of making up a decision about UInt

What if I call func with T=MyFancyTingamabobChiselType?
Is this an Exception?
Or return a DontCare?

  • DontCare is a subtype of Data
  • DontCare is not a subtype of UInt?
  • UInt is a subtype of Data
  • DontCare isn’t a perfect bottom type
    Currently
val x = … // hardware type: Vec(0, UInt(128.W))
val y = Wire(UInt()) // …
y := //
x(y) => return UInt(128.W) because we gave it a sample_element constructor
Works in Chisel and crashes in FIRRTL
 
x= Vec(128elts)
x(129.U) => crash?
VecInit(Seq.empty)
x(y) => crash?
List()(0) > crash
List().map(...) => ok

What we like to support

Wire(Vec(n == 0, MyFancyChiselType(blah blah blah))) := VecInit(Seq.fill(n == 0)(whatever)
Wire(Vec(n == 0, MyFancyChiselType(blah blah blah))) := VecInit(Seq.empty[MyFancyChiselType])

@grebe
Copy link
Contributor

grebe commented Mar 16, 2020

It may make sense to add a notion of Vec Literals after the backend supports vector expressions. See FIRRTL#929

@erlingrj
Copy link

Are there any updates on this feature? Would be very nice to use BundleLiterals with bundles containing vectors in Chisel-Test

@jackkoenig
Copy link
Contributor

jackkoenig commented Feb 26, 2021

There is a PR 🙂 : #1793

@ekiwi
Copy link
Contributor

ekiwi commented Aug 18, 2021

Implemented in #1834

@ekiwi ekiwi closed this as completed Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature New feature, will be included in release notes
Projects
None yet
Development

No branches or pull requests

10 participants