From fa23397fb9ddb95f8dcdcfd725131c3fffebcf04 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Fri, 8 Nov 2024 11:32:26 -0500 Subject: [PATCH] [FIRRTL] Make metadata symbols private 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 --- lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp | 2 ++ test/Dialect/FIRRTL/emit-metadata.mlir | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp b/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp index f775b9d26814..932f3196b035 100644 --- a/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp +++ b/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp @@ -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); @@ -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)); } diff --git a/test/Dialect/FIRRTL/emit-metadata.mlir b/test/Dialect/FIRRTL/emit-metadata.mlir index cc9a65c5e31d..ebf5742db92b 100644 --- a/test/Dialect/FIRRTL/emit-metadata.mlir +++ b/test/Dialect/FIRRTL/emit-metadata.mlir @@ -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 @@ -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 @@ -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. //