Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request][FIRRTL] add a pass on firrtl.circuit to create a firrtl.extmodule for all firrtl.instance without its firrtl.module or firrtl.extmodule #8030

Closed
sequencer opened this issue Jan 4, 2025 · 2 comments

Comments

@sequencer
Copy link
Contributor

sequencer commented Jan 4, 2025

This is for the separate linking flow, this is a almost minimal example provided generated by the zaozi project:

#loc = loc(unknown)
"firrtl.circuit"() <{annotations = [], name = "ReferableSpec"}> ({
  "firrtl.module"() <{annotations = [], convention = #firrtl<convention scalarized>, layers = [], portAnnotations = [], portDirections = array<i1: false, false, true>, portLocations = [#loc, #loc, #loc], portNames = ["asyncDomain", "syncDomain", "passthrough"], portSymbols = [], portTypes = [!firrtl.bundle<reset: asyncreset, clock: clock>, !firrtl.bundle<reset: uint<1>, clock: clock>, !firrtl.bundle<i flip: uint<8>, o: uint<8>>], sym_name = "ReferableSpec"}> ({
  ^bb0(%arg0: !firrtl.bundle<reset: asyncreset, clock: clock>, %arg1: !firrtl.bundle<reset: uint<1>, clock: clock>, %arg2: !firrtl.bundle<i flip: uint<8>, o: uint<8>>):
    %0 = "firrtl.wire"() <{annotations = [], name = "io", nameKind = #firrtl<name_kind droppable_name>}> : () -> !firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>
    %1 = "firrtl.subfield"(%0) <{fieldIndex = 0 : i32}> : (!firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>) -> !firrtl.bundle<reset: asyncreset, clock: clock>
    "firrtl.connect"(%1, %arg0) : (!firrtl.bundle<reset: asyncreset, clock: clock>, !firrtl.bundle<reset: asyncreset, clock: clock>) -> ()
    %2 = "firrtl.subfield"(%0) <{fieldIndex = 1 : i32}> : (!firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>) -> !firrtl.bundle<reset: uint<1>, clock: clock>
    "firrtl.connect"(%2, %arg1) : (!firrtl.bundle<reset: uint<1>, clock: clock>, !firrtl.bundle<reset: uint<1>, clock: clock>) -> ()
    %3 = "firrtl.subfield"(%0) <{fieldIndex = 2 : i32}> : (!firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>) -> !firrtl.bundle<i flip: uint<8>, o: uint<8>>
    "firrtl.connect"(%arg2, %3) : (!firrtl.bundle<i flip: uint<8>, o: uint<8>>, !firrtl.bundle<i flip: uint<8>, o: uint<8>>) -> ()
    %4 = "firrtl.subfield"(%0) <{fieldIndex = 0 : i32}> : (!firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>) -> !firrtl.bundle<reset: asyncreset, clock: clock>
    %5 = "firrtl.subfield"(%4) <{fieldIndex = 1 : i32}> : (!firrtl.bundle<reset: asyncreset, clock: clock>) -> !firrtl.clock
    %6 = "firrtl.subfield"(%0) <{fieldIndex = 0 : i32}> : (!firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>) -> !firrtl.bundle<reset: asyncreset, clock: clock>
    %7 = "firrtl.subfield"(%6) <{fieldIndex = 0 : i32}> : (!firrtl.bundle<reset: asyncreset, clock: clock>) -> !firrtl.asyncreset
    %8:3 = "firrtl.instance"() <{moduleName = @ReferableSpec, name = "inst0", nameKind = #firrtl<name_kind interesting_name>, portDirections = array<i1: false, false, true>, portNames = ["asyncDomain", "syncDomain", "passthrough"]}> : () -> (!firrtl.bundle<reset: asyncreset, clock: clock>, !firrtl.bundle<reset: uint<1>, clock: clock>, !firrtl.bundle<i flip: uint<8>, o: uint<8>>)
    %9 = "firrtl.wire"() <{annotations = [], name = "ReferableSpec_io", nameKind = #firrtl<name_kind droppable_name>}> : () -> !firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>
    %10 = "firrtl.subfield"(%9) <{fieldIndex = 0 : i32}> : (!firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>) -> !firrtl.bundle<reset: asyncreset, clock: clock>
    "firrtl.connect"(%8#0, %10) : (!firrtl.bundle<reset: asyncreset, clock: clock>, !firrtl.bundle<reset: asyncreset, clock: clock>) -> ()
    %11 = "firrtl.subfield"(%9) <{fieldIndex = 1 : i32}> : (!firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>) -> !firrtl.bundle<reset: uint<1>, clock: clock>
    "firrtl.connect"(%8#1, %11) : (!firrtl.bundle<reset: uint<1>, clock: clock>, !firrtl.bundle<reset: uint<1>, clock: clock>) -> ()
    %12 = "firrtl.subfield"(%9) <{fieldIndex = 2 : i32}> : (!firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>) -> !firrtl.bundle<i flip: uint<8>, o: uint<8>>
    "firrtl.connect"(%12, %8#2) : (!firrtl.bundle<i flip: uint<8>, o: uint<8>>, !firrtl.bundle<i flip: uint<8>, o: uint<8>>) -> ()
    %13 = "firrtl.subfield"(%0) <{fieldIndex = 2 : i32}> : (!firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>) -> !firrtl.bundle<i flip: uint<8>, o: uint<8>>
    %14 = "firrtl.subfield"(%13) <{fieldIndex = 1 : i32}> : (!firrtl.bundle<i flip: uint<8>, o: uint<8>>) -> !firrtl.uint<8>
    %15 = "firrtl.subfield"(%9) <{fieldIndex = 2 : i32}> : (!firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>) -> !firrtl.bundle<i flip: uint<8>, o: uint<8>>
    %16 = "firrtl.subfield"(%15) <{fieldIndex = 1 : i32}> : (!firrtl.bundle<i flip: uint<8>, o: uint<8>>) -> !firrtl.uint<8>
    "firrtl.connect"(%14, %16) : (!firrtl.uint<8>, !firrtl.uint<8>) -> ()
    %17 = "firrtl.subfield"(%9) <{fieldIndex = 2 : i32}> : (!firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>) -> !firrtl.bundle<i flip: uint<8>, o: uint<8>>
    %18 = "firrtl.subfield"(%17) <{fieldIndex = 0 : i32}> : (!firrtl.bundle<i flip: uint<8>, o: uint<8>>) -> !firrtl.uint<8>
    %19 = "firrtl.subfield"(%0) <{fieldIndex = 2 : i32}> : (!firrtl.bundle<asyncDomain flip: bundle<reset: asyncreset, clock: clock>, syncDomain flip: bundle<reset: uint<1>, clock: clock>, passthrough: bundle<i flip: uint<8>, o: uint<8>>>) -> !firrtl.bundle<i flip: uint<8>, o: uint<8>>
    %20 = "firrtl.subfield"(%19) <{fieldIndex = 0 : i32}> : (!firrtl.bundle<i flip: uint<8>, o: uint<8>>) -> !firrtl.uint<8>
    "firrtl.connect"(%18, %20) : (!firrtl.uint<8>, !firrtl.uint<8>) -> ()
  }) : () -> ()
}) : () -> ()

Since our eDSL frontend only can operate on the firrtl.module block,: It's not possible to insert the firrtl.extmodule to firrtl.circuit at the time when adding the firrtl.instance to firrtl.module.

It generates a non-valid firrtl.circuit, since all instances cannot refer to the firrtl.extmodules.

Since firrtl.instance operator already contains the necessary information for creating the external module, it's reasonable to adding all non-refered firrtl.instance as firrtl.extmodule to firrtl.circuit with a simple pass.

@uenoku
Copy link
Member

uenoku commented Jan 6, 2025

I don't think we can support this. This is ill-formed IR and it's frontend's responsibility to produce well-formed IR.

Since our eDSL frontend only can operate on the firrtl.module block,: It's not possible to insert the firrtl.extmodule to firrtl.circuit at the time when adding the firrtl.instance to firrtl.module.

I'm not sure how your eDSL works but MLIR CAPI provides standard builder utilities so you should be able to change insertion point to firrtl.circuit body.

@sequencer
Copy link
Contributor Author

Thanks! I found it's simple to do the post-processing at the final stage of firrtl.circuit generation with the mlirOperationWalk API. Also discussed with @seldridge about this. I agree using a spacial pass to do so is not good at all. Since we should provide a valid IR for each stage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants