chipsalliance / firrtl

Flexible Intermediate Representation for RTL
https://www.chisel-lang.org/firrtl/
Apache License 2.0
722 stars 176 forks source link

Support lowering single-bit bit vectors to single-bit bit vectors in Verilog #733

Open michaeljclark opened 6 years ago

michaeljclark commented 6 years ago

This repository's issues are reserved for feature requests and bug reports.

We are trying to blackbox some external Verilog IP that contains the following types:

  output [1:0]   EX1_BITVEC2,
  output [0:0]   EX2_BITVEC1,
  output         EX3_BOOL

We have declarations in Chisel like this:

  val EX1_BITVEC2 = IO(Output(UInt(2.W)))
  val EX2_BITVEC1 = IO(Output(UInt(1.W)))
  val EX3_BOOL = IO(Output(Bool()))

The FIRRTL output unfortunately can't distinguish the single-bit vector case and outputs the following:

  output [1:0]   EX1_BITVEC2,
  output         EX2_BITVEC1,
  output         EX3_BOOL

This causes Vivado to fail when the external IP tries to connect to EX2_BITVEC1[0]

The problem is FIRRTL doesn't support a Boolean type and instead always lowers single bit bitvectors into booleams. We are wondering if it would be possible to annotate specific single bit bitvectors with a label such they can be emitted as single-bit bitvectors instead of booleans. e.g.

  val EX2_BITVEC1 = IO(Output(UInt(1.W)), lowerAsVector=true)

Our only use case is for blackboxing external IP, which unfortunately we can't change. We don't mind if the annotation is on either the IO, Input, Output or UInt. I'm not sure what would work best. Basically the circuit needs enough information

The problem is the annotation needs to be added to the FIRRTL and I'm not sure how it could be represented in FIRRTL other than as a comment.

    output EX2_BITVEC1 : UInt<1> @[lowerAsVector=true]

The most elegant fix would be adding a Bool type to FIRRTL, but for source compatibility reasons one would need to continue to lower single-bit bit vectors as Bools.

Currently, we have a sed script to replace said signals in the generated Verilog, which is pretty inelegant. It's an external IP so we have no other way around this issue at present.

seldridge commented 6 years ago

One quick comment if this winds up going the Chisel annotation approach:

The problem is the annotation needs to be added to the FIRRTL and I'm not sure how it could be represented in FIRRTL other than as a comment.

The annotations live in a separate file, either manually written or generated by something (usually the Chisel compiler). For the Chisel-flow, you tag these signals in Chisel with an annotator, write a FIRRTL pass to consume them. E.g., all the "don't touch" annotations in rocket-chip master are generating the following in freechips.rocketchip.system.DefaultConfig.anno:

- targetString: TestHarness.DCache_dcache.io.cpu.resp
  transformClass: firrtl.Transform
  value: DONTtouch!

Chisel3 code here: https://github.com/freechipsproject/chisel3/blob/c327dc328ca819031a086ae102fefe2909831e24/chiselFrontend/src/main/scala/chisel3/core/ChiselAnnotation.scala#L34

The @[...] is the "Info" of the IR and is largely only source locators, though this can technically be any string you want to cram in there.

The prototype for what you want is something like:

- targetString: MyCircuit.MyModule.EX2_BITVEC1
  transformClass: firrtl.EmitAnnotation
  value: singleBitVec

Then some annotator that you write that tags the signal SingleBitVecEmit(EX2_BITVEC1). That automatically emits your annotations for you and FIRRTL acts on them.

The emitters do consume EmitAnnotation and the specific tweak based on what you want is here: https://github.com/freechipsproject/firrtl/blob/master/src/main/scala/firrtl/Emitter.scala#L206. The "emit one module per file" option works like this as an annotation.

It may be worthwhile to have a discussion if the Verilog emitter should be modified to emit [0:0] if that is more canonical.