ucb-bar / chisel2-deprecated

chisel.eecs.berkeley.edu
388 stars 90 forks source link

A question on design #673

Closed da-steve101 closed 8 years ago

da-steve101 commented 8 years ago

Should the following work in chisel?

class Tmp extends Module {
  val io = new Bundle {
    val a = Flo(INPUT)
    val b = Flo(OUTPUT)
  }
  val aBits = io.a.toBits
  val bBits = io.b.toBits
  bBits := aBits
}

As it doesn't is the reason behind #584 but perhaps just allowing this to work would be better. Thoughts?

sdtwigg commented 8 years ago

toBits should be considered an operation and thus (generally) returns a module-internal readonly value so bBits := aBits is not allowed. You would want to do something like io.b := io.b.fromBits(aBits)

shunshou commented 8 years ago

I personally think, regardless of whether or not it's allowed, it's scary to allow conversion between Flo, Dbl and bits, simply because Flo,Dbl were never meant to be synthesizable, and anything with a more legit bit representation (Fixed, UInt, SInt) is synthesizable... Allowing a designer to locally switch between non-synthesizable constructs and synthesizable ones might lead to lots of confusion as far as the validity of RTL is concerned...

Where Flo,Dbl become exceedingly useful is in allowing the designer to verify functional correctness of equations without having to worry about fixed-point induced errors (rounding, overflow, etc.).

What I do is have a globally used "gen" that can switch context between Dbl and Fixed just for added verification. In my code, I never manually convert between the two locally.

Edit: Also might as well warn you if you haven't used Flo/Dbl too much. I don't think they've been thoroughly tested, and I have run into various bugs in the past. (like the unary_- for Dbl didn't work a few months ago; I had to change it to Dbl(0.0)-this in my Chisel type overlay).

da-steve101 commented 8 years ago

So I was doing something similar. Had an input of 512 bits which I needed to split up into a Vec of x type. Was a bit annoying that it wasn't as easy for Flo and Dbl. I use them for more than testing though. I implement the operations in blackboxes. You could overload the operations in a custom type but I find I create an object and implement the operations in functions. eg)

addDbl( a : Dbl, b : Dbl) : Dbl

Its pretty nice cause calling the function to create the Blackbox as it doesn't screw up the hierarchy either (But passing it as an argument does). I haven't run into too many issues

shunshou commented 8 years ago

Are you swapping in black boxes w/ the eventual intention of using FPUs for operations? Not sure if I got what you meant from your second to last sentence... There is some merit to implementing operations outside of the type. Specifically, a set of datatypes (SInt, UInt, Flo, Dbl) all have certain common operations, like +,-, etc., and extending them outside of the type is more flexible... (for example, it's a PITA to want to add functionality with MyUInt extends UInt, since MyUInt is not Num[UInt]) @grebe is working on a type class solution that might be of some interest to you (not sure if it's in a stable state to be released tho?)

Also, I think they're planning to migrate Flo/Dbl functionality to Chisel3 via blackboxes.

da-steve101 commented 8 years ago

Sorry it was a bit ambiguous

object Ops {
  def addFlo( a : Flo, b : Flo ) : Flo = {
    val fpAdd = Module(new MyFpAddBlackBox)
    fpAdd.io.a := a
    fpAdd.io.b := b
    fpAdd.io.res
  }
}

class MyMod[T <: Bits]( addFunct : (T, T) => T ) extends Module {
  val io = ...
  val addInputA = Flo()
  val addInputB = Flo()
  val addRes = addFunct(addInputA, addInputB)
}

Instantiate as

val m = Module(new MyMod(Ops.addFlo))

As opposed to other method which doesn't work

class MyMod( b : MyFpAddBlackBox ) extends Module {

}
da-steve101 commented 8 years ago

But a bit off topic. I can see the reasoning behind not having Flo/Dbl able to connect to bits by default. But some clean method to connect them would be nice. This is a bit ugly.

io.b := io.b.fromBits(io.a.toBits)
sdtwigg commented 8 years ago

This should work as well (particularly since Flo only has 1 form): io.b := Flo().fromBits(io.a.toBits)

da-steve101 commented 8 years ago

But that won't work with dynamic types. I guess you can put

io.b := genType.fromBits(myuintVal)

Which isn't that bad