Direction Woes

[jackkoenig]

Consider the following compatibility code:

import Chisel._

object ChiselTest {

  class MyBundle extends Bundle {
    val input = UInt(INPUT, width = 32)
    val output = UInt(OUTPUT, width = 32)
    val default = UInt(width = 32)
    val flipped = UInt(width = 32).flip
  }

  class MyModule extends Module {
    val gen = new MyBundle
    println("gen")
    println(gen.input.dir)
    println(gen.output.dir)
    println(gen.default.dir)
    println(gen.flipped.dir)
    println()

    val io = gen.flip
    println("MyModule.io")
    println(io.input.dir)
    println(io.output.dir)
    println(io.default.dir)
    println(io.flipped.dir)
    println()

    println("Wire")
    val wire = Wire(new MyBundle)
    println(wire.input.dir)
    println(wire.output.dir)
    println(wire.default.dir)
    println(wire.flipped.dir)
    println()
  }
}

object TestChisel extends App {
  chisel3.Driver.execute(args, () => new ChiselTest.MyModule)
}

Prior to the directions refactor, this would print

gen
input
output
unspecified
unspecified

MyModule.io
output
input
unspecified
unspecified

Wire
unspecified
unspecified
unspecified
unspecified

On master (with exceptions replaced with text), this prints

gen
INPUT
OUTPUT
NODIR
Throw an Exception!

MyModule.io
INPUT
OUTPUT
NODIR
Throws an Exception!

Wire
INPUT
OUTPUT
NODIR
NODIR

There are several issues here:

• Wires now have directions (this behavior is also present in chisel3)
• Flip of a default results in an exception instead of just NODIR
• The directions of MyModule.io are wrong

The behavior of the last point is due to the fact that Chisel.dir gives ActualDirection if the Data is bound, otherwise it gives UserDirection. What Chisel.dir used to do was give the "current actual direction." If something has a local direction that is later flipped, .dir would give the flipped direction.

It looks like there really 3 "types" of direction:

1. Actual Direction of bound hardware
2. The local direction specified inside of a bundle
3. The "current" direction of unbound type (ie. if you were to bind it now, what would the actual direction be)

It seems to me that Chisel.dir implements only (3), which makes sense, because (3) can give all of the information of the other 2 if just in a somewhat less obvious way.

In my opinion, (3) is much more useful than (2), especially because (2) is pretty easy to derive with (3). For example

val myBundle = new Bundle {
  val foo = Flipped(new Bundle {
    val bar = Input(UInt(32.W))
  })
}

// Using just .dir with (3) behavior
myBundle.foo.cloneType.bar.dir // INPUT
myBundle.foo.bar.dir // OUTPUT

[jackkoenig]

Perhaps of interest to @sdtwigg as well

[ducky64]

So:

• Under new semantics, Bundles cannot mix explicitly directioned and default direction (no-dir) elements. In this case, flipping a no-dir element doesn't make sense at all (essentially a no-op) and is probably indicative of an user error. But it shouldn't be producing exceptions under Chisel compatibility, can you paste the error you're getting?
• Whether Wire / Reg / etc should have an ActualDirection when asked for can be the subject of debate. I don't think they are checked internally. Under chisel3 semantics, Wire / Reg / etc elements must all be the same direction. I don't know if that check is in yet.
• For MyModule.io being wrong: the chisel3 semantics (separate UserDirection/SpecifiedDirection from ActualDirection) makes the compatibility Data.dir a hack at best. The reasoning is that before you've bound a data type, you never know if something above it is going to override the direction (either forced-Input/Output or Flip), so asking for its final direction is meaningless and could lead to subtle bugs. Of course, Chisel did let you do unsafe things like that, though the data structures aren't there anymore. I think Chisel altered the direction of all children on a flip, while the new system only maintains local direction plus a parent pointer, though the parent pointer is only known at binding-time. Nasty workarounds (like populating the parent pointer on a Flip / etc) are possible, though.

[ducky64]

Did #673 resolve this?

[jackkoenig]

I guess it's resolved enough. It's still a regression in functionality but transparently so. Posterity can reopen if it comes up again.