circt icon indicating copy to clipboard operation
circt copied to clipboard

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

Open dtzSiFive opened this issue 1 year ago • 1 comments

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>
    }
  }
}

dtzSiFive avatar Aug 05 '24 19:08 dtzSiFive

Another example, closer to what I had when first encountered this:

firrtl.circuit "TH" {
	firrtl.layer @T  bind { }
  firrtl.module @TH() attributes {convention = #firrtl<convention scalarized>} {
    %d_p = firrtl.instance d @DUT(out p: !firrtl.rwprobe<uint<1>, @T>)
    %c1_ui1 = firrtl.constant 1 : !firrtl.uint<1>
    %c1_ui1_0 = firrtl.constant 1 : !firrtl.uint<1>
    firrtl.layerblock @T {
      firrtl.ref.force_initial %c1_ui1_0, %d_p, %c1_ui1 : !firrtl.uint<1>, !firrtl.rwprobe<uint<1>, @T>, !firrtl.uint<1>
    }
  }

  firrtl.extmodule @Unknown()
  firrtl.module @DUT(out %p: !firrtl.rwprobe<uint<1>, @T>) attributes {convention = #firrtl<convention scalarized>} {
    %c0_ui1 = firrtl.constant 0 : !firrtl.uint<1>

    firrtl.instance u @Unknown()

    firrtl.layerblock @T {
      %w = firrtl.wire sym @sym : !firrtl.uint<1>
      firrtl.matchingconnect %w, %c0_ui1 : !firrtl.uint<1>
      %0 = firrtl.ref.rwprobe <@DUT::@sym> : !firrtl.rwprobe<uint<1>>
      %1 = firrtl.ref.cast %0 : (!firrtl.rwprobe<uint<1>>) -> !firrtl.rwprobe<uint<1>, @T>
      firrtl.ref.define %p, %1 : !firrtl.rwprobe<uint<1>, @T>
    }
  }
}

LayerSink puts the DUT into the layer (circt-opt --no-implicit-module --firrtl-layer-sink layer-sink-oops.mlir):

firrtl.circuit "TH" {
  firrtl.layer @T  bind {
  }
  firrtl.module @TH() attributes {convention = #firrtl<convention scalarized>} {
    firrtl.layerblock @T {
      %c1_ui1 = firrtl.constant 1 : !firrtl.uint<1>
      %d_p = firrtl.instance d @DUT(out p: !firrtl.rwprobe<uint<1>, @T>)
      %c1_ui1_0 = firrtl.constant 1 : !firrtl.uint<1>
      firrtl.ref.force_initial %c1_ui1_0, %d_p, %c1_ui1 : !firrtl.uint<1>, !firrtl.rwprobe<uint<1>, @T>, !firrtl.uint<1>
    }
  }
  firrtl.extmodule @Unknown()
  firrtl.module @DUT(out %p: !firrtl.rwprobe<uint<1>, @T>) attributes {convention = #firrtl<convention scalarized>} {
    firrtl.instance u @Unknown()
    firrtl.layerblock @T {
      %c0_ui1 = firrtl.constant 0 : !firrtl.uint<1>
      %w = firrtl.wire sym @sym : !firrtl.uint<1>
      firrtl.matchingconnect %w, %c0_ui1 : !firrtl.uint<1>
      %0 = firrtl.ref.rwprobe <@DUT::@sym> : !firrtl.rwprobe<uint<1>>
      %1 = firrtl.ref.cast %0 : (!firrtl.rwprobe<uint<1>>) -> !firrtl.rwprobe<uint<1>, @T>
      firrtl.ref.define %p, %1 : !firrtl.rwprobe<uint<1>, @T>
    }
  }
}

This test also demonstrates that instances are not sunk always -- probably they need to have uses (only) in the layer. As shown here that instance is not sunk like it is in the first post. Anyway here it's just a stand-in for something that may have side-effects.

dtzSiFive avatar Aug 05 '24 19:08 dtzSiFive