[FIRRTL] Add Port Direction

[seldridge]

Add port direction information to the FIRRTL Dialect. Stop using an outer FlipType to mark a module output port. All modules now include an IntegerAttribute with key "portDirections" which densely stores the direction of each port as a bit. Block argument zero is in bit zero, arg one is in bit one, etc.

This is printed/parsed in a special format which has inputs looking like input %a: uint<1> and outputs looking like output %b: uint<1>. This is extremely close to the existing FIRRTL syntax.

@lattner suggested to remove input (an input isn't specially marked) and just use output to mark outputs. @jackkoenig and @azidar were in favor of keeping the input and output. Having both is closer to the spirit of FIRRTL IR, so I went with the latter approach.

After this PR, the only place a flip can show up is:

1. Inside a bundle
2. On an instance or memory port

(2) is kind of weird, but technically correct since FIRRTL IR treats the "type" of a memory or instance as a bundle. We, instead, split this up into its constituent first-level subfields. An even cleaner representation would be to either revert to a bundle representation or to strip these flips and store directionality somewhere else (in the parent module, in a portDirections attribute on an instance, or, for memories, just make it implicit).

Example

The following FIRRTL text:

circuit Foo:
  module Bar:
    input a: {a: UInt<1>, flip b: UInt<1>}
    a.b <= a.a
  module Foo:
    input a: {a: UInt<1>, flip b: UInt<1>}

    inst bar of Bar
    bar.a <= a

Is parsed into:

module  {
  firrtl.circuit "Foo"  {
    firrtl.module @Bar(input %a: !firrtl.bundle<a: uint<1>, b: flip<uint<1>>>) {
      %0 = firrtl.subfield %a("b") : (!firrtl.bundle<a: uint<1>, b: flip<uint<1>>>) -> !firrtl.uint<1>
      %1 = firrtl.subfield %a("a") : (!firrtl.bundle<a: uint<1>, b: flip<uint<1>>>) -> !firrtl.uint<1>
      firrtl.connect %0, %1 : !firrtl.uint<1>, !firrtl.uint<1>
    }
    firrtl.module @Foo(input %a: !firrtl.bundle<a: uint<1>, b: flip<uint<1>>>) {
      %bar_a = firrtl.instance @Bar  {name = "bar"} : !firrtl.flip<bundle<a: uint<1>, b: flip<uint<1>>>>
      firrtl.connect %bar_a, %a : !firrtl.flip<bundle<a: uint<1>, b: flip<uint<1>>>>, !firrtl.bundle<a: uint<1>, b: flip<uint<1>>>
    }
  }
}

After lowering, the ports get split up into separate input and output ports:

module  {
  firrtl.circuit "Foo"  {
    firrtl.module @Bar(input %a_a: !firrtl.uint<1>, output %a_b: !firrtl.uint<1>) {
      firrtl.connect %a_b, %a_a : !firrtl.uint<1>, !firrtl.uint<1>
    }
    firrtl.module @Foo(input %a_a: !firrtl.uint<1>, output %a_b: !firrtl.uint<1>) {
      %bar_a_a, %bar_a_b = firrtl.instance @Bar  {name = "bar"} : !firrtl.flip<uint<1>>, !firrtl.uint<1>
      firrtl.connect %bar_a_a, %a_a : !firrtl.flip<uint<1>>, !firrtl.uint<1>
      firrtl.connect %a_b, %bar_a_b : !firrtl.uint<1>, !firrtl.uint<1>
    }
  }
}

For the above, the port encoding can be inspected using the default printer. Both modules have port directions of 0b10:

"module"() ( {
  "firrtl.circuit"() ( {
    "firrtl.module"() ( {
    ^bb0(%a_a: !firrtl.uint<1>, %a_b: !firrtl.uint<1>):  // no predecessors
      "firrtl.connect"(%a_b, %a_a) : (!firrtl.uint<1>, !firrtl.uint<1>) -> ()
    }) {portDirections = 2 : ui2, portNames = ["a_a", "a_b"], sym_name = "Bar", type = (!firrtl.uint<1>, !firrtl.uint<1>) -> ()} : () -> ()
    "firrtl.module"() ( {
    ^bb0(%a_a: !firrtl.uint<1>, %a_b: !firrtl.uint<1>):  // no predecessors
      %bar_a_a, %bar_a_b = "firrtl.instance"() {annotations = [], moduleName = @Bar, name = "bar"} : () -> (!firrtl.flip<uint<1>>, !firrtl.uint<1>)
      "firrtl.connect"(%bar_a_a, %a_a) : (!firrtl.flip<uint<1>>, !firrtl.uint<1>) -> ()
      "firrtl.connect"(%a_b, %bar_a_b) : (!firrtl.uint<1>, !firrtl.uint<1>) -> ()
    }) {portDirections = 2 : ui2, portNames = ["a_a", "a_b"], sym_name = "Foo", type = (!firrtl.uint<1>, !firrtl.uint<1>) -> ()} : () -> ()
  }) {name = "Foo"} : () -> ()
}) : () -> ()

Metadata

Fixes #989.

[lattner]

This is looking great, but i'd recommend dropping the input keyword from the port list syntax. The default can be input and we can mark outputs explicitly.

[lattner]

This seems more clear if it called something like direction::get(bool isOutput)

[lattner]

@lattner suggested to remove input (an input isn't specially marked) and just use output to mark outputs. @jackkoenig and @azidar were in favor of keeping the input and output. Having both is closer to the spirit of FIRRTL IR, so I went with the latter approach.

I didn't see this - can you explain more of the rationale? The .mlir syntax diverges wildly from the fir syntax -- I don't see a reason that "similarity" would tip the scale. Dropping the keyword eliminates a bunch of verbosity.

[jackkoenig]

@lattner suggested to remove input (an input isn't specially marked) and just use output to mark outputs. @jackkoenig and @azidar were in favor of keeping the input and output. Having both is closer to the spirit of FIRRTL IR, so I went with the latter approach.

I didn't see this - can you explain more of the rationale? The .mlir syntax diverges wildly from the fir syntax -- I don't see a reason that "similarity" would tip the scale. Dropping the keyword eliminates a bunch of verbosity.

To me it's more about "clarity" than "similarity". It obviously depends though; if the preferred MLIR-style is to prefer terseness over clarity then I think it makes sense to remove input. If we omit input, we could also save 2 characters by using flip instead of output but perhaps that would be a step too far 🙂

[lattner]

Makes sense, I also love clarity in API design, but IR design is a bit of a different art. We stare at these dumps and hand write them a lot. I'd recommend going with "just output". But seldridge gets to make the final call, I'm good either way!

[seldridge]

I gave this some thought and combining both suggestions (improved terseness and improved clarity) would be to use "in" and "out". This is then about the same terseness as either "input" with default output or "output" with default input (on average, assuming even distribution of inputs and outputs). This would also help with any possible confusion for a new llvm/circt user, a Scala FIRRTL Compiler user/dev switching to llvm/circt, or a traditional Verilog dev who has to learn a rule about "outputs are default" or "inputs are default".

Barring no last-minute objections, I'll update this and land it later tonight.

[lattner]

sgtm