Skip to content

Commit

Permalink
[LowerClasses] Lower classes that instantiate properties. (#6688)
Browse files Browse the repository at this point in the history
The previous logic would create classes for any FIRRTL modules that
had property ports. However, if some intermediate modules had property
ports, but their parents did not, we would not convert their parents,
which would break pass invariants and lead to failed assertions.

This adds logic to also convert any FIRRTL modules that instantiate
FIRRTL modules that will be converted. This upholds the pass
invariants, and makes it robust in case property ports are used in
part of the hierarchy but not exported from the top-level module of
the circuit.
  • Loading branch information
mikeurbach committed Feb 13, 2024
1 parent 4001ec8 commit 4fadf7b
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
16 changes: 15 additions & 1 deletion lib/Dialect/FIRRTL/Transforms/LowerClasses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,23 @@ bool LowerClassesPass::shouldCreateClass(FModuleLike moduleLike) {
if (moduleLike.isPublic())
return true;

return llvm::any_of(moduleLike.getPorts(), [](PortInfo port) {
// Create a class for modules with property ports.
bool hasClassPorts = llvm::any_of(moduleLike.getPorts(), [](PortInfo port) {
return isa<PropertyType>(port.type);
});

if (hasClassPorts)
return true;

// Create a class for modules that instantiate classes or modules with
// property ports.
for (auto op :
moduleLike.getOperation()->getRegion(0).getOps<FInstanceLike>())
for (auto result : op->getResults())
if (type_isa<PropertyType>(result.getType()))
return true;

return false;
}

// Create an OM Class op from a FIRRTL Class op or Module op with properties.
Expand Down
11 changes: 11 additions & 0 deletions test/Dialect/FIRRTL/lower-classes.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -327,3 +327,14 @@ firrtl.circuit "AnyCast" {
firrtl.propassign %foo, %0 : !firrtl.anyref
}
}

// CHECK-LABEL: firrtl.circuit "ModuleWithPropertySubmodule"
firrtl.circuit "ModuleWithPropertySubmodule" {
firrtl.module private @ModuleWithPropertySubmodule() {
%c0 = firrtl.integer 0
%inst.prop = firrtl.instance inst @SubmoduleWithProperty(in prop: !firrtl.integer)
firrtl.propassign %inst.prop, %c0 : !firrtl.integer
}
firrtl.module private @SubmoduleWithProperty(in %prop: !firrtl.integer) {
}
}

0 comments on commit 4fadf7b

Please sign in to comment.