llvm / llvm/circt

[FIRRTL][LayerSink] Analyze instances and conservatively don't sink them if not safe

Open
#7,441 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug FIRRTL
Dominant language
C++
Stars
2.2k
Forks
524
Avg merge
3d 2h
Merged PRs (30d)
46

Description

Instances and anything else with side-effects cannot be sunk into a layer without changing the behavior of the base program.

Consider this FIRRTL input:

FIRRTL version 4.0.0
circuit DUT :
  layer T, bind :
  extmodule Unknown :
    output p : UInt<1>

  public module DUT :
    inst u of Unknown
    layerblock T :
      node n = u.p

Presently, firtool (via LayerSink) puts the instance of u into the layer:

// Generated by CIRCT firtool-1.80.0g20240805_461c631
// external module Unknown

module DUT_T();
  Unknown u (
    .p (/* unused */)
  );
endmodule

module DUT();
endmodule


// ----- 8< ----- FILE "layers_DUT_T.sv" ----- 8< -----

// Generated by CIRCT firtool-1.80.0g20240805_461c631
`ifndef layers_DUT_T
`define layers_DUT_T
bind DUT DUT_T t ();
`endif // layers_DUT_T

More specifically -- in case pipeline changes -- the following fed to layer sink exhibits the problem:

firrtl.circuit "DUT" {
	firrtl.layer @T bind { }
  firrtl.extmodule @Unknown(out p : !firrtl.uint<1>)
  firrtl.module public @DUT() attributes {convention = #firrtl<convention scalarized>} {
    // Who knows what this instance does, do not sink!
    %u_p = firrtl.instance u @Unknown(out p : !firrtl.uint<1>)

    firrtl.layerblock @T {
      %n = firrtl.node %u_p : !firrtl.uint<1>
    }
  }
}

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the LayerSink pass and use the supplied FIRRTL or MLIR reproducer as the first test case. Trace how the instance %u is analyzed and verify that an instance with unknown side effects remains in the base module rather than being moved into the layer; completion should preserve the conservative behavior in the generated output.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.