freechipsproject/firrtl

For real-time public discussion of FIRRTL. Debates requiring community feedback should be done in firrtl/issues. User questions are welcome.

Jack Koenig

@jackkoenig

I have not tested this, but a transform:

import firrtl._
import firrtl.ir._
import firrtl.Foreachers._
import firrtl.annotations._
import firrtl.passes.memlib._

import scala.collection.mutable

class DontDedupSeqMems extends Transform {                                                                                                                                        
  def inputForm: CircuitForm = HighForm
  def outputForm: CircuitForm = HighForm

  // Collect all SeqMems from a Module
  private def collectSeqMems(module: DefModule): Seq[String] = {
    val seqMems = mutable.ArrayBuffer.empty[String]
    // Iterate on Statements
    def onStmt(stmt: Statement): Unit = stmt.foreach(onStmt) match {
      case m: DefMemory => ToMemIR.updateStmts(m) match {
          case smem: DefAnnotatedMemory =>
            seqMems += smem.name
          case _ =>
        }
      case _ =>
    }
    module.foreach(onStmt)
    seqMems
  }

  def execute(state: CircuitState): CircuitState = {
    val cirName = CircuitName(state.circuit.main)
    val annos = state.circuit.modules.flatMap { m =>
      val mems = collectSeqMems(mems)
      val modName = ModuleName(m.name, cirName)
      mems.map(mem => NoDedupMemAnnotation(ComponentName(mem, modName)))
    }
    state.copy(annotations = annos ++ state.annotations)
  }
}

Julius Baxter

@juliusbaxter

Hmm, the first method didn't quite work

looks like firrtl read in the file but didn't change anything. How would I invoke the transform? The FIRRTL github wiki doesn't quite explain that bit...

Jack Koenig

@jackkoenig

you sure you spelt the names right on the first? it's not a full path, just the name of the top module, name of module containing the mem, then the name of the mem

for the second, you run custom transforms with -fct <package path and name of transform>

Julius Baxter

@juliusbaxter

ahhh

Jack Koenig

@jackkoenig

-fct DontDedupSeqMems in this case

Julius Baxter

@juliusbaxter

sorry okay, I did the full path in my design,

Jack Koenig

@jackkoenig

that kind of thing should work but doesn't fully right now, and not with this annotation haha

Julius Baxter

@juliusbaxter

sweet okay thanks, I'll have a crack at that after revistiing the first method

all good man

Julius Baxter

@juliusbaxter

@jackkoenig by <module_name> and <mem_name> are either of those instance names?

Jack Koenig

@jackkoenig

Module name is not instance, mem name is

It wouldn't work for a cmem, but consider FPU from firrtl/regress: https://github.com/freechipsproject/firrtl/blob/master/regress/FPU.fir#L373

If that were a mem we were preventing dedup, we would write the target as: FPU.FPU.regfile

it just so happens to be in the top module, but yeah

Julius Baxter

@juliusbaxter

mem = smem?

Julius Baxter

@juliusbaxter

hmm, kinda works but not entirely for some reason. have like > 8 instances of a particular memory, it's only adding 2 new versions of it

Jack Koenig

@jackkoenig

mem = smem, are those memories in modules that are being deduped?

Kamy

@kammoh

My FPGA synthesis tool can infer efficient ternary adders only when the three input signals are on the same expression/line, e.g.A <= B + C + D. Is there any hackish way to add ternary (or n-input, n>2) ops to lowFIRRTL and/or the Verilog emitter?

Jack Koenig

@jackkoenig

Addition can get tricky because of the widths

Kamy

@kammoh

max(B.width,C.width,D.width)?

Jack Koenig

@jackkoenig

I mean in Verilog, the FIRRTL semantics are clear

In Verilog, the width of the LHS of the assignment affects intermediate widths on the RHS

Kamy

@kammoh

I see, that's true

Jack Koenig

@jackkoenig

I'm struggling to see how this can go wrong in pure addition though

but if you combine operators, something like assign a = (b + c) >> 1

if the width of a is the same as that of b, and c, you lose the overflow

thus there can be semantic difference between:

assign _T_0 = b + c
assign a = _T_0 >> 1

and

assign a = (b + c) >> 1

But like I said, pure addition may be okay

anyway I don't think there's an easy hack, I think you'd have to do something like: freechipsproject/firrtl#851

Kamy

@kammoh

yeah I think it should be fine. The n-ary add/sub expressions in the emitted verilog could also help with readability of the generated code.

Jack Koenig

@jackkoenig

I'll keep thinking about any possible issues but if you want to try it, go ahead and I think #851 is a good template for how to do it

Kamy

@kammoh

Thanks for the pointer! #851 is indeed looks like what I should try to do

Jack Koenig

@jackkoenig

Another simple improvement to the emitted verilog would be inlining mux conditions

Kamy

@kammoh

What do you exactly mean by inlining?

Jack Koenig

@jackkoenig

assign _T_0 = a && b;
assign z = _T_0 ? x : y;
// vs.
assign z = a && b ? x : y;

The same thing as "combining"

Kamy

@kammoh

Oh yes that I had also thought about

Jack Koenig

@jackkoenig

Also note, don't make addition "ternary", just have a nested expression add(x, add(y, z))

Kamy

@kammoh

Though I don't think it's gonna affect the QoR in any flow

Jack Koenig

@jackkoenig

you can emit it as x + (y + z), and if the tool doesn't like that then try x + y + z

I know the DoPrims have a Seq for their expression args, but it's still not supposed to be more than 2

Kamy

@kammoh

and check if one of the operands is itself add/sub then emit the whole chunk and not traverse the inner?

got you

That's reasonable

Jack Koenig

@jackkoenig

Yeah, it's simpler from an IR perspective to only have to handle 2 args, but of course we can always emit different Verilog

Thus what combine cats does

Kamy

@kammoh

Thank you so much @jackkoenig!

Jack Koenig

@jackkoenig

Of course!

Where communities thrive

People

Repo info

Activity