terapines
/
circt
mirror of https://github.com/llvm/circt.git
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[FIRRTL] Initial support for classes and objects in Dedup. (#6582) 

This adds initial support for Dedup to handle deduping classes and
objects. For the most part, this amounts to ensuring core
functionality is implemented in terms of the FModuleLike and
FInstanceLike interfaces, which classes and objects implement,
respectively.

There are a couple places where some specific logic is added for
objects, similar to the specific logic that was already there for
instance ops.

Finally, a small change related to paths is needed in
LowerClasses. There is some logic there that confirms a hierarchical
path's root module is contained within the path's owning module. In
the case of deduping hierarchical paths, an extra layer of hierarchy
is added to hierpath ops, which breaks this logic. A new condition is
added, and if the owning module is anywhere in the hierarchical path,
it is used as the root module.

An end-to-end firtool test is added to demonstrate that classes and
objects dedup, and the final paths are valid in both the local and
hierarchical cases.

This initial support is capable of generating valid paths, but does
not actually confirm that paths which dedup point to entities that
dedup. Comments have been left, and a ticket opened to address this:
https://github.com/llvm/circt/issues/6583.

This initial support does not handle references to objects in class
ports, in the case the objects' classes are deduped. A ticket has also
been opened to address this:
https://github.com/llvm/circt/issues/6603
This commit is contained in:
Mike Urbach 2024-01-24 10:42:21 -07:00 committed by GitHub
parent cb19b5458d
commit 19d522f1e5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 194 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
lib/Dialect/FIRRTL/Transforms/Dedup.cpp
View File

@ -84,6 +84,7 @@ struct StructuralHasherSharedConstants {
moduleNameAttr = StringAttr::get(context, "moduleName");
innerSymAttr = StringAttr::get(context, "inner_sym");
portSymsAttr = StringAttr::get(context, "portSyms");
portNamesAttr = StringAttr::get(context, "portNames");
nonessentialAttributes.insert(StringAttr::get(context, "annotations"));
nonessentialAttributes.insert(StringAttr::get(context, "name"));
nonessentialAttributes.insert(StringAttr::get(context, "portAnnotations"));
@ -104,6 +105,9 @@ struct StructuralHasherSharedConstants {
// This is a cached "portSyms" string attr.
StringAttr portSymsAttr;
// This is a cached "portNames" string attr.
StringAttr portNamesAttr;
// This is a set of every attribute we should ignore.
DenseSet<Attribute> nonessentialAttributes;
};
@ -159,6 +163,15 @@ private:
void update(OpResult result) {
record(result.getAsOpaquePointer());
// Like instance ops, don't use object ops' result types since they might be
// replaced by dedup. Record the class names and lazily combine their hashes
// using the same mechanism as instances and modules.
if (auto objectOp = dyn_cast<ObjectOp>(result.getOwner())) {
referredModuleNames.push_back(objectOp.getType().getNameAttr().getAttr());
return;
}
update(result.getType());
}
@ -201,8 +214,11 @@ private:
for (auto namedAttr : dict) {
auto name = namedAttr.getName();
auto value = namedAttr.getValue();
// Skip names and annotations.
if (constants.nonessentialAttributes.contains(name))
// Skip names and annotations, except in certain cases.
// Names of ports are load bearing for classes, so we do hash those.
bool isClassPortNames =
isa<ClassLike>(op) && name == constants.portNamesAttr;
if (constants.nonessentialAttributes.contains(name) && !isClassPortNames)
continue;
// Hash the port types.
@ -247,6 +263,11 @@ private:
// Hash the interned pointer.
update(name.getAsOpaquePointer());
// TODO: properly handle DistinctAttr, including its use in paths.
// See https://github.com/llvm/circt/issues/6583.
if (isa<DistinctAttr>(value))
continue;
// If this is an symbol reference, we need to perform name erasure.
if (auto innerRef = dyn_cast<hw::InnerRefAttr>(value))
update(innerRef);
@ -588,6 +609,9 @@ struct Equivalence {
if (failed(check(diag, a, cast<IntegerAttr>(aAttr), b,
cast<IntegerAttr>(bAttr))))
return failure();
} else if (isa<DistinctAttr>(aAttr) && isa<DistinctAttr>(bAttr)) {
// TODO: properly handle DistinctAttr, including its use in paths.
// See https://github.com/llvm/circt/issues/6583
} else if (aAttr != bAttr) {
diag.attachNote(a->getLoc())
<< "first operation has attribute '" << attrName.getValue()
@ -617,21 +641,22 @@ struct Equivalence {
}
// NOLINTNEXTLINE(misc-no-recursion)
LogicalResult check(InFlightDiagnostic &diag, InstanceOp a, InstanceOp b) {
auto aName = a.getModuleNameAttr().getAttr();
auto bName = b.getModuleNameAttr().getAttr();
LogicalResult check(InFlightDiagnostic &diag, FInstanceLike a,
FInstanceLike b) {
auto aName = a.getReferencedModuleNameAttr();
auto bName = b.getReferencedModuleNameAttr();
if (aName == bName)
return success();
// If the modules instantiate are different we will want to know why the
// sub module did not dedupliate. This code recursively checks the child
// module.
auto aModule = a.getReferencedModule(instanceGraph);
auto bModule = b.getReferencedModule(instanceGraph);
auto aModule = instanceGraph.lookup(aName)->getModule();
auto bModule = instanceGraph.lookup(bName)->getModule();
// Create a new error for the submodule.
diag.attachNote(std::nullopt)
<< "in instance " << a.getNameAttr() << " of " << aName
<< ", and instance " << b.getNameAttr() << " of " << bName;
<< "in instance " << a.getInstanceNameAttr() << " of " << aName
<< ", and instance " << b.getInstanceNameAttr() << " of " << bName;
check(diag, aModule, bModule);
return failure();
}
@ -648,8 +673,8 @@ struct Equivalence {
// If its an instance operaiton, perform some checking and possibly
// recurse.
if (auto aInst = dyn_cast<InstanceOp>(a)) {
auto bInst = cast<InstanceOp>(b);
if (auto aInst = dyn_cast<FInstanceLike>(a)) {
auto bInst = cast<FInstanceLike>(b);
if (failed(check(diag, aInst, bInst)))
return failure();
}
@ -940,9 +965,15 @@ private:
auto *toNode = instanceGraph[toModule];
auto toModuleRef = FlatSymbolRefAttr::get(toModule.getModuleNameAttr());
for (auto *oldInstRec : llvm::make_early_inc_range(fromNode->uses())) {
auto inst = ::cast<InstanceOp>(*oldInstRec->getInstance());
inst.setModuleNameAttr(toModuleRef);
inst.setPortNamesAttr(toModule.getPortNamesAttr());
auto inst = oldInstRec->getInstance();
if (auto instOp = dyn_cast<InstanceOp>(*inst)) {
instOp.setModuleNameAttr(toModuleRef);
instOp.setPortNamesAttr(toModule.getPortNamesAttr());
} else if (auto objectOp = dyn_cast<ObjectOp>(*inst)) {
auto classLike = cast<ClassLike>(*toNode->getModule());
ClassType classType = detail::getInstanceTypeForClassLike(classLike);
objectOp.getResult().setType(classType);
}
oldInstRec->getParent()->addInstance(inst, toNode);
oldInstRec->erase();
}
@ -1523,14 +1554,8 @@ class DedupPass : public DedupBase<DedupPass> {
// If module has symbol (name) that must be preserved even if unused,
// skip it. All symbol uses must be supported, which is not true if
// non-private.
if (!module.isPrivate() || !module.canDiscardOnUseEmpty()) {
return success();
}
// Explicitly skip class-like modules. This is presently unreachable
// due to above and current implementation but check anyway as dedup
// code does not handle these or object operations.
if (isa<ClassLike>(*module)) {
if (!module.isPrivate() ||
(!module.canDiscardOnUseEmpty() && !isa<ClassLike>(*module))) {
return success();
}

37
lib/Dialect/FIRRTL/Transforms/LowerClasses.cpp
View File

@ -204,19 +204,16 @@ LogicalResult LowerClassesPass::processPaths(
auto moduleName = target.getModule().getModuleNameAttr();
// Copy the middle part from the annotation's NLA.
// Verify a nonlocal annotation refers to a HierPathOp.
hw::HierPathOp hierPathOp;
if (auto hierName = anno.getMember<FlatSymbolRefAttr>("circt.nonlocal")) {
auto hierPathOp =
hierPathOp =
dyn_cast<hw::HierPathOp>(symbolTable.lookup(hierName.getAttr()));
if (!hierPathOp) {
op->emitError("annotation does not point at a HierPathOp");
error = true;
return false;
}
// Copy the old path, dropping the module name.
auto oldPath = hierPathOp.getNamepath().getValue();
llvm::append_range(path, llvm::reverse(oldPath.drop_back()));
moduleName = cast<hw::InnerRefAttr>(oldPath.front()).getModule();
}
auto [it, inserted] = pathInfoTable.try_emplace(id);
@ -236,6 +233,34 @@ LogicalResult LowerClassesPass::processPaths(
if (!owningModule)
return true;
// Copy the middle part from the annotation's NLA.
if (hierPathOp) {
// Copy the old path, dropping the module name.
auto oldPath = hierPathOp.getNamepath().getValue();
llvm::append_range(path, llvm::reverse(oldPath.drop_back()));
// Set the moduleName based on the hierarchical path. If the
// owningModule is in the hierarichal path, set the moduleName to the
// owning module. Otherwise use the top of the hierarchical path.
bool pathContainsOwningModule =
llvm::any_of(oldPath, [&](auto pathFragment) {
return llvm::TypeSwitch<Attribute, bool>(pathFragment)
.Case([&](hw::InnerRefAttr innerRef) {
return innerRef.getModule() ==
owningModule.getModuleNameAttr();
})
.Case([&](FlatSymbolRefAttr symRef) {
return symRef.getAttr() == owningModule.getModuleNameAttr();
})
.Default([](auto attr) { return false; });
});
if (pathContainsOwningModule) {
moduleName = owningModule.getModuleNameAttr();
} else {
moduleName = cast<hw::InnerRefAttr>(oldPath.front()).getModule();
}
}
// Copy the leading part of the hierarchical path from the owning module
// to the start of the annotation's NLA.
auto *node = instanceGraph.lookup(moduleName);

116
test/firtool/classes-dedupe.fir Normal file
View File

@ -0,0 +1,116 @@
; RUN: firtool %s -ir-verilog | FileCheck %s
FIRRTL version 3.3.0
circuit Test : %[[
{
"class": "firrtl.transforms.MustDeduplicateAnnotation",
"modules": ["~Test|CPU_1", "~Test|CPU_2"]
}
]]
; CHECK: hw.hierpath private [[NLA1:@.+]] [@Test::@sym, @CPU_1::[[SYM1:@.+]]]
; CHECK: hw.hierpath private [[NLA2:@.+]] [@Test::@sym, @CPU_1::[[SYM2:@.+]], @Fetch_1::[[SYM3:@.+]]]
module Test :
input in : UInt<1>
output out_1 : UInt<1>
output out_2 : UInt<1>
output om_out_1 : AnyRef
output om_out_2 : AnyRef
inst cpu_1 of CPU_1
inst cpu_2 of CPU_2
connect cpu_1.in, in
connect cpu_2.in, in
connect out_1, cpu_1.out
connect out_2, cpu_2.out
propassign om_out_1, cpu_1.om_out
propassign om_out_2, cpu_2.om_out
; CHECK-LABEL: hw.module private @CPU_1
; CHECK-SAME: out out : i1 {hw.exportPort = #hw<innerSym[[SYM1]]>}
module CPU_1 :
input in : UInt<1>
output out : UInt<1>
output om_out : AnyRef
object om of OM_1
propassign om_out, om
; CHECK: hw.instance "fetch_1" sym [[SYM2]]
inst fetch_1 of Fetch_1
inst fetch_2 of Fetch_1
connect fetch_1.in, in
connect fetch_2.in, in
connect out, fetch_1.out
; CHECK-NOT: CPU_2
module CPU_2 :
input in : UInt<1>
output out : UInt<1>
output om_out : AnyRef
object om of OM_2
propassign om_out, om
inst fetch_1 of Fetch_2
inst fetch_2 of Fetch_2
connect fetch_1.in, in
connect fetch_2.in, in
connect out, fetch_1.out
module Fetch_1 :
input in : UInt<1>
output out : UInt<1>
; CHECK: %foo = sv.wire sym [[SYM3]]
wire foo : UInt<1>
connect foo, in
connect out, foo
; CHECK-NOT: Fetch_2
module Fetch_2 :
input in : UInt<1>
output out : UInt<1>
wire foo : UInt<1>
connect foo, in
connect out, foo
class Foo_1 :
output out_foo : Integer
propassign out_foo, Integer(1)
class Foo_2 :
output out_bar : Integer
propassign out_bar, Integer(1)
; CHECK-LABEL: om.class @OM_1(%basepath: !om.basepath)
class OM_1 :
output out_1 : Path
output out_2 : Path
output out_foo_1 : Inst<Foo_1>
output out_foo_2 : Inst<Foo_2>
object foo_1 of Foo_1
propassign out_foo_1, foo_1
object foo_2 of Foo_2
propassign out_foo_2, foo_2
; CHECK: om.path_create reference %basepath [[NLA1]]
propassign out_1, path("OMReferenceTarget:~Test|CPU_1>out")
; CHECK: om.path_create reference %basepath [[NLA2]]
propassign out_2, path("OMReferenceTarget:~Test|CPU_1/fetch_1:Fetch_1>foo")
; CHECK-NOT: OM_2
class OM_2 :
output out_1 : Path
output out_2 : Path
output out_foo_1 : Inst<Foo_1>
output out_foo_2 : Inst<Foo_2>
object foo_1 of Foo_1
propassign out_foo_1, foo_1
object foo_2 of Foo_2
propassign out_foo_2, foo_2
propassign out_1, path("OMReferenceTarget:~Test|CPU_2>out")
propassign out_2, path("OMReferenceTarget:~Test|CPU_2/fetch_1:Fetch_2>foo")