Skip to content

Commit

Permalink
[FIRRTL][CheckCombLoops] Support property types (llvm#5383)
Browse files Browse the repository at this point in the history
This change adds support for property types to CheckCombLoops. Right now
property types show up as regular ports on instances and modules, and
are assigned the through the PropAssignOp, which implements the
ConnectLike interface. Module ports, instance ports, and connect like
operations had to be updated to support FIRRTLTypes instead of just
FIRRTLBaseTypes.
  • Loading branch information
youngar authored Jun 13, 2023
1 parent fb7a78b commit 00565a2
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 4 deletions.
2 changes: 1 addition & 1 deletion lib/Dialect/FIRRTL/FIRRTLTypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ bool FIRRTLType::isGround() {
.Case<BundleType, FVectorType, FEnumType, OpenBundleType, OpenVectorType>(
[](Type) { return false; })
// Not ground per spec, but leaf of aggregate.
.Case<RefType>([](Type) { return false; })
.Case<PropertyType, RefType>([](Type) { return false; })
.Default([](Type) {
llvm_unreachable("unknown FIRRTL type");
return false;
Expand Down
8 changes: 5 additions & 3 deletions lib/Dialect/FIRRTL/Transforms/CheckCombLoops.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ class DiscoverLoops {
return op->getResult(0);
return {};
});
if (childVal && childVal.getType().isa<FIRRTLBaseType>())
if (childVal && childVal.getType().isa<FIRRTLType>())
children.push_back(childVal);
}
}
Expand Down Expand Up @@ -248,12 +248,12 @@ class DiscoverLoops {
void preprocess(SmallVector<Value> &worklist) {
// All the input ports are added to the worklist.
for (BlockArgument arg : module.getArguments()) {
auto argType = arg.getType();
auto argType = cast<FIRRTLType>(arg.getType());
if (argType.isa<RefType>())
continue;
if (module.getPortDirection(arg.getArgNumber()) == Direction::In)
worklist.push_back(arg);
if (!argType.cast<FIRRTLBaseType>().isGround())
if (!argType.isGround())
setValRefsTo(arg, FieldRef(arg, 0));
}
DenseSet<Value> memPorts;
Expand Down Expand Up @@ -365,6 +365,8 @@ class DiscoverLoops {
worklist.push_back(port);
if (!type.isGround())
setValRefsTo(port, FieldRef(port, 0));
} else if (auto type = port.getType().dyn_cast<PropertyType>()) {
worklist.push_back(port);
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions test/Dialect/FIRRTL/check-comb-cycles.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -584,3 +584,16 @@ firrtl.circuit "CycleThroughForceable" {
firrtl.strictconnect %w, %n : !firrtl.uint<1>
}
}

// -----

firrtl.circuit "Properties" {
firrtl.module @Child(in %in: !firrtl.string, out %out: !firrtl.string) {
firrtl.propassign %out, %in : !firrtl.string
}
// expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: Properties.{child0.in <- child0.out <- child0.in}}}
firrtl.module @Properties() {
%in, %out = firrtl.instance child0 @Child(in in: !firrtl.string, out out: !firrtl.string)
firrtl.propassign %in, %out : !firrtl.string
}
}

0 comments on commit 00565a2

Please sign in to comment.