freechipsproject/firrtl

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

Kevin Laeufer

@ekiwi

Now the question is: for LowFirrtl, do we require the rhs of a connect to be of the same or smaller size than the lhs?

Essentially what I am asking is: Is this functionality of Legalize necessary to lower Mid into Lo firrtl?

Schuyler Eldridge

@seldridge:matrix.org

[m]

This is technically illegal High FIRRTL, but Scala FIRRTL has never enforced this check nor does it remove such connects until Legalize. I would say that this may have been assumed (intentionally or unintentionally) by any transform writer who wrote an inputForm=LowForm transform.

3 replies

Jack Koenig

@jackkoenig

Legalize would be better named LegalizeForVerilog or perhaps better OptimizationsThatSeemToBeRequiredToBridgeBetweenFIRRTLAndVerilogSemantics (but that one seemed a bit long). The comment should say that though

Kevin Laeufer

@ekiwi

I am trying to split it into two: https://github.com/chipsalliance/firrtl/pull/2304/commits/b98f06631f1b101b81c048b90b019549b9da8b7c

I think we should maybe add to the LoFirrtl spec that all connects have the same width on lhs and rhs.

Jack Koenig

@jackkoenig

That sounds like a good idea to me

lsteveol

@lsteveol

I have this problem, you see, some people I work with really like (ok, just about demand) that verilog ports have i_ and o_ prefixes. I was looking at a transform that @ekiwi shared in chipsalliance/chisel3#1059, so I can't claim credit for this (at least the version that worked :)). I was playing around trying to add an onPort method to update the port names, but I keep getting an internal firrtl error. Eventually it has a key not found: in java error. I have not done a lot of FIRRTL transforms so I'm assuming it's mostly me. HEre is a scastie (https://scastie.scala-lang.org/wumZa4vMRVySlfIISMmrLg)

Based on the debug printing, I'm grabbing the ports and creating a new ir.Port properly. I'm assuming there is some issue when I'm copying the module, but not really sure.

Kevin Laeufer

@ekiwi

If you change the port names of you modules, you also need to change the names at all the places that they are connected to or read.

That will make your transform a bit harder to write compared to the module name prefixing since module names are only ever used to define instances whereas port names can be used in expressions as well as when connecting to something.

Jack Koenig

@jackkoenig

You'll need to find the SubField for each of those ports on instances of the module and update it; you'll also need to rerun InferTypes (which you can do by invalidating it)

Kevin Laeufer

@ekiwi

You would also have to make sure this runs after LowerTypes since before that you could have multi-directional ports

Schuyler Eldridge

@seldridge:matrix.org

[m]

The ManipulateNames transform was supposed to be usable for this type of thing. There's a lot of machinery necessary to do this correctly and to generate the rename map. Without modifying that, you can probably write a pre-pass that will generate rename allow-lists for all input ports, then run ManipulateNames to rename those. Then run it again for output ports with a different allow-list.

Jack Koenig

@jackkoenig

Yeah I think ManipulateNames is your best bet here

Kevin Laeufer

@ekiwi

Do you have an example for that?

Schuyler Eldridge

@seldridge:matrix.org

[m]

https://github.com/chipsalliance/firrtl/blob/master/src/test/scala/firrtlTests/transforms/ManipulateNamesSpec.scala shows how you could do this.

That has examples of doing prefixing/suffixing. The only thing missing is generating allow-list annotations to keep the name changes targeted to just inputs or outputs.

Kevin Laeufer

@ekiwi

Is there ever a reason why a TransformManager would not reschedule a transform after it has been invalidated?

I currently have a SplitExpression pass that is scheduled as a cleanup by the TransformManager but somehow the manager refuses to move it in front of another transform that is invalidated by SplitExpressoion and is also does not reschedule the invalidated transform.

Schuyler Eldridge

@seldridge:matrix.org

[m]

It may not be rescheduled if it's not necessary by something downstream. If it's in the target list, it should always be rescheduled after invalidation.

Also, the dependency manager ordering is not optimal, but it should be valid. (See: chipsalliance/firrtl#1808)

lsteveol

@lsteveol

Thanks for the reference to ManipulateNames. I'm a little out of my league here and this one looks like a doosy. I will try to see if I can make sense out of it.

2 replies

Schuyler Eldridge

@seldridge:matrix.org

[m]

Yep, though you can probably just use an inspecting aspect or whatever just generates annotations. You don't need to actually inject code here (fortunately).

sam-shahrestani

@sam-shahrestani

I'm getting a weird interaction between ForceNameAnnotation and InlineAnnotation. I have 4 modules with InlineAnnotation on them and 4 instances (one for each module) with ForceNameAnnotation on them using the chisel3 util. The Inline works correctly but ForceName ends up renaming the module that wraps these 4 modules, which is very unusual. Am I misunderstanding how this works or do these 2 annotations just not work with each other?

1 reply

lsteveol

@lsteveol

While I'm pretty sure I already know the answer to this I'm going to ask anyways. Is there a transform that can remove the intermediate assign statements from the Verilog? I have part of a design that is a mix of analog cells. We put this together with Chisel, and later import that back into virtuoso with a schematic import of the verilog. The intermediate assign statements cause a cds_thru cell to be added for each one, which is annoying the layout teams as they now have to add a layout component for this.

Based on the fact that a <= connect seems to always include this, I don't believe it's possible. I cannot use the Analog attach in this case as most of the ports do have some directionality to them and I place part of the analog cells as a Bundle.

31 replies

Kevin Laeufer

@ekiwi

Does anyone know if Shell.parse is reentrant?

4 replies

anoop

@mysoreanoop

Very fundamental newb here;

I'm able to convert a ~4 MB sized RocketTile's FIR to verilog using this:

firrtl -i rocket.fir -X verilog -o rocket --log-file logFile -ll info

But when trying the same for ~102 MB sized BlackParrot's FIR (obtained through Yosys) to verilog using that same command, it doesn't progress, no running logs either unlike that of rocket's run. Anyway to debug further?

6 replies

Tom Alcorn

@tdb-alcorn

can someone remind me, what's the easiest way to disable individual optimization passes in firrtl?

2 replies

Schuyler Eldridge

@seldridge:matrix.org

[m]

Interesting. What were you seeing for the queue? It's supposed to do structural deduplication. Basically, if the structure is the same, but the names are different, it will merge them.

5 replies

Some transforms do have the ability to disable them. For others, you may need to create a custom transform manager with a different set of target transforms. Do note that some "optimizations" are actually required and transforms may have hard requirements on other transforms running (meaning some transforms can't be disabled). You can look at src/main/scala/firrtl/stage/Forms.scala to see how this is broken up. I think there's some vals about optimized vs. minimum optimized that may shed some light on what's necessary.

Schuyler Eldridge

@seldridge:matrix.org

[m]

Got it. Yeah, there's no difference between Bool and a 1-but UInt in FIRRTL. They're both UInt<1> inside FIRRTL.

Tynan McAuley

@tymcauley

Hello all! I've been writing a custom FIRRTL transform which does some circuit analysis based on connectivity information. To do that analysis, I ended up using the CircuitGraph class, and the fanInSignals/fanOutSignals methods to traverse the circuit. This has worked out really well, but while I was putting this together, I looked at some of the existing FIRRTL transforms for inspiration. In particular, CheckCombLoops seemed like a transform that would be doing similar work. However, I noticed that this transform uses an InstanceKeyGraph, and appears to build its own connectivity information. From the comments in that file, it appears this is a performance optimization (building connectivity information module-by-module rather than for the whole circuit). Is that the main motivation behind not using the CircuitGraph?

Jiuyang Liu

@sequencer

I think there are two reasons

circuit graph is some younger code than those passes.
Constructing circuit graph need to construct that huge DiGraph, which is really big effort, I did look at ConstantProp and CheckCombLoop passes, those passes are optimized for performance.

Tynan McAuley

@tymcauley

Awesome, that makes sense, thanks for the explanation! And ya, I can see how rather than fully elaborating the circuit netlist, you can just extract inter-module connectivity data for each module definition, and then use that to do all of the comb-loop checking.

Tynan McAuley

@tymcauley

Higher-level question about these data structures: Could the ConnectionGraph be refactored to use the same strategy as the CheckCombLoops transform? So instead of elaborating every module instance, it could point to the elaborated connectivity sub-graph for that module? There'd be a bit more translation involved when extracting a ReferenceTarget from this system (probably similar in concept this this method from CheckCombLoops).

Is there any functionality that we'd lose by going this route? I guess it's not entirely clear if this would be a performance win when working on a design that doesn't have a whole lot of re-used hierarchical instances.

Muhammad Hadir Khan

@hadirkhan10

I am looking to dig deep into the SRAM-specific domain for chisel/firrtl/Verilog. Looking here: https://stackoverflow.com/questions/66344890/syncreadmem-generated-verilog-vs-rocketchip-emitted-verilog
I know a flag replaces SyncReadMem in chisel with black box Verilog modules. Is this Chisel specific? Or if I can generate a FIRRTL mem type, this same flag will emit Verilog with srams as a black box and a .conf file?

1 reply

Muhammad Hadir Khan

@hadirkhan10

I am seeing some work being done in the past by @stevenmburns @baltazarortiz on enabling yosys to read the Verilog and emit FIRRTL. I am looking to use Yosys to read Verilog, detect SRAMs and emit FIRRTL with mem type so that I can use the --repl-seq-mem flag to generate verilog with black boxing the SRAMs and generate a .conf file. If there is work already done on this, I would like some links to source code.

4 replies

the_tech_lover

@69ca9e084e3d4a8_twitter

Inside the AddDefaults phase, the transform method has the following code fragments:

      // if there is no compiler or emitter specified, add the default emitter
      (if (em) Seq(RunFirrtlTransformAnnotation(DefaultEmitterTarget)) else Seq())

The comment says if there is no compiler or emitter specified, add the default emitter. But seems that the existence of a CompilerAnnotation does not get checked, only the RunFirrtlTransformAnnotation(_: firrtl.Emitter):

    var bb, em, im = true
    annotations.foreach {
      case _: BlackBoxTargetDirAnno => bb = false
      case _: InfoModeAnnotation    => im = false
      case RunFirrtlTransformAnnotation(_: firrtl.Emitter) => em = false
      case _ =>
    }

Will this be a problem?
Another minor thing: the val default = new FirrtlOptions() variable has no reference elsewhere in the transform method here.

the_tech_lover

@69ca9e084e3d4a8_twitter

Inside the AddDefaults phase, the transform method has the following code fragments:
      // if there is no compiler or emitter specified, add the default emitter
      (if (em) Seq(RunFirrtlTransformAnnotation(DefaultEmitterTarget)) else Seq())
The comment says if there is no compiler or emitter specified, add the default emitter. But seems that the existence of a CompilerAnnotation does not get checked, only the RunFirrtlTransformAnnotation(_: firrtl.Emitter):
    var bb, em, im = true
    annotations.foreach {
      case _: BlackBoxTargetDirAnno => bb = false
      case _: InfoModeAnnotation    => im = false
      case RunFirrtlTransformAnnotation(_: firrtl.Emitter) => em = false
      case _ =>
    }
Will this be a problem?
Another minor thing: the val default = new FirrtlOptions() variable has no reference elsewhere in the transform method here.

Seems that another RunFirrtlTransformAnnotation(compiler.get.emitter) will be added to the annotation sequence during the phase of AddImplicitEmitter when there exists a CompilerAnnotation:

        case a: CompilerAnnotation =>
          Seq(
            a,
            RunFirrtlTransformAnnotation(compiler.get.emitter),
            EmitCircuitAnnotation(compiler.get.emitter.getClass)
          )

So, there will be two emitters added. Is this even allowed?

2 replies

jurevreca12

@jurevreca12

Hello, I am trying ot use treadle directly to simulate my Chisel design. The example on https://github.com/chipsalliance/treadle.git seems to be using an older version of Chisel (I am using 3.5.2). Concretely I am having trouble with the

code:val s = Driver.emit(() => new GCD)
val tester = TreadleTester(s)

The Driver.emit() api is deprecated in 3.5.2. So now I am not sure, how to simulate my chisel design with treadle.

7 replies

jurevreca12

@jurevreca12

One of these code snippets runs much faster than the other. Why is this the case? Also what is the quickest way to get to a TreadleTester object using a chisel design?
The first snippet is:

tester = TreadleTester((new ChiselStage).execute(Array(), Seq(ChiselGeneratorAnnotation(() => new ProcessingPipeline(lbirModel))))

and it runs for around 35 seconds.
The second snippet is:

tester = TreadleTester(Seq(FirrtlSourceAnnotation((new ChiselStage).emitChirrtl(new ProcessingPipeline(lbirModel))))

and the second snippet runs for around 208 seconds. This seemed counter intuitve to me, as Chirrtl is the highest level IR, and so should be the quickest to obtain. Right?

Radhika Jain

@poddar92

I am trying to write a new transform to add TDR ports in the memory module in rocketchip; using SourceAnnotation and SinkAnnotation i could add input TDR ports to the mem_ext modules; Anyone know how I can add the output port from the memory_ext modules? these modules are replaced/created using --repl-seq-mem transform in FIRRTL. The SourceAnnotation in FIRRTL transform takes the signalName as input (not module), and in chisel source this module does not exist.

Ryan Macdonald

@ryanmacdonald

Are there any examples of how I can run custom FIRRTL transforms as part of elaborating Verilog that aren't part of the FIRRTL project's src/main/scala/firrtl/passes directory? Or can the --custom-transforms flag to FIRRTL magically accept arguments that are in extraneous packages? The background here is that I'm trying to use a pre-built firrtl.jar to elaborate my Verilog and still run my custom passes, instead of having a patch on top of my FIRRTL version and re-building everything

4 replies

Abraham Gonzalez

@abejgonzalez

Hi everyone. Is it possible for the SFC to run in a “no-op” mode where only if RunTransform annotations are found then it will run all necessary dependent passes for that transform, the annotation given transform, and nothing more? For more context, I was hoping to use SFC as a pre-processing step that can handle Interval/Fixed types and custom transforms before the CIRCT FIRRTL compiler. See https://github.com/ucb-bar/chipyard/pull/1239#discussion_r987063721. Thanks in advance!

Kevin Laeufer

@ekiwi

I would just run the passes manually

Abraham Gonzalez

@abejgonzalez

How would dependencies be handled if they are run manually? For example, if a pass depends on Forms.LowForm, would that automatically be run? What about if there are multiple RunTransform annotations - how would order be determined?

Kevin Laeufer

@ekiwi

I was suggesting to just instantiate the passes and call the execute function manually, however that might not be super practical.

Alternatively you could create your own Compiler instance and give it the passes that you want to run explicitly (you might have to write some code to dig through the annotations yourself): https://github.com/firesim/firesim/blob/366ceabe13ce27056ec1c53bf93d017ec182d85e/sim/midas/src/test/scala/firrtl/testutils/LeanTransformSpec.scala#L20

Abraham Gonzalez

@abejgonzalez

Moving discussion into a thread

12 replies

Muhammad Hadir Khan

@hadirkhan10

Does FIRRTL support generating always_latch sv construct?

3 replies

Where communities thrive

People

Repo info

Activity