8000 [OM] Create OM Nodes for metadata by prithayan · Pull Request #5224 · llvm/circt · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[OM] Create OM Nodes for metadata #5224

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 21 commits into from
Jun 2, 2023
Merged

[OM] Create OM Nodes for metadata #5224

merged 21 commits into from
Jun 2, 2023

Conversation

prithayan
Copy link
Contributor
@prithayan prithayan commented May 17, 2023 •
edited
Loading

Create OM nodes to represent the json metadata in CreateSifiveMetadata.
This adds the required infrastructure for including the om dialect, to FIRRTL.
The emit-metadata pass now generates the metadata as om.class in the IR, in addition to the verbatim.
The lit tests also check that the om dialect is generated and preserved in the mlir after ExportVerilog.

Example IR:
Memory metadata:

    firrtl.memmodule private @head_ext(in W0_addr: !firrtl.uint<5>, in W0_en: !firrtl.uint<1>, in W0_clk: !firrtl.clock, in W0_data: !firrtl.uint<5>) attributes {dataWidth = 5 : ui32, depth = 20 : ui64, extraPorts = [], maskBits = 1 : ui32, numReadPorts = 0 : ui32, numReadWritePorts = 0 : ui32, numWritePorts = 1 : ui32, readLatency = 1 : ui32, writeLatency = 1 : ui32}
    firrtl.memmodule private @head_0_ext(in W0_addr: !firrtl.uint<5>, in W0_en: !firrtl.uint<1>, in W0_clk: !firrtl.clock, in W0_data: !firrtl.uint<5>) attributes {dataWidth = 5 : ui32, depth = 20 : ui64, extraPorts = [], maskBits = 1 : ui32, numReadPorts = 0 : ui32, numReadWritePorts = 0 : ui32, numWritePorts = 1 : ui32, readLatency = 1 : ui32, writeLatency = 1 : ui32}

    om.class @MemoryConf(%name: !om.str, %depth: ui64, %width: ui32, %maskBits: ui32, %readPorts: ui32, %writePorts: ui32, %readwritePorts: ui32, %writeLatency: ui32, %readLatency: ui32) {
      om.class.field @name, %name : !om.str
      om.class.field @depth, %depth : ui64
      om.class.field @width, %width : ui32
      om.class.field @maskBits, %maskBits : ui32
      om.class.field @readPorts, %readPorts : ui32
      om.class.field @writePorts, %writePorts : ui32
      om.class.field @readwritePorts, %readwritePorts : ui32
      om.class.field @writeLatency, %writeLatency : ui32
      om.class.field @readLatency, %readLatency : ui32
    }
    om.class @MemoryMetadata() {
      %0 = om.constant #om.str<"head_ext"> : !om.str
      %1 = om.constant 20 : ui64
      %2 = om.constant 5 : ui32
      %3 = om.constant 1 : ui32
      %4 = om.constant 0 : ui32
      %5 = om.constant 1 : ui32
      %6 = om.constant 0 : ui32
      %7 = om.constant 1 : ui32
      %8 = om.constant 1 : ui32
      %9 = om.object @MemoryConfModules(%0, %1, %2, %3, %4, %5, %6, %7, %8) : (!om.str, ui64, ui32, ui32, ui32, ui32, ui32, ui32, ui32) -> !om.class.type<@MemoryConfModules>
      om.class.field @m0, %9 : !om.class.type<@MemoryConfModules>
      %10 = om.constant #om.str<"head_0_ext"> : !om.str
      %11 = om.constant 20 : ui64
      %12 = om.constant 5 : ui32
      %13 = om.constant 1 : ui32
      %14 = om.constant 0 : ui32
      %15 = om.constant 1 : ui32
      %16 = om.constant 0 : ui32
      %17 = om.constant 1 : ui32
      %18 = om.constant 1 : ui32
      %19 = om.object @MemoryConfModules(%10, %11, %12, %13, %14, %15, %16, %17, %18) : (!om.str, ui64, ui32, ui32, ui32, ui32, ui32, ui32, ui32) -> !om.class.type<@MemoryConfModules>
      om.class.field @m1, %19 : !om.class.type<@MemoryConfModules>
 }

BlackBox metadata

    om.class @SitestBlackBoxModules(%moduleName: !om.str) {
      om.class.field @moduleName, %moduleName : !om.str
    }
    om.class @SitestBlackBoxMetadata_dut_blackboxes.json() {
      %0 = om.constant #om.str<"DUTBlackbox1"> : !om.str
      %1 = om.object @SitestBlackBoxModules(%0) : (!om.str) -> !om.class.type<@SitestBlackBoxModules>
      om.class.field @m1, %1 : !om.class.type<@SitestBlackBoxModules>
      %2 = om.constant #om.str<"DUTBlackbox2"> : !om.str
      %3 = om.object @SitestBlackBoxModules(%2) : (!om.str) -> !om.class.type<@SitestBlackBoxModules>
      om.class.field @m2, %3 : !om.class.type<@SitestBlackBoxModules>
    }
    om.class @SitestBlackBoxMetadata_test_blackboxes.json() {
      %0 = om.constant #om.str<"TestBlackbox"> : !om.str
      %1 = om.object @SitestBlackBoxModules(%0) : (!om.str) -> !om.class.type<@SitestBlackBoxModules>
      om.class.field @m0, %1 : !om.class.type<@SitestBlackBoxModules>
    }

@prithayan prithayan requested a review from mikeurbach May 17, 2023 22:39
uenoku
uenoku reviewed May 22, 2023
View reviewed changes
uenoku
uenoku reviewed May 22, 2023
View reviewed changes
This was referenced May 23, 2023
Merged
Merged
prithayan added a commit that referenced this pull request May 24, 2023
@prithayan
[FIRRTL] Make emit-metadata a mlir::ModuleOp pass (#5247)
cc5c6c8 
Make the `CreateSiFiveMetadata` a `mlir::ModuleOp` pass.
This is required to generate `om` Dialect operations (#5224).
The `om::ClassOp` must have `mlir::ModuleOp` as the parent. This PR updates the
Pass infrastructure and also the lit tests.
@prithayan prithayan force-pushed the dev/pbarua/metadataOMIR branch from d5ec402 to efe851e Compare May 24, 2023 17:22
@prithayan prithayan marked this pull request as ready for review May 24, 2023 20:29
@prithayan
Copy link
Contributor Author

@mikeurbach , @uenoku , this PR is ready for review. Please take a look.

uenoku
uenoku reviewed May 25, 2023
View reviewed changes
lib/Conversion/ExportVerilog/ExportVerilog.cpp
Comment on lines +5381 to +5736
.Case<om::ClassOp>([&](auto op) {
symbolCache.addDefinition(op.getSymNameAttr(), op);
Copy link
Member
@uenoku uenoku May 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense.

uenoku
uenoku reviewed 10000 May 25, 2023
View reviewed changes
mikeurbach
mikeurbach reviewed May 25, 2023
View reviewed changes
Copy link
Contributor
@mikeurbach mikeurbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The output looks good to me in the tests, just some minor comments.

uenoku
uenoku reviewed May 25, 2023
View reviewed changes
uenoku
uenoku reviewed May 26, 2023
View reviewed changes
@prithayan prithayan force-pushed the dev/pbarua/metadataOMIR branch from 7dc92e4 to f039d37 Compare May 26, 2023 15:53
@prithayan prithayan force-pushed the dev/pbarua/metadataOMIR branch from f039d37 to 3a76056 Compare May 30, 2023 15:19
@prithayan prithayan force-pushed the dev/pbarua/metadataOMIR branch from 3a76056 to c6002c1 Compare June 2, 2023 18:02
@prithayan
Copy link
Contributor Author

Thanks for the review, @mikeurbach and @uenoku.
Added the helper builds, and factored out the om ir creation into a separate class to keep the OM creation out of the main methods.

@prithayan prithayan merged commit 1b426fe into main Jun 2, 2023
seldridge
seldridge reviewed Jul 28, 2023
View reviewed changes
test/Dialect/FIRRTL/SFCTests/directories.fir
Comment on lines +137 to +172
; MLIR_OUT: om.class @SitestBlackBoxModulesSchema(%moduleName: !om.sym_ref) {
; MLIR_OUT: om.class.field @moduleName, %moduleName : !om.sym_ref
; MLIR_OUT: }

; MLIR_OUT: om.class @SitestBlackBoxMetadata() {
; MLIR_OUT: %0 = om.constant #om.sym_ref<@Foo_BlackBox> : !om.sym_ref
; MLIR_OUT: %1 = om.object @SitestBlackBoxModulesSchema(%0) : (!om.sym_ref) -> !om.class.type<@SitestBlackBoxModulesSchema>
; MLIR_OUT: om.class.field @exterMod_Foo_BlackBox, %1 : !om.class.type<@SitestBlackBoxModulesSchema>
; MLIR_OUT: %2 = om.constant #om.sym_ref<@Bar_BlackBox> : !om.sym_ref
; MLIR_OUT: %3 = om.object @SitestBlackBoxModulesSchema(%2) : (!om.sym_ref) -> !om.class.type<@SitestBlackBoxModulesSchema>
; MLIR_OUT: om.class.field @exterMod_Bar_BlackBox, %3 : !om.class.type<@SitestBlackBoxModulesSchema>
; MLIR_OUT: %4 = om.constant #om.sym_ref<@Baz_BlackBox> : !om.sym_ref
; MLIR_OUT: %5 = om.object @SitestBlackBoxModulesSchema(%4) : (!om.sym_ref) -> !om.class.type<@SitestBlackBoxModulesSchema>
; MLIR_OUT: om.class.field @exterMod_Baz_BlackBox, %5 : !om.class.type<@SitestBlackBoxModulesSchema>
; MLIR_OUT: }

; MLIR_OUT: om.class @MemorySchema(%name: !om.sym_ref, %depth: ui64, %width: ui32, %maskBits: ui32, %readPorts: ui32, %writePorts: ui32, %readwritePorts: ui32, %writeLatency: ui32, %readLatency: ui32) {
; MLIR_OUT: om.class.field @name, %name : !om.sym_ref
; MLIR_OUT: om.class.field @depth, %depth : ui64
; MLIR_OUT: om.class.field @width, %width : ui32
; MLIR_OUT: om.class.field @maskBits, %maskBits : ui32
; MLIR_OUT: om.class.field @readPorts, %readPorts : ui32
; MLIR_OUT: om.class.field @writePorts, %writePorts : ui32
; MLIR_OUT: om.class.field @readwritePorts, %readwritePorts : ui32
; MLIR_OUT: om.class.field @writeLatency, %writeLatency : ui32
; MLIR_OUT: om.class.field @readLatency, %readLatency : ui32
; MLIR_OUT: }
; MLIR_OUT: om.class @MemoryMetadata() {
; MLIR_OUT: om.constant #om.sym_ref<@foo_m_ext> : !om.sym_ref
; MLIR_OUT: om.object @MemorySchema
; MLIR_OUT: om.constant #om.sym_ref<@bar_m_ext> : !om.sym_ref
; MLIR_OUT: om.object @MemorySchema
; MLIR_OUT: om.constant #om.sym_ref<@baz_m_ext> : !om.sym_ref
; MLIR_OUT: om.object @MemorySchema
; MLIR_OUT: }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very late review! Can you move this to a separate test?

This test is mega-focused on testing directory output directory behavior and I was surprised to see om.class lowering added to it. 😅

@prithayan prithayan deleted the dev/pbarua/metadataOMIR branch August 11, 2023 05:19
@seldridge seldridge mentioned this pull request Nov 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seldridge seldridge seldridge left review comments

@mikeurbach mikeurbach mikeurbach left review comments

@uenoku uenoku uenoku left review comments

@dtzSiFive dtzSiFive Awaiting requested review from dtzSiFive

@darthscsi darthscsi Awaiting requested review from darthscsi darthscsi is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@prithayan @seldridge @mikeurbach @uenoku
0