Skip to content

Commit

Permalink
[FIRRTL] Make metadata symbols private
Browse files Browse the repository at this point in the history
For internal symbols created in the `CreateSiFiveMetadata` pass, make
these private and not public.  This caused some downstream errors in
an out-of-tree project which was doing MLIR-level linking and not renaming
duplicate symbols.  While this downstream project will be fixed, it is
also better to not have these symbols public as that prevents them from
being DCE'd.

h/t @uenoku for the proposed fix here.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
  • Loading branch information
seldridge committed Nov 8, 2024
1 parent 8713411 commit fa23397
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
2 changes: 2 additions & 0 deletions lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ struct ObjectModelIR {
mem->getLoc(),
nlaBuilder.getStringAttr(circtNamespace.newName("memNLA")),
nlaBuilder.getArrayAttr(namepath));
nla.setVisibility(SymbolTable::Visibility::Private);
pathRef = createPathRef(preExtractedLeafInstance, nla, builderOM);
} else {
pathRef = createPathRef({}, {}, builderOM);
Expand Down Expand Up @@ -404,6 +405,7 @@ struct ObjectModelIR {
dutMod->getLoc(),
nlaBuilder.getStringAttr(circtNamespace.newName("dutNLA")),
nlaBuilder.getArrayAttr(namepath));
nla.setVisibility(SymbolTable::Visibility::Private);
// Create the path ref op and record it.
pathOpsToDut.emplace_back(createPathRef(leafInst, nla, builder));
}
Expand Down
6 changes: 3 additions & 3 deletions test/Dialect/FIRRTL/emit-metadata.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ firrtl.circuit "Foo" {
}

// CHECK-LABEL: firrtl.circuit "Foo"
// CHECK: hw.hierpath @[[dutPathSym:.+]] [@Foo::@bar]
// CHECK: hw.hierpath private @[[dutPathSym:.+]] [@Foo::@bar]

// CHECK-LABEL: firrtl.module @Foo(
// CHECK-NEXT: firrtl.instance bar
Expand Down Expand Up @@ -322,7 +322,7 @@ firrtl.circuit "Foo" {

// (1) OM Info -----------------------------------------------------------------
// CHECK-LABEL: firrtl.circuit "Foo"
// CHECK: hw.hierpath @[[memPathSym:.+]] [@Bar::@baz, @Baz::@m]
// CHECK: hw.hierpath private @[[memPathSym:.+]] [@Bar::@baz, @Baz::@m]

// CHECK-LABEL: firrtl.module @Baz()
// CHECK-NEXT: firrtl.instance m
Expand Down Expand Up @@ -712,7 +712,7 @@ firrtl.circuit "Foo" {

// (1) OM Info -----------------------------------------------------------------
// CHECK-LABEL: firrtl.circuit "Foo"
// CHECK: hw.hierpath @memNLA [@Bar::@baz, @Baz::@m]
// CHECK: hw.hierpath private @memNLA [@Bar::@baz, @Baz::@m]
//
// There should be only one tracker on the instance.
//
Expand Down

0 comments on commit fa23397

Please sign in to comment.