From 7032ca61b932e029fd3052d61e77a504b86681f3 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Fri, 19 Jan 2024 10:21:34 -0800 Subject: [PATCH] [CheckCombLoops] Refactor comb loop detection pass (#5647) This is a re-write of comb loop detection pass. It simplifies the pass to use DFS over a reaching definitions graph to detect comb loops in the IR. Also adds support for RefTypes and fixes few bugs. Fixes: #4691, #5462, #6587 --- .../FIRRTL/Transforms/CheckCombLoops.cpp | 1069 ++++++++++------- test/Dialect/FIRRTL/check-comb-cycles.mlir | 461 ++++++- 2 files changed, 1047 insertions(+), 483 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/CheckCombLoops.cpp b/lib/Dialect/FIRRTL/Transforms/CheckCombLoops.cpp index 2dd2dab3f7c0..d1462f9e0cec 100644 --- a/lib/Dialect/FIRRTL/Transforms/CheckCombLoops.cpp +++ b/lib/Dialect/FIRRTL/Transforms/CheckCombLoops.cpp @@ -6,448 +6,569 @@ // //===----------------------------------------------------------------------===// // -// This file implements the FIRRTL combinational cycles detection pass. The -// algorithm handles aggregates and sub-index/field/access ops. +// This file implements the FIRRTL combinational cycles detection pass. +// Terminology: +// In the context of the circt that the MLIR represents, a Value is called the +// driver of another Value, if the driver actively sets the the other Value. The +// driver Value is responsible for determining the logic level of the driven +// Value. +// This pass is a dataflow analysis that interprets the operations of the +// circt to build a connectivity graph, which represents the driver +// relationships. Each node in this connectivity graph is a FieldRef, and an +// edge exists from a source node to a desination node if the source drives the +// destination.. +// Algorithm to construct the connectivity graph. // 1. Traverse each module in the Instance Graph bottom up. -// 2. Preprocess step: Gather all the Value which serve as the root for the -// DFS traversal. The input arguments and wire ops and Instance results -// are the roots. -// Then populate the map for Value to all the FieldRefs it can refer to, -// and another map of FieldRef to all the Values that refer to it. -// (A single Value can refer to multiple FieldRefs, if the Value is the -// result of a SubAccess op. Multiple values can refer to the same -// FieldRef, since multiple SubIndex/Field ops with the same fieldIndex -// can exist in the IR). We also maintain an aliasingValuesMap that maps -// each Value to the set of Values that can refer to the same FieldRef. -// 3. Start from DFS traversal from the root. Push the root to the DFS stack. -// 4. Pop a Value from the DFS stack, add all the Values that alias with it -// to the Visiting set. Add all the unvisited children of the Values in the -// alias set to the DFS stack. -// 5. If any child is already present in the Visiting set, then a cycle is -// found. -// +// 2. Preprocess step: Construct the circt connectivity directed graph. +// Perform a dataflow analysis, on the domain of FieldRefs. Interpret each +// operation, to record the values that can potentially drive another value. +// Each node in the graph is a fieldRef. Each edge represents a dataflow from +// the source to the sink. +// 3. Perform a DFS traversal on the graph, to detect combinational loops and +// paths between ports. +// 4. Inline the combinational paths discovered in each module to its instance +// and continue the analysis through the instance graph. //===----------------------------------------------------------------------===// #include "PassDetails.h" #include "circt/Dialect/FIRRTL/FIRRTLInstanceGraph.h" +#include "circt/Dialect/FIRRTL/FIRRTLOps.h" #include "circt/Dialect/FIRRTL/FIRRTLUtils.h" #include "circt/Dialect/FIRRTL/FIRRTLVisitors.h" #include "circt/Dialect/FIRRTL/Passes.h" #include "llvm/ADT/DenseMap.h" -#include "llvm/ADT/DepthFirstIterator.h" +#include "llvm/ADT/EquivalenceClasses.h" #include "llvm/ADT/PostOrderIterator.h" -#include "llvm/ADT/SCCIterator.h" -#include "llvm/ADT/SmallSet.h" -#include #define DEBUG_TYPE "check-comb-loops" using namespace circt; using namespace firrtl; -using SetOfFieldRefs = DenseSet; - -/// A value is in VisitingSet if its subtree is still being traversed. That is, -/// all its children have not yet been visited. If any Value is visited while -/// its still in the `VisitingSet`, that implies a back edge and a cycle. -struct VisitingSet { -private: - /// The stack is maintained to keep track of the cycle, if one is found. This - /// is required for an iterative DFS traversal, its implicitly recorded for a - /// recursive version of this algorithm. Each entry in the stack is a list of - /// aliasing Values, which were visited at the same time. - SmallVector> visitingStack; - /// This map of the Visiting values, is for faster query, to check if a Value - /// is in VisitingSet. It also records the corresponding index into the - /// visitingStack, for faster pop until the Value. - DenseMap valToStackMap; - -public: - void appendEmpty() { visitingStack.push_back({}); } - void appendToEnd(SmallVector &values) { - auto stackSize = visitingStack.size() - 1; - visitingStack.back().append(values.begin(), values.end()); - // Record the stack location where this Value is pushed. - llvm::for_each(values, [&](Value v) { valToStackMap[v] = stackSize; }); - } - bool contains(Value v) { - return valToStackMap.find(v) != valToStackMap.end(); - } - // Pop all the Values which were visited after v. Then invoke f (if present) - // on a popped value for each index. - void popUntilVal(Value v, - const llvm::function_ref f = {}) { - auto valPos = valToStackMap[v]; - while (visitingStack.size() != valPos) { - auto poppedVals = visitingStack.pop_back_val(); - Value poppedVal; - llvm::for_each(poppedVals, [&](Value pv) { - if (!poppedVal) - poppedVal = pv; - valToStackMap.erase(pv); - }); - if (f && poppedVal) - f(poppedVal); - } - } -}; +using DrivenBysMapType = DenseMap>; class DiscoverLoops { + /// Adjacency list representation. + /// Each entry is a pair, the first element is the FieldRef corresponding to + /// the graph vertex. The second element is the list of vertices, that have an + /// edge to this vertex. + /// The directed edges represent a connectivity relation, of a source that + /// drives the sink. + using DrivenByGraphType = + SmallVector>, 64>; + public: - DiscoverLoops(FModuleOp module, InstanceGraph &instanceGraph, - DenseMap &portPaths) - : module(module), instanceGraph(instanceGraph), portPaths(portPaths) {} + DiscoverLoops( + FModuleOp module, InstanceGraph &instanceGraph, + const DenseMap &otherModulePortPaths, + DrivenBysMapType &thisModulePortPaths) + : module(module), instanceGraph(instanceGraph), + modulePortPaths(otherModulePortPaths), portPaths(thisModulePortPaths) {} LogicalResult processModule() { LLVM_DEBUG(llvm::dbgs() << "\n processing module :" << module.getName()); - SmallVector worklist; - // Traverse over ports and ops, to populate the worklist and get the - // FieldRef corresponding to every Value. Also process the InstanceOps and - // get the paths that exist between the ports of the referenced module. - preprocess(worklist); - - llvm::DenseSet visited; - VisitingSet visiting; - SmallVector dfsStack; - SmallVector inputArgFields; - // Record all the children of Value being visited. - SmallVector children; - // If this is an input port field, then record it. This is used to - // discover paths from input to output ports. Only the last input port - // that is visited on the DFS traversal is recorded. - SmallVector inputArgFieldsTemp; - SmallVector aliasingValues; - - // worklist is the list of roots, to begin the traversal from. - for (auto root : worklist) { - dfsStack = {root}; - inputArgFields.clear(); - LLVM_DEBUG(llvm::dbgs() << "\n Starting traversal from root :" - << getFieldName(FieldRef(root, 0)).first); - if (auto inArg = dyn_cast(root)) { - if (module.getPortDirection(inArg.getArgNumber()) == Direction::In) - // This is required, such that paths to output port can be discovered. - // If there is an overlapping path from two input ports to an output - // port, then the already visited nodes must be re-visited to discover - // the comb paths to the output port. - visited.clear(); - } - while (!dfsStack.empty()) { - auto dfsVal = dfsStack.back(); - if (!visiting.contains(dfsVal)) { - unsigned dfsSize = dfsStack.size(); - - LLVM_DEBUG(llvm::dbgs() << "\n Stack pop :" - << getFieldName(FieldRef(dfsVal, 0)).first - << "," << dfsVal;); - - // Visiting set will contain all the values which alias with the - // dfsVal, this is required to detect back edges to aliasing Values. - // That is fieldRefs that can refer to the same memory location. - visiting.appendEmpty(); - children.clear(); - inputArgFieldsTemp.clear(); - // All the Values that refer to the same FieldRef are added to the - // aliasingValues. - aliasingValues = {dfsVal}; - auto aToVIter = aliasingValuesMap.find(dfsVal); - if (aToVIter != aliasingValuesMap.end()) { - aliasingValues.append(aToVIter->getSecond().begin(), - aToVIter->getSecond().end()); - } - // If `dfsVal` is a subfield, then get all the FieldRefs that it - // refers to and then get all the values that alias with it. - forallRefersTo(dfsVal, [&](FieldRef ref) { - // If this subfield refers to instance/mem results(input port), then - // add the output port FieldRefs that exist in the referenced module - // comb paths to the children. - handlePorts(ref, children); - // Get all the values that refer to this FieldRef, and add them to - // the aliasing values. - if (auto arg = dyn_cast(ref.getValue())) - if (module.getPortDirection(arg.getArgNumber()) == Direction::In) - inputArgFieldsTemp.push_back(ref); - - return success(); - }); - if (!inputArgFieldsTemp.empty()) - inputArgFields = std::move(inputArgFieldsTemp); - - visiting.appendToEnd(aliasingValues); - visited.insert(aliasingValues.begin(), aliasingValues.end()); - // Add the Value to `children`, to which a path exists from `dfsVal`. - for (auto dfsFromVal : aliasingValues) { - - for (auto &use : dfsFromVal.getUses()) { - auto childVal = - TypeSwitch(use.getOwner()) - // Registers stop walk for comb loops. - .Case([](auto _) { return Value(); }) - // For non-register declarations, look at data result. - .Case([](auto op) { return op.getDataRaw(); }) - // Handle connect ops specially. - .Case([&](FConnectLike connect) -> Value { - if (use.getOperandNumber() == 1) { - auto dst = connect.getDest(); - if (handleConnects(dst, inputArgFields).succeeded()) - return dst; - } - return {}; - }) - // For everything else (e.g., expressions), if has single - // result use that. - .Default([](auto op) -> Value { - if (op->getNumResults() == 1) - return op->getResult(0); - return {}; - }); - if (childVal && type_isa(childVal.getType())) - children.push_back(childVal); - } - } - for (auto childVal : children) { - // This childVal can be ignored, if - // It is a Register or a subfield of a register. - if (!visited.contains(childVal)) - dfsStack.push_back(childVal); - // If the childVal is a sub, then check if it aliases with any of - // the predecessors (the visiting set). - if (visiting.contains(childVal)) { - // Comb Cycle Detected !! - reportLoopFound(childVal, visiting); - return failure(); - } - } - // child nodes added, continue the DFS - if (dfsSize != dfsStack.size()) - continue; - } - // FieldRef is an SCC root node, pop the visiting stack to remove the - // nodes that are no longer active predecessors, that is their sub-tree - // is already explored. All the Values reachable from `dfsVal` have been - // explored, remove it and its children from the visiting stack. - visiting.popUntilVal(dfsVal); - - auto popped = dfsStack.pop_back_val(); - (void)popped; - LLVM_DEBUG({ - llvm::dbgs() << "\n dfs popped :" - << getFieldName(FieldRef(popped, 0)).first; - dump(); - }); - } - } - - return success(); + constructConnectivityGraph(module); + return dfsTraverse(drivenBy); } - // Preprocess the module ops to get the - // 1. roots for DFS traversal, - // 2. FieldRef corresponding to each Value. - void preprocess(SmallVector &worklist) { - // All the input ports are added to the worklist. - for (BlockArgument arg : module.getArguments()) { - auto argType = type_cast(arg.getType()); - if (type_isa(argType)) + void constructConnectivityGraph(FModuleOp module) { + LLVM_DEBUG(llvm::dbgs() << "\n Module :" << module.getName()); + + // ALl the module output ports must be added as the initial nodes. + for (auto port : module.getArguments()) { + if (module.getPortDirection(port.getArgNumber()) != Direction::Out) continue; - if (module.getPortDirection(arg.getArgNumber()) == Direction::In) - worklist.push_back(arg); - if (!argType.isGround()) - setValRefsTo(arg, FieldRef(arg, 0)); + walkGroundTypes(port.getType().cast(), + [&](uint64_t index, FIRRTLBaseType t, auto isFlip) { + getOrAddNode(FieldRef(port, index)); + }); } - DenseSet memPorts; - - for (auto &op : module.getOps()) { - TypeSwitch(&op) - // Wire is added to the worklist - .Case([&](WireOp wire) { - worklist.push_back(wire.getResult()); - auto ty = type_dyn_cast(wire.getResult().getType()); - if (ty && !ty.isGround()) - setValRefsTo(wire.getResult(), FieldRef(wire.getResult(), 0)); + walk(module, [&](Operation *op) { + llvm::TypeSwitch(op) + .Case([&](auto) {}) + .Case([&](Forceable forceableOp) { + // Any declaration that can be forced. + if (auto node = dyn_cast(op)) + recordDataflow(node.getData(), node.getInput()); + if (!forceableOp.isForceable() || + forceableOp.getDataRef().use_empty()) + return; + auto data = forceableOp.getData(); + auto ref = forceableOp.getDataRef(); + // Record dataflow from data to the probe. + recordDataflow(ref, data); + recordProbe(data, ref); }) - // All sub elements are added to the worklist. - .Case([&](SubfieldOp sub) { - auto res = sub.getResult(); - bool isValid = false; - auto fieldIndex = sub.getAccessedField().getFieldID(); - if (memPorts.contains(sub.getInput())) { - auto memPort = sub.getInput(); - BundleType type = memPort.getType(); - auto enableFieldId = - type.getFieldID((unsigned)ReadPortSubfield::en); - auto dataFieldId = - type.getFieldID((unsigned)ReadPortSubfield::data); - auto addressFieldId = - type.getFieldID((unsigned)ReadPortSubfield::addr); - if (fieldIndex == enableFieldId || fieldIndex == dataFieldId || - fieldIndex == addressFieldId) { - setValRefsTo(memPort, FieldRef(memPort, 0)); - } else - return; - } - SmallVector fields; - forallRefersTo( - sub.getInput(), - [&](FieldRef subBase) { - isValid = true; - fields.push_back(subBase.getSubField(fieldIndex)); - return success(); - }, - false); - if (isValid) { - for (auto f : fields) - setValRefsTo(res, f); - } + .Case([&](MemOp mem) { handleMemory(mem); }) + .Case([&](RefSendOp send) { + recordDataflow(send.getResult(), send.getBase()); + }) + .Case([&](RefDefineOp def) { + // Dataflow from src to dst. + recordDataflow(def.getDest(), def.getSrc()); + if (!def.getDest().getType().getForceable()) + return; + // Dataflow from dst to src, for RWProbe. + probesReferToSameData(def.getSrc(), def.getDest()); }) + .Case( + [&](auto ref) { handleRefForce(ref.getDest(), ref.getSrc()); }) + .Case([&](auto inst) { handleInstanceOp(inst); }) .Case([&](SubindexOp sub) { - auto res = sub.getResult(); - bool isValid = false; - auto index = sub.getAccessedField().getFieldID(); - SmallVector fields; - forallRefersTo( + recordValueRefersToFieldRef( sub.getInput(), - [&](FieldRef subBase) { - isValid = true; - fields.push_back(subBase.getSubField(index)); - return success(); - }, - false); - if (isValid) { - for (auto f : fields) - setValRefsTo(res, f); - } + sub.getInput().getType().get().getFieldID(sub.getIndex()), + sub.getResult()); }) - .Case([&](SubaccessOp sub) { - FVectorType vecType = sub.getInput().getType(); - auto res = sub.getResult(); - bool isValid = false; - SmallVector fields; - forallRefersTo( + .Case([&](SubfieldOp sub) { + recordValueRefersToFieldRef( sub.getInput(), - [&](FieldRef subBase) { - isValid = true; - // The result of a subaccess can refer to multiple storage - // locations corresponding to all the possible indices. - for (size_t index = 0; index < vecType.getNumElements(); - ++index) - fields.push_back(subBase.getSubField( - 1 + index * (hw::FieldIdImpl::getMaxFieldID( - vecType.getElementType()) + - 1))); - return success(); - }, - false); - if (isValid) { - for (auto f : fields) - setValRefsTo(res, f); - } + sub.getInput().getType().get().getFieldID(sub.getFieldIndex()), + sub.getResult()); }) - .Case( - [&](InstanceOp ins) { handleInstanceOp(ins, worklist); }) - .Case([&](MemOp mem) { - if (!(mem.getReadLatency() == 0)) { - return; - } - for (auto memPort : mem.getResults()) { - if (!type_isa(memPort.getType())) - continue; - memPorts.insert(memPort); - } + .Case([&](SubaccessOp sub) { + auto vecType = sub.getInput().getType().get(); + auto input = sub.getInput(); + auto result = sub.getResult(); + // Flatten the subaccess. The result can refer to any of the + // elements. + for (size_t index = 0; index < vecType.getNumElements(); ++index) + recordValueRefersToFieldRef(input, vecType.getFieldID(index), + result); }) - .Default([&](auto) {}); - } + .Case([&](RefSubOp sub) { + size_t fieldID = TypeSwitch( + sub.getInput().getType().getType()) + .Case([&](auto type) { + return type.getFieldID(sub.getIndex()); + }); + recordValueRefersToFieldRef(sub.getInput(), fieldID, + sub.getResult()); + }) + .Case([&](auto op) { + auto type = op.getType(); + auto res = op.getResult(); + auto getFieldId = [&](unsigned index) { + size_t fieldID = + TypeSwitch(type) + .Case( + [&](auto type) { return type.getFieldID(index); }); + return fieldID; + }; + for (auto [index, v] : llvm::enumerate(op.getOperands())) + recordValueRefersToFieldRef(res, getFieldId(index), v); + }) + .Case([&](FConnectLike connect) { + recordDataflow(connect.getDest(), connect.getSrc()); + }) + .Default([&](Operation *op) { + // All other expressions. + if (op->getNumResults() == 1) { + auto res = op->getResult(0); + for (auto src : op->getOperands()) + recordDataflow(res, src); + } + }); + }); } - void handleInstanceOp(InstanceOp ins, SmallVector &worklist) { - for (auto port : ins.getResults()) { - if (auto type = type_dyn_cast(port.getType())) { - worklist.push_back(port); - if (!type.isGround()) - setValRefsTo(port, FieldRef(port, 0)); - } else if (auto type = type_dyn_cast(port.getType())) { - worklist.push_back(port); - } - } + static std::string getName(FieldRef v) { return getFieldName(v).first; }; + + unsigned getOrAddNode(Value v) { + auto iter = valToFieldRefs.find(v); + if (iter == valToFieldRefs.end()) + return getOrAddNode({v, 0}); + return getOrAddNode(*iter->second.begin()); } - void handlePorts(FieldRef ref, SmallVectorImpl &children) { - if (auto inst = dyn_cast_or_null(ref.getDefiningOp())) { - auto res = cast(ref.getValue()); - auto portNum = res.getResultNumber(); - auto refMod = inst.getReferencedModule(instanceGraph); - if (!refMod) - return; - FieldRef modArg(refMod.getArgument(portNum), ref.getFieldID()); - auto pathIter = portPaths.find(modArg); - if (pathIter == portPaths.end()) + // Get the node id if it exists, else add it to the graph. + unsigned getOrAddNode(FieldRef f) { + auto iter = nodes.find(f); + if (iter != nodes.end()) + return iter->second; + // Add the fieldRef to the graph. + auto id = drivenBy.size(); + // The node id can be used to index into the graph. The entry is a pair, + // first element is the corresponding FieldRef, and the second entry is a + // list of adjacent nodes. + drivenBy.push_back({f, {}}); + nodes[f] = id; + return id; + } + + // Construct the connectivity graph, by adding `dst` and `src` as new nodes, + // if not already existing. Then add an edge from `src` to `dst`. + void addDrivenBy(FieldRef dst, FieldRef src) { + auto srcNode = getOrAddNode(src); + auto dstNode = getOrAddNode(dst); + drivenBy[dstNode].second.push_back(srcNode); + } + + // Add `dstVal` as being driven by `srcVal`. + void recordDataflow(Value dstVal, Value srcVal) { + // Ignore connectivity from constants. + if (auto *def = srcVal.getDefiningOp()) + if (def->hasTrait()) return; - for (auto modOutPort : pathIter->second) { - auto outPortNum = - cast(modOutPort.getValue()).getArgNumber(); - if (modOutPort.getFieldID() == 0) { - children.push_back(inst.getResult(outPortNum)); + // Check if srcVal/dstVal is a fieldRef to an aggregate. Then, there may + // exist other values, that refer to the same fieldRef. Add a connectivity + // from all such "aliasing" values. + auto dstIt = valToFieldRefs.find(dstVal); + auto srcIt = valToFieldRefs.find(srcVal); + + // Block the dataflow through registers. + // dstVal refers to a register, If dstVal is not recorded as the fieldref of + // an aggregate, and its either a register, or result of a sub op. + if (dstIt == valToFieldRefs.end()) + if (auto *def = dstVal.getDefiningOp()) + if (isa(def)) + return; + + auto dstValType = getBaseType(dstVal.getType()); + auto srcValType = getBaseType(srcVal.getType()); + + // Handle Properties. + if (!(srcValType && dstValType)) + return addDrivenBy({dstVal, 0}, {srcVal, 0}); + + auto addDef = [&](FieldRef dst, FieldRef src) { + // If the dstVal and srcVal are aggregate types, then record the dataflow + // between each individual ground type. This is equivalent to flattening + // the type to ensure all the contained FieldRefs are also recorded. + if (dstValType && !dstValType.isGround()) + walkGroundTypes(dstValType, [&](uint64_t dstIndex, FIRRTLBaseType t, + bool dstIsFlip) { + // Handle the case when the dst and src are not of the same type. + // For each dst ground type, and for each src ground type. + if (srcValType == dstValType) { + // This is the only case when the flip is valid. Flip is relevant + // only for connect ops, and src and dst of a connect op must be + // type equivalent! + if (dstIsFlip) + std::swap(dst, src); + addDrivenBy(dst.getSubField(dstIndex), src.getSubField(dstIndex)); + } else if (srcValType && !srcValType.isGround()) + walkGroundTypes(srcValType, [&](uint64_t srcIndex, FIRRTLBaseType t, + auto) { + addDrivenBy(dst.getSubField(dstIndex), src.getSubField(srcIndex)); + }); + // Else, the src is ground type. + else + addDrivenBy(dst.getSubField(dstIndex), src); + }); + + addDrivenBy(dst, src); + }; + + // Both the dstVal and srcVal, can refer to multiple FieldRefs, ensure that + // we capture the dataflow between each pair. Occurs when val result of + // subaccess op. + + // Case 1: None of src and dst are fields of an aggregate. But they can be + // aggregate values. + if (dstIt == valToFieldRefs.end() && srcIt == valToFieldRefs.end()) + return addDef({dstVal, 0}, {srcVal, 0}); + // Case 2: Only src is the field of an aggregate. Get all the fields that + // the src refers to. + if (dstIt == valToFieldRefs.end() && srcIt != valToFieldRefs.end()) { + llvm::for_each(srcIt->getSecond(), [&](const FieldRef &srcField) { + addDef(FieldRef(dstVal, 0), srcField); + }); + return; + } + // Case 3: Only dst is the field of an aggregate. Get all the fields that + // the dst refers to. + if (dstIt != valToFieldRefs.end() && srcIt == valToFieldRefs.end()) { + llvm::for_each(dstIt->getSecond(), [&](const FieldRef &dstField) { + addDef(dstField, FieldRef(srcVal, 0)); + }); + return; + } + // Case 4: Both src and dst are the fields of an aggregate. Get all the + // fields that they refers to. + llvm::for_each(srcIt->second, [&](const FieldRef &srcField) { + llvm::for_each(dstIt->second, [&](const FieldRef &dstField) { + addDef(dstField, srcField); + }); + }); + } + + // Record srcVal as driving the original data value that the probe refers to. + void handleRefForce(Value dstProbe, Value srcVal) { + recordDataflow(dstProbe, srcVal); + auto dstNode = getOrAddNode(dstProbe); + // Now add srcVal as driving the data that dstProbe refers to. + auto leader = rwProbeClasses.findLeader(dstNode); + if (leader == rwProbeClasses.member_end()) + return; + auto iter = rwProbeRefersTo.find(*leader); + assert(iter != rwProbeRefersTo.end()); + if (iter->second != dstNode) + drivenBy[iter->second].second.push_back(getOrAddNode(srcVal)); + } + + // Check the referenced module paths and add input ports as the drivers for + // the corresponding output port. The granularity of the connectivity + // relations is per field. + void handleInstanceOp(InstanceOp inst) { + auto refMod = inst.getReferencedModule(instanceGraph); + // TODO: External modules not handled !! + if (!refMod) + return; + auto modulePaths = modulePortPaths.find(refMod); + if (modulePaths == modulePortPaths.end()) + return; + // Note: Handling RWProbes. + // 1. For RWProbes, output ports can be source of dataflow. + // 2. All the RWProbes that refer to the same base value form a strongly + // connected component. Each has a dataflow from the other, including + // itself. + // Steps to add the instance ports to the connectivity graph: + // 1. Find the set of RWProbes that refer to the same base value. + // 2. Add them to the same rwProbeClasses. + // 3. Choose the first RWProbe port from this set as a representative base + // value. And add it as the source driver for every other RWProbe + // port in the set. + // 4. This will ensure we can detect cycles involving different RWProbes to + // the same base value. + for (auto &path : modulePaths->second) { + auto modSinkPortField = path.first; + auto sinkArgNum = + cast(modSinkPortField.getValue()).getArgNumber(); + FieldRef sinkPort(inst.getResult(sinkArgNum), + modSinkPortField.getFieldID()); + auto sinkNode = getOrAddNode(sinkPort); + bool sinkPortIsForceable = false; + if (auto refResultType = + type_dyn_cast(inst.getResult(sinkArgNum).getType())) + sinkPortIsForceable = refResultType.getForceable(); + + DenseSet setOfEquivalentRWProbes; + unsigned minArgNum = sinkArgNum; + unsigned basePortNode = sinkNode; + for (auto &modSrcPortField : path.second) { + auto srcArgNum = + cast(modSrcPortField.getValue()).getArgNumber(); + // Self loop will exist for RWProbes, ignore them. + if (modSrcPortField == modSinkPortField) continue; + + FieldRef srcPort(inst.getResult(srcArgNum), + modSrcPortField.getFieldID()); + bool srcPortIsForceable = false; + if (auto refResultType = + type_dyn_cast(inst.getResult(srcArgNum).getType())) + srcPortIsForceable = refResultType.getForceable(); + // RWProbes can potentially refer to the same base value. Such ports + // have a path from each other, a false loop, detect such cases. + if (sinkPortIsForceable && srcPortIsForceable) { + auto srcNode = getOrAddNode(srcPort); + // If a path is already recorded, continue. + if (rwProbeClasses.findLeader(srcNode) != + rwProbeClasses.member_end() && + rwProbeClasses.findLeader(sinkNode) == + rwProbeClasses.findLeader(srcNode)) + continue; + // Check if sinkPort is a driver of sourcePort. + auto drivenBysToSrcPort = modulePaths->second.find(modSrcPortField); + if (drivenBysToSrcPort != modulePaths->second.end()) + if (llvm::find(drivenBysToSrcPort->second, modSinkPortField) != + drivenBysToSrcPort->second.end()) { + // This case can occur when there are multiple RWProbes on the + // port, which refer to the same base value. So, each of such + // probes are drivers of each other. Hence the false + // loops. Instead of recording this in the drivenByGraph, + // record it separately with the rwProbeClasses. + setOfEquivalentRWProbes.insert(srcNode); + if (minArgNum > srcArgNum) { + // Make one of the RWProbe port the base node. Use the first + // port for deterministic error messages. + minArgNum = srcArgNum; + basePortNode = srcNode; + } + continue; + } } - FieldRef instanceOutPort(inst.getResult(outPortNum), - modOutPort.getFieldID()); - llvm::append_range(children, fieldToVals[instanceOutPort]); + addDrivenBy(sinkPort, srcPort); } - } else if (auto mem = dyn_cast(ref.getDefiningOp())) { - if (mem.getReadLatency() > 0) + if (setOfEquivalentRWProbes.empty()) + continue; + + // Add all the rwprobes to the same class. + for (auto probe : setOfEquivalentRWProbes) + rwProbeClasses.unionSets(probe, sinkNode); + + // Make the first port as the base value. + // Note: this is a port and the actual reference base exists in another + // module. + auto leader = rwProbeClasses.getLeaderValue(sinkNode); + rwProbeRefersTo[leader] = basePortNode; + + setOfEquivalentRWProbes.insert(sinkNode); + // Add the base RWProbe port as a driver to all other RWProbe ports. + for (auto probe : setOfEquivalentRWProbes) + if (probe != basePortNode) + drivenBy[probe].second.push_back(basePortNode); + } + } + + // Record the FieldRef, corresponding to the result of the sub op + // `result = base[index]` + void recordValueRefersToFieldRef(Value base, unsigned fieldID, Value result) { + + // Check if base is itself field of an aggregate. + auto it = valToFieldRefs.find(base); + if (it != valToFieldRefs.end()) { + // Rebase it to the original aggregate. + // Because of subaccess op, each value can refer to multiple FieldRefs. + SmallVector entry; + for (auto &sub : it->second) + entry.emplace_back(sub.getValue(), sub.getFieldID() + fieldID); + // Update the map at the end, to avoid invaliding the iterator. + valToFieldRefs[result].append(entry.begin(), entry.end()); + return; + } + // Break cycles from registers. + if (auto *def = base.getDefiningOp()) { + if (isa(def)) return; - auto memPort = ref.getValue(); - auto type = type_cast(memPort.getType()); - auto enableFieldId = type.getFieldID((unsigned)ReadPortSubfield::en); - auto dataFieldId = type.getFieldID((unsigned)ReadPortSubfield::data); - auto addressFieldId = type.getFieldID((unsigned)ReadPortSubfield::addr); - if (ref.getFieldID() == enableFieldId || - ref.getFieldID() == addressFieldId) { - for (auto dataField : fieldToVals[FieldRef(memPort, dataFieldId)]) - children.push_back(dataField); - } } + valToFieldRefs[result].emplace_back(base, fieldID); } - void reportLoopFound(Value childVal, VisitingSet visiting) { - // TODO: Work harder to provide best information possible to user, - // especially across instances or when we trace through aliasing values. - // We're about to exit, and can afford to do some slower work here. - auto getName = [&](Value v) { - if (isa_and_nonnull( - v.getDefiningOp())) { - assert(!valRefersTo[v].empty()); - // Pick representative of the "alias set", not deterministic. - return getFieldName(*valRefersTo[v].begin()).first; + void handleMemory(MemOp mem) { + if (mem.getReadLatency() > 0) + return; + // Add the enable and address fields as the drivers of the data field. + for (auto memPort : mem.getResults()) + // TODO: Can reftype ports create cycle ? + if (auto type = type_dyn_cast(memPort.getType())) { + auto enableFieldId = type.getFieldID((unsigned)ReadPortSubfield::en); + auto addressFieldId = type.getFieldID((unsigned)ReadPortSubfield::addr); + auto dataFieldId = type.getFieldID((unsigned)ReadPortSubfield::data); + addDrivenBy({memPort, static_cast(dataFieldId)}, + {memPort, static_cast(enableFieldId)}); + addDrivenBy({memPort, static_cast(dataFieldId)}, + {memPort, static_cast(addressFieldId)}); } - return getFieldName(FieldRef(v, 0)).first; + } + + // Perform an iterative DFS traversal of the given graph. Record paths between + // the ports and detect and report any cycles in the graph. + LogicalResult dfsTraverse(const DrivenByGraphType &graph) { + auto numNodes = graph.size(); + SmallVector onStack(numNodes, false); + SmallVector dfsStack; + + auto hasCycle = [&](unsigned rootNode, DenseSet &visited, + bool recordPortPaths = false) { + if (visited.contains(rootNode)) + return success(); + dfsStack.push_back(rootNode); + + while (!dfsStack.empty()) { + auto currentNode = dfsStack.back(); + + if (!visited.contains(currentNode)) { + visited.insert(currentNode); + onStack[currentNode] = true; + LLVM_DEBUG(llvm::dbgs() + << "\n visiting :" + << drivenBy[currentNode].first.getValue().getType() + << drivenBy[currentNode].first.getValue() << "," + << drivenBy[currentNode].first.getFieldID() << "\n" + << getName(drivenBy[currentNode].first)); + + FieldRef currentF = drivenBy[currentNode].first; + if (recordPortPaths && currentNode != rootNode) { + if (isa(currentF.getValue())) + portPaths[drivenBy[rootNode].first].insert(currentF); + // Even if the current node is not a port, there can be RWProbes of + // the current node at the port. + addToPortPathsIfRWProbe(currentNode, + portPaths[drivenBy[rootNode].first]); + } + } else { + onStack[currentNode] = false; + dfsStack.pop_back(); + } + + for (auto neighbor : graph[currentNode].second) { + if (!visited.contains(neighbor)) { + dfsStack.push_back(neighbor); + } else if (onStack[neighbor]) { + // Cycle found !! + SmallVector path; + auto loopNode = neighbor; + // Construct the cyclic path. + do { + SmallVector::iterator it = + llvm::find_if(drivenBy[loopNode].second, + [&](unsigned node) { return onStack[node]; }); + if (it == drivenBy[loopNode].second.end()) + break; + + path.push_back(drivenBy[loopNode].first); + loopNode = *it; + } while (loopNode != neighbor); + + reportLoopFound(path, drivenBy[neighbor].first.getLoc()); + return failure(); + } + } + } + return success(); }; - auto errorDiag = mlir::emitError( - module.getLoc(), "detected combinational cycle in a FIRRTL module"); - SmallVector path; - path.push_back(childVal); - visiting.popUntilVal( - childVal, [&](Value visitingVal) { path.push_back(visitingVal); }); - assert(path.back() == childVal); - path.pop_back(); + DenseSet visited; + for (unsigned node = 0; node < graph.size(); ++node) { + bool isPort = false; + if (auto arg = dyn_cast(drivenBy[node].first.getValue())) + if (module.getPortDirection(arg.getArgNumber()) == Direction::Out) { + // For output ports, reset the visited. Required to revisit the entire + // graph, to discover all the paths that exist from any input port. + visited.clear(); + isPort = true; + } + if (hasCycle(node, visited, isPort).failed()) + return failure(); + } + return success(); + } + + void reportLoopFound(SmallVectorImpl &path, Location loc) { + auto errorDiag = mlir::emitError( + module.getLoc(), "detected combinational cycle in a FIRRTL module"); // Find a value we can name - auto *it = - llvm::find_if(path, [&](Value v) { return !getName(v).empty(); }); + std::string firstName; + FieldRef *it = llvm::find_if(path, [&](FieldRef v) { + firstName = getName(v); + return !firstName.empty(); + }); if (it == path.end()) { errorDiag.append(", but unable to find names for any involved values."); - errorDiag.attachNote(childVal.getLoc()) << "cycle detected here"; + errorDiag.attachNote(loc) << "cycle detected here"; return; } + // Begin the path from the "smallest string". + for (circt::FieldRef *findStartIt = it; findStartIt != path.end(); + ++findStartIt) { + auto n = getName(*findStartIt); + if (!n.empty() && n < firstName) { + firstName = n; + it = findStartIt; + } + } errorDiag.append(", sample path: "); bool lastWasDots = false; errorDiag << module.getName() << ".{" << getName(*it); - for (auto v : - llvm::concat(llvm::make_range(std::next(it), path.end()), - llvm::make_range(path.begin(), std::next(it)))) { + for (auto v : llvm::concat( + llvm::make_range(std::next(it), path.end()), + llvm::make_range(path.begin(), std::next(it)))) { auto name = getName(v); if (!name.empty()) { errorDiag << " <- " << name; @@ -461,110 +582,136 @@ class DiscoverLoops { errorDiag << "}"; } - LogicalResult handleConnects(Value dst, - SmallVector &inputArgFields) { + void dumpMap() { + LLVM_DEBUG({ + llvm::dbgs() << "\n Connectivity Graph ==>"; + for (const auto &[index, i] : llvm::enumerate(drivenBy)) { + llvm::dbgs() << "\n ===>dst:" << getName(i.first) + << "::" << i.first.getValue(); + for (auto s : i.second) + llvm::dbgs() << "<---" << getName(drivenBy[s].first) + << "::" << drivenBy[s].first.getValue(); + } - bool onlyFieldZero = true; - auto pathsToOutPort = [&](FieldRef dstFieldRef) { - if (dstFieldRef.getFieldID() != 0) - onlyFieldZero = false; - if (!isa(dstFieldRef.getValue())) { - return failure(); + llvm::dbgs() << "\n Value to FieldRef :"; + for (const auto &fields : valToFieldRefs) { + llvm::dbgs() << "\n Val:" << fields.first; + for (auto f : fields.second) + llvm::dbgs() << ", FieldRef:" << getName(f) << "::" << f.getFieldID(); } - onlyFieldZero = false; - for (auto inArg : inputArgFields) { - portPaths[inArg].insert(dstFieldRef); + llvm::dbgs() << "\n Port paths:"; + for (const auto &p : portPaths) { + llvm::dbgs() << "\n Output :" << getName(p.first); + for (auto i : p.second) + llvm::dbgs() << "\n Input :" << getName(i); } - return success(); - }; - forallRefersTo(dst, pathsToOutPort); - - if (onlyFieldZero) { - if (isa( - dst.getDefiningOp())) - return failure(); - } - return success(); + llvm::dbgs() << "\n rwprobes:"; + for (auto node : rwProbeRefersTo) { + llvm::dbgs() << "\n node:" << getName(drivenBy[node.first].first) + << "=> probe:" << getName(drivenBy[node.second].first); + } + for (auto i = rwProbeClasses.begin(), e = rwProbeClasses.end(); i != e; + ++i) { // Iterate over all of the equivalence sets. + if (!i->isLeader()) + continue; // Ignore non-leader sets. + // Print members in this set. + llvm::interleave(llvm::make_range(rwProbeClasses.member_begin(i), + rwProbeClasses.member_end()), + llvm::dbgs(), "\n"); + llvm::dbgs() << "\n dataflow at leader::" << i->getData() << "\n =>" + << rwProbeRefersTo[i->getData()]; + llvm::dbgs() << "\n Done\n"; // Finish set. + } + }); } - void setValRefsTo(Value val, FieldRef ref) { - assert(val && ref && " Value and Ref cannot be null"); - valRefersTo[val].insert(ref); - auto fToVIter = fieldToVals.find(ref); - if (fToVIter != fieldToVals.end()) { - for (auto aliasingVal : fToVIter->second) { - aliasingValuesMap[val].insert(aliasingVal); - aliasingValuesMap[aliasingVal].insert(val); - } - fToVIter->getSecond().insert(val); - } else - fieldToVals[ref].insert(val); + void recordProbe(Value data, Value ref) { + auto refNode = getOrAddNode(ref); + auto dataNode = getOrAddNode(data); + rwProbeRefersTo[rwProbeClasses.getOrInsertLeaderValue(refNode)] = dataNode; } - void - forallRefersTo(Value val, - const llvm::function_ref f, - bool baseCase = true) { - auto refersToIter = valRefersTo.find(val); - if (refersToIter != valRefersTo.end()) { - for (auto ref : refersToIter->second) - if (f(ref).failed()) - return; - } else if (baseCase) { - FieldRef base(val, 0); - if (f(base).failed()) - return; - } + // Add both the probes to the same equivalence class, to record that they + // refer to the same data value. + void probesReferToSameData(Value probe1, Value probe2) { + auto p1Node = getOrAddNode(probe1); + auto p2Node = getOrAddNode(probe2); + rwProbeClasses.unionSets(p1Node, p2Node); } - void dump() { - for (const auto &valRef : valRefersTo) { - llvm::dbgs() << "\n val :" << valRef.first; - for (auto node : valRef.second) - llvm::dbgs() << "\n Refers to :" << getFieldName(node).first; - } - for (const auto &dtv : fieldToVals) { - llvm::dbgs() << "\n Field :" << getFieldName(dtv.first).first - << " ::" << dtv.first.getValue(); - for (auto val : dtv.second) - llvm::dbgs() << "\n val :" << val; - } - for (const auto &p : portPaths) { - llvm::dbgs() << "\n Output port : " << getFieldName(p.first).first - << " has comb path from :"; - for (const auto &src : p.second) - llvm::dbgs() << "\n Input port : " << getFieldName(src).first; - } + void addToPortPathsIfRWProbe(unsigned srcNode, + DenseSet &inputPortPaths) { + // Check if there exists any RWProbe for the srcNode. + auto baseFieldRef = drivenBy[srcNode].first; + if (auto defOp = dyn_cast_or_null(baseFieldRef.getDefiningOp())) + if (defOp.isForceable() && !defOp.getDataRef().use_empty()) { + // Assumption, the probe must exist in the equivalence classes. + auto rwProbeNode = + rwProbeClasses.getLeaderValue(getOrAddNode(defOp.getDataRef())); + // For all the probes, that are in the same eqv class, i.e., refer to + // the same value. + for (auto probe : + llvm::make_range(rwProbeClasses.member_begin( + rwProbeClasses.findValue(rwProbeNode)), + rwProbeClasses.member_end())) { + auto probeVal = drivenBy[probe].first; + // If the probe is a port, then record the path from the probe to the + // input port. + if (probeVal.getValue().isa()) { + inputPortPaths.insert(probeVal); + } + } + } } +private: FModuleOp module; InstanceGraph &instanceGraph; - /// Map of a Value to all the FieldRefs that it refers to. - DenseMap valRefersTo; - DenseMap> aliasingValuesMap; - - DenseMap> fieldToVals; + /// Map of values to the set of all FieldRefs (same base) that this may be + /// directly derived from through indexing operations. + DenseMap> valToFieldRefs; /// Comb paths that exist between module ports. This is maintained across /// modules. - DenseMap &portPaths; + const DenseMap &modulePortPaths; + /// The comb paths between the ports of this module. This is the final + /// output of this intra-procedural analysis, that is used to construct the + /// inter-procedural dataflow. + DrivenBysMapType &portPaths; + + /// This is an adjacency list representation of the connectivity graph. This + /// can be indexed by the graph node id, and each entry is the list of graph + /// nodes that has an edge to it. Each graph node represents a FieldRef and + /// each edge represents a source that directly drives the sink node. + DrivenByGraphType drivenBy; + /// Map of FieldRef to its corresponding graph node. + DenseMap nodes; + + /// The base value that the RWProbe refers to. Used to add an edge to the base + /// value, when the probe is forced. + DenseMap rwProbeRefersTo; + + /// An eqv class of all the RWProbes that refer to the same base value. + llvm::EquivalenceClasses rwProbeClasses; }; -/// This pass constructs a local graph for each module to detect combinational -/// cycles. To capture the cross-module combinational cycles, this pass inlines -/// the combinational paths between IOs of its subinstances into a subgraph and -/// encodes them in a `combPathsMap`. +/// This pass constructs a local graph for each module to detect +/// combinational cycles. To capture the cross-module combinational cycles, +/// this pass inlines the combinational paths between IOs of its +/// subinstances into a subgraph and encodes them in `modulePortPaths`. class CheckCombLoopsPass : public CheckCombLoopsBase { public: void runOnOperation() override { auto &instanceGraph = getAnalysis(); - DenseMap portPaths; + DenseMap modulePortPaths; + // Traverse modules in a post order to make sure the combinational paths - // between IOs of a module have been detected and recorded in `portPaths` - // before we handle its parent modules. + // between IOs of a module have been detected and recorded in + // `modulePortPaths` before we handle its parent modules. for (auto *igNode : llvm::post_order(&instanceGraph)) { if (auto module = dyn_cast(*igNode->getModule())) { - DiscoverLoops rdf(module, instanceGraph, portPaths); + DiscoverLoops rdf(module, instanceGraph, modulePortPaths, + modulePortPaths[module]); if (rdf.processModule().failed()) { return signalPassFailure(); } diff --git a/test/Dialect/FIRRTL/check-comb-cycles.mlir b/test/Dialect/FIRRTL/check-comb-cycles.mlir index e66af4b88b4c..9ed2f31684a8 100644 --- a/test/Dialect/FIRRTL/check-comb-cycles.mlir +++ b/test/Dialect/FIRRTL/check-comb-cycles.mlir @@ -4,7 +4,9 @@ // CHECK: firrtl.circuit "hasnoloops" firrtl.circuit "hasnoloops" { firrtl.module @thru(in %in1: !firrtl.uint<1>, in %in2: !firrtl.uint<1>, out %out1: !firrtl.uint<1>, out %out2: !firrtl.uint<1>) { - firrtl.connect %out1, %in1 : !firrtl.uint<1>, !firrtl.uint<1> + %a = firrtl.wire : !firrtl.uint<1> + firrtl.connect %out1, %a : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.connect %a, %in1 : !firrtl.uint<1>, !firrtl.uint<1> firrtl.connect %out2, %in2 : !firrtl.uint<1>, !firrtl.uint<1> } firrtl.module @hasnoloops(in %clk: !firrtl.clock, in %a: !firrtl.uint<1>, out %b: !firrtl.uint<1>) { @@ -54,8 +56,8 @@ firrtl.circuit "hasloops" { firrtl.module @hasloops(in %clk: !firrtl.clock, in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>, out %d: !firrtl.uint<1>) { %y = firrtl.wire : !firrtl.uint<1> firrtl.connect %c, %b : !firrtl.uint<1>, !firrtl.uint<1> - %0 = firrtl.and %c, %y : (!firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> - %z = firrtl.node %0 : !firrtl.uint<1> + %t = firrtl.and %c, %y : (!firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> + %z = firrtl.node %t : !firrtl.uint<1> firrtl.connect %y, %z : !firrtl.uint<1>, !firrtl.uint<1> firrtl.connect %d, %z : !firrtl.uint<1>, !firrtl.uint<1> } @@ -66,7 +68,7 @@ firrtl.circuit "hasloops" { // Combinational loop through a combinational memory read port // CHECK-NOT: firrtl.circuit "hasloops" firrtl.circuit "hasloops" { - // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: hasloops.{y <- z <- m.r.data <- m.r.addr <- y}}} + // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: hasloops.{m.r.addr <- y <- z <- m.r.data <- m.r.addr}}} firrtl.module @hasloops(in %clk: !firrtl.clock, in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>, out %d: !firrtl.uint<1>) { %y = firrtl.wire : !firrtl.uint<1> %z = firrtl.wire : !firrtl.uint<1> @@ -95,7 +97,7 @@ firrtl.circuit "hasloops" { firrtl.module @thru(in %in: !firrtl.uint<1>, out %out: !firrtl.uint<1>) { firrtl.connect %out, %in : !firrtl.uint<1>, !firrtl.uint<1> } - // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: hasloops.{y <- z <- inner.out <- inner.in <- y}}} + // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: hasloops.{inner.in <- y <- z <- inner.out <- inner.in}}} firrtl.module @hasloops(in %clk: !firrtl.clock, in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>, out %d: !firrtl.uint<1>) { %y = firrtl.wire : !firrtl.uint<1> %z = firrtl.wire : !firrtl.uint<1> @@ -113,7 +115,7 @@ firrtl.circuit "hasloops" { // Multiple simple loops in one SCC // CHECK-NOT: firrtl.circuit "hasloops" firrtl.circuit "hasloops" { - // expected-error @below {{hasloops.{c <- b <- ... <- a <- ... <- c}}} + // expected-error @below {{hasloops.{b <- ... <- d <- ... <- e <- b}}} firrtl.module @hasloops(in %i: !firrtl.uint<1>, out %o: !firrtl.uint<1>) { %a = firrtl.wire : !firrtl.uint<1> %b = firrtl.wire : !firrtl.uint<1> @@ -136,7 +138,7 @@ firrtl.circuit "hasloops" { // ----- firrtl.circuit "strictConnectAndConnect" { - // expected-error @below {{strictConnectAndConnect.{b <- a <- b}}} + // expected-error @below {{strictConnectAndConnect.{a <- b <- a}}} firrtl.module @strictConnectAndConnect(out %a: !firrtl.uint<11>, out %b: !firrtl.uint<11>) { %w = firrtl.wire : !firrtl.uint<11> firrtl.strictconnect %b, %w : !firrtl.uint<11> @@ -147,11 +149,34 @@ firrtl.circuit "strictConnectAndConnect" { // ----- +firrtl.circuit "outputPortCycle" { + // expected-error @below {{outputPortCycle.{reg[0].a <- w.a <- reg[0].a}}} + firrtl.module @outputPortCycle(out %reg: !firrtl.vector>, 2>) { + %0 = firrtl.subindex %reg[0] : !firrtl.vector>, 2> + %1 = firrtl.subindex %reg[0] : !firrtl.vector>, 2> + %w = firrtl.wire : !firrtl.bundle> + firrtl.connect %w, %0 : !firrtl.bundle>, !firrtl.bundle> + firrtl.connect %1, %w : !firrtl.bundle>, !firrtl.bundle> + } +} + +// ----- + +firrtl.circuit "outputRead" { + firrtl.module @outputRead(out %reg: !firrtl.vector>, 2>) { + %0 = firrtl.subindex %reg[0] : !firrtl.vector>, 2> + %1 = firrtl.subindex %reg[1] : !firrtl.vector>, 2> + firrtl.connect %1, %0 : !firrtl.bundle>, !firrtl.bundle> + } +} + +// ----- + firrtl.circuit "vectorRegInit" { firrtl.module @vectorRegInit(in %clk: !firrtl.clock) { - %reg = firrtl.reg %clk : !firrtl.clock, !firrtl.vector, 2> - %0 = firrtl.subindex %reg[0] : !firrtl.vector, 2> - firrtl.connect %0, %0 : !firrtl.uint<8>, !firrtl.uint<8> + %reg = firrtl.reg %clk : !firrtl.clock, !firrtl.vector>, 2> + %0 = firrtl.subindex %reg[0] : !firrtl.vector>, 2> + firrtl.connect %0, %0 : !firrtl.bundle>, !firrtl.bundle> } } @@ -182,7 +207,7 @@ firrtl.circuit "PortReadWrite" { firrtl.circuit "Foo" { firrtl.module private @Bar(in %a: !firrtl.uint<1>) {} - // expected-error @below {{Foo.{bar.a <- a <- bar.a}}} + // expected-error @below {{Foo.{a <- bar.a <- a}}} firrtl.module @Foo(out %a: !firrtl.uint<1>) { %bar_a = firrtl.instance bar interesting_name @Bar(in a: !firrtl.uint<1>) firrtl.strictconnect %bar_a, %a : !firrtl.uint<1> @@ -192,6 +217,20 @@ firrtl.circuit "Foo" { // ----- +firrtl.circuit "outputPortCycle" { + firrtl.module private @Bar(in %a: !firrtl.bundle, b: uint<4>>) {} + // expected-error @below {{outputPortCycle.{bar.a.a <- port[0].a <- bar.a.a}}} + firrtl.module @outputPortCycle(out %port: !firrtl.vector, b: uint<4>>, 2>) { + %0 = firrtl.subindex %port[0] : !firrtl.vector, b: uint<4>>, 2> + %1 = firrtl.subindex %port[0] : !firrtl.vector, b: uint<4>>, 2> + %w = firrtl.instance bar interesting_name @Bar(in a: !firrtl.bundle, b: uint<4>>) + firrtl.connect %w, %0 : !firrtl.bundle, b: uint<4>>, !firrtl.bundle, b: uint<4>> + firrtl.connect %1, %w : !firrtl.bundle, b: uint<4>>, !firrtl.bundle, b: uint<4>> + } +} + +// ----- + // Node combinational loop through vector subindex // CHECK-NOT: firrtl.circuit "hasloops" firrtl.circuit "hasloops" { @@ -212,7 +251,7 @@ firrtl.circuit "hasloops" { // Node combinational loop through vector subindex // CHECK-NOT: firrtl.circuit "hasloops" firrtl.circuit "hasloops" { - // expected-error @below {{hasloops.{bar_a[0] <- b[0] <- bar_b[0] <- bar_a[0]}}} + // expected-error @below {{hasloops.{b[0] <- bar_b[0] <- bar_a[0] <- b[0]}}} firrtl.module @hasloops(out %b: !firrtl.vector, 2>) { %bar_a = firrtl.wire : !firrtl.vector, 2> %bar_b = firrtl.wire : !firrtl.vector, 2> @@ -233,7 +272,7 @@ firrtl.circuit "hasloops" { // Combinational loop through instance ports // CHECK-NOT: firrtl.circuit "hasloops" firrtl.circuit "hasLoops" { - // expected-error @below {{hasLoops.{bar.a[0] <- b[0] <- bar.b[0] <- bar.a[0]}}} + // expected-error @below {{hasLoops.{b[0] <- bar.b[0] <- bar.a[0] <- b[0]}}} firrtl.module @hasLoops(out %b: !firrtl.vector, 2>) { %bar_a, %bar_b = firrtl.instance bar @Bar(in a: !firrtl.vector, 2>, out b: !firrtl.vector, 2>) %0 = firrtl.subindex %b[0] : !firrtl.vector, 2> @@ -257,7 +296,7 @@ firrtl.circuit "hasLoops" { // ----- firrtl.circuit "bundleWire" { - // expected-error @below {{bundleWire.{w.foo.bar.baz <- out2 <- x <- w.foo.bar.baz}}} + // expected-error @below {{bundleWire.{out2 <- x <- w.foo.bar.baz <- out2}}} firrtl.module @bundleWire(in %arg: !firrtl.bundle>, qux: sint<64>>>, out %out1: !firrtl.uint<1>, out %out2: !firrtl.sint<64>) { @@ -281,6 +320,27 @@ firrtl.circuit "bundleWire" { // ----- +// Combinational loop through instance ports +// CHECK-NOT: firrtl.circuit "hasloops" +firrtl.circuit "hasLoops" { + // expected-error @below {{hasLoops.{b[0] <- bar.b[0] <- bar.a[0] <- b[0]}}} + firrtl.module @hasLoops(out %b: !firrtl.vector, 2>) { + %bar_a, %bar_b = firrtl.instance bar @Bar(in a: !firrtl.vector, 2>, out b: !firrtl.vector, 2>) + %0 = firrtl.subindex %b[0] : !firrtl.vector, 2> + %1 = firrtl.subindex %bar_a[0] : !firrtl.vector, 2> + firrtl.strictconnect %1, %0 : !firrtl.uint<1> + %4 = firrtl.subindex %bar_b[0] : !firrtl.vector, 2> + %5 = firrtl.subindex %b[0] : !firrtl.vector, 2> + firrtl.strictconnect %5, %4 : !firrtl.uint<1> + } + + firrtl.module private @Bar(in %a: !firrtl.vector, 2>, out %b: !firrtl.vector, 2>) { + firrtl.strictconnect %b, %a : !firrtl.vector, 2> + } +} + +// ----- + firrtl.circuit "registerLoop" { // CHECK: firrtl.module @registerLoop(in %clk: !firrtl.clock) firrtl.module @registerLoop(in %clk: !firrtl.clock) { @@ -316,7 +376,7 @@ firrtl.circuit "hasloops" { // Combinational loop through a combinational memory read port // CHECK-NOT: firrtl.circuit "hasloops" firrtl.circuit "hasloops" { - // expected-error @below {{hasloops.{y <- z <- m.r.data <- m.r.en <- y}}} + // expected-error @below {{hasloops.{m.r.data <- m.r.en <- y <- z <- m.r.data}}} firrtl.module @hasloops(in %clk: !firrtl.clock, in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>, out %d: !firrtl.uint<1>) { %y = firrtl.wire : !firrtl.uint<1> %z = firrtl.wire : !firrtl.uint<1> @@ -351,7 +411,7 @@ firrtl.circuit "hasloops" { firrtl.connect %inner_in, %in : !firrtl.uint<1>, !firrtl.uint<1> firrtl.connect %out, %inner_out : !firrtl.uint<1>, !firrtl.uint<1> } - // expected-error @below {{hasloops.{y <- z <- inner2.out <- inner2.in <- y}}} + // expected-error @below {{hasloops.{inner2.in <- y <- z <- inner2.out <- inner2.in}}} firrtl.module @hasloops(in %clk: !firrtl.clock, in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>, out %d: !firrtl.uint<1>) { %y = firrtl.wire : !firrtl.uint<1> %z = firrtl.wire : !firrtl.uint<1> @@ -364,7 +424,6 @@ firrtl.circuit "hasloops" { } } - // ----- // CHECK: firrtl.circuit "hasloops" @@ -467,7 +526,7 @@ firrtl.circuit "revisitOps" { %1 = firrtl.mux(%in1, %in1, %in2) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> firrtl.connect %out, %1 : !firrtl.uint<1>, !firrtl.uint<1> } - // expected-error @below {{revisitOps.{inner2.out <- inner2.in2 <- x <- inner2.out}}} + // expected-error @below {{revisitOps.{inner2.in2 <- x <- inner2.out <- inner2.in2}}} firrtl.module @revisitOps() { %in1, %in2, %out = firrtl.instance inner2 @thru(in in1: !firrtl.uint<1>,in in2: !firrtl.uint<1>, out out: !firrtl.uint<1>) %x = firrtl.wire : !firrtl.uint<1> @@ -489,7 +548,7 @@ firrtl.circuit "revisitOps" { %1 = firrtl.mux(%w, %in1_0, %in2_1) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> firrtl.connect %out_1, %1 : !firrtl.uint<1>, !firrtl.uint<1> } - // expected-error @below {{revisitOps.{inner2.out[1] <- inner2.in2[1] <- x <- inner2.out[1]}}} + // expected-error @below {{revisitOps.{inner2.in2[1] <- x <- inner2.out[1] <- inner2.in2[1]}}} firrtl.module @revisitOps() { %in1, %in2, %out = firrtl.instance inner2 @thru(in in1: !firrtl.vector,2>, in in2: !firrtl.vector,3>, out out: !firrtl.vector,2>) %in1_0 = firrtl.subindex %in1[0] : !firrtl.vector,2> @@ -516,7 +575,7 @@ firrtl.circuit "revisitOps" { %2 = firrtl.mux(%w, %in0_0, %1) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> firrtl.connect %out_1, %2 : !firrtl.uint<1>, !firrtl.uint<1> } - // expected-error @below {{revisitOps.{inner2.out[1] <- inner2.in2[1] <- x <- inner2.out[1]}}} + // expected-error @below {{revisitOps.{inner2.in2[1] <- x <- inner2.out[1] <- inner2.in2[1]}}} firrtl.module @revisitOps() { %in0, %in1, %in2, %out = firrtl.instance inner2 @thru(in in0: !firrtl.vector,2>, in in1: !firrtl.vector,2>, in in2: !firrtl.vector,3>, out out: !firrtl.vector,2>) %in1_0 = firrtl.subindex %in1[0] : !firrtl.vector,2> @@ -577,7 +636,7 @@ firrtl.circuit "CycleStartsUnnammed" { // ----- firrtl.circuit "CycleThroughForceable" { - // expected-error @below {{sample path: CycleThroughForceable.{w <- n <- w}}} + // expected-error @below {{sample path: CycleThroughForceable.{n <- w <- n}}} firrtl.module @CycleThroughForceable() { %w, %w_ref = firrtl.wire forceable : !firrtl.uint<1>, !firrtl.rwprobe> %n, %n_ref = firrtl.node %w forceable : !firrtl.uint<1> @@ -587,6 +646,39 @@ firrtl.circuit "CycleThroughForceable" { // ----- +firrtl.circuit "CycleThroughForceableRef" { + // expected-error @below {{sample path: CycleThroughForceableRef.{n <- n <- w <- ... <- n}}} + firrtl.module @CycleThroughForceableRef() { + %w, %w_ref = firrtl.wire forceable : !firrtl.uint<1>, !firrtl.rwprobe> + %n, %n_ref = firrtl.node %w forceable : !firrtl.uint<1> + %read = firrtl.ref.resolve %n_ref : !firrtl.rwprobe> + firrtl.strictconnect %w, %read : !firrtl.uint<1> + } +} + +// ----- + +firrtl.circuit "Force" { + // expected-error @below {{sample path: Force.{w <- w}}} + firrtl.module @Force(in %clock: !firrtl.clock, in %x: !firrtl.uint<4>) { + %c = firrtl.constant 0 : !firrtl.uint<1> + %w, %w_ref = firrtl.wire forceable : !firrtl.uint<4>, !firrtl.rwprobe> + firrtl.ref.force %clock, %c, %w_ref, %w : !firrtl.clock, !firrtl.uint<1>, !firrtl.uint<4> + } +} + +// ----- + +firrtl.circuit "RefDefineAndCastWidths" { + firrtl.module @RefDefineAndCastWidths(in %x: !firrtl.uint<2>, out %p : !firrtl.probe) { + %w, %ref = firrtl.wire forceable : !firrtl.uint<2>, !firrtl.rwprobe> + %cast = firrtl.ref.cast %ref : (!firrtl.rwprobe>) -> !firrtl.probe + firrtl.ref.define %p, %cast : !firrtl.probe + } +} + +// ----- + firrtl.circuit "Properties" { firrtl.module @Child(in %in: !firrtl.string, out %out: !firrtl.string) { firrtl.propassign %out, %in : !firrtl.string @@ -599,8 +691,202 @@ firrtl.circuit "Properties" { } // ----- -// Incorrect visit of instance op results was resulting in missed cycles. +firrtl.circuit "hasnoloops" { + firrtl.module @thru(in %clk: !firrtl.clock, in %in1: !firrtl.uint<1>, in %in2: !firrtl.uint<1>, out %out1: !firrtl.uint<1>, out %out2: !firrtl.uint<1>) { + %a, %w_ref = firrtl.wire forceable : !firrtl.uint<1>, !firrtl.rwprobe> + firrtl.connect %out1, %a : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.ref.force %clk, %a, %w_ref, %in1 : !firrtl.clock, !firrtl.uint<1>, !firrtl.uint<1> + firrtl.connect %out2, %in2 : !firrtl.uint<1>, !firrtl.uint<1> + } + firrtl.module @hasnoloops(in %clk: !firrtl.clock, in %a: !firrtl.uint<1>, out %b: !firrtl.uint<1>) { + %x = firrtl.wire : !firrtl.uint<1> + %clock, %inner_in1, %inner_in2, %inner_out1, %inner_out2 = firrtl.instance inner @thru(in clk: !firrtl.clock, in in1: !firrtl.uint<1>, in in2: !firrtl.uint<1>, out out1: !firrtl.uint<1>, out out2: !firrtl.uint<1>) + firrtl.connect %inner_in1, %a : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.connect %x, %inner_out1 : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.connect %inner_in2, %x : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.connect %b, %inner_out2 : !firrtl.uint<1>, !firrtl.uint<1> + } +} + +// ----- + +firrtl.circuit "forceLoop" { + firrtl.module @thru1(in %clk: !firrtl.clock, in %in: !firrtl.uint<1>, out %out: !firrtl.rwprobe>) { + %wire, %w_ref = firrtl.wire forceable : !firrtl.uint<1>, !firrtl.rwprobe> + firrtl.ref.define %out, %w_ref : !firrtl.rwprobe> + } + firrtl.module @thru2(in %clk: !firrtl.clock, in %in: !firrtl.uint<1>, out %out: !firrtl.rwprobe>) { + %inner1_clk, %inner1_in, %inner1_out = firrtl.instance inner1 @thru1(in clk: !firrtl.clock, in in: !firrtl.uint<1>, out out: !firrtl.rwprobe>) + firrtl.connect %inner1_clk, %clk : !firrtl.clock, !firrtl.clock + firrtl.connect %inner1_in, %in : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.ref.define %out, %inner1_out : !firrtl.rwprobe> + } + // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: forceLoop.{inner2.in <- y <- z <- ... <- inner2.out <- inner2.in}}} + firrtl.module @forceLoop(in %clk: !firrtl.clock, in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>, out %d: !firrtl.uint<1>) { + %y = firrtl.wire : !firrtl.uint<1> + %z = firrtl.wire : !firrtl.uint<1> + firrtl.connect %c, %b : !firrtl.uint<1>, !firrtl.uint<1> + %inner2_clk, %inner2_in, %w_ref = firrtl.instance inner2 @thru2(in clk: !firrtl.clock, in in: !firrtl.uint<1>, out out: !firrtl.rwprobe>) + firrtl.connect %inner2_clk, %clk : !firrtl.clock, !firrtl.clock + firrtl.connect %inner2_in, %y : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.ref.force %clk, %c, %w_ref, %inner2_in : !firrtl.clock, !firrtl.uint<1>, !firrtl.uint<1> + %inner2_out = firrtl.ref.resolve %w_ref : !firrtl.rwprobe> + firrtl.connect %z, %inner2_out : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.connect %y, %z : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.connect %d, %z : !firrtl.uint<1>, !firrtl.uint<1> + } +} + +// ----- + +// Cycle through RWProbe ports. +firrtl.circuit "RefSink" { + + firrtl.module @RefSource(out %a_ref: !firrtl.probe>, + out %a_rwref: !firrtl.rwprobe>) { + %a, %_a_rwref = firrtl.wire forceable : !firrtl.uint<1>, + !firrtl.rwprobe> + %b, %_b_rwref = firrtl.wire forceable : !firrtl.uint<1>, + !firrtl.rwprobe> + %a_ref_send = firrtl.ref.send %b : !firrtl.uint<1> + firrtl.ref.define %a_ref, %a_ref_send : !firrtl.probe> + firrtl.ref.define %a_rwref, %_a_rwref : !firrtl.rwprobe> + firrtl.strictconnect %b, %a : !firrtl.uint<1> + } + +// expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: RefSink.{b <- ... <- refSource.a_ref <- refSource.a_rwref <- b}}} + firrtl.module @RefSink( + in %clock: !firrtl.clock, + in %enable: !firrtl.uint<1> + ) { + %c0_ui1 = firrtl.constant 0 : !firrtl.uint<1> + %c1_ui1 = firrtl.constant 1 : !firrtl.uint<1> + %refSource_a_ref, %refSource_a_rwref = + firrtl.instance refSource @RefSource( + out a_ref: !firrtl.probe>, + out a_rwref: !firrtl.rwprobe> + ) + %a_ref_resolve = + firrtl.ref.resolve %refSource_a_ref : !firrtl.probe> + %b = firrtl.node %a_ref_resolve : !firrtl.uint<1> + firrtl.ref.force_initial %c1_ui1, %refSource_a_rwref, %b : + !firrtl.uint<1>, !firrtl.uint<1> + } +} + +// ----- +// Cycle through RWProbe ports. +firrtl.circuit "RefSink" { + + firrtl.module @RefSource(out %b_ref: !firrtl.rwprobe>, + out %a_rwref: !firrtl.rwprobe>) { + %a, %_a_rwref = firrtl.wire forceable : !firrtl.uint<1>, + !firrtl.rwprobe> + %b, %_b_rwref = firrtl.wire forceable : !firrtl.uint<1>, + !firrtl.rwprobe> + firrtl.ref.define %b_ref, %_b_rwref : !firrtl.rwprobe> + firrtl.ref.define %a_rwref, %_a_rwref : !firrtl.rwprobe> + firrtl.strictconnect %b, %a : !firrtl.uint<1> + } + +// expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: RefSink.{b <- ... <- refSource.b_ref <- refSource.a_rwref <- b}}} + firrtl.module @RefSink( + in %clock: !firrtl.clock, + in %enable: !firrtl.uint<1> + ) { + %c0_ui1 = firrtl.constant 0 : !firrtl.uint<1> + %c1_ui1 = firrtl.constant 1 : !firrtl.uint<1> + %refSource_b_ref, %refSource_a_rwref = + firrtl.instance refSource @RefSource( + out b_ref: !firrtl.rwprobe>, + out a_rwref: !firrtl.rwprobe> + ) + %a_ref_resolve = + firrtl.ref.resolve %refSource_b_ref : !firrtl.rwprobe> + %b = firrtl.node %a_ref_resolve : !firrtl.uint<1> + firrtl.ref.force_initial %c1_ui1, %refSource_a_rwref, %b : + !firrtl.uint<1>, !firrtl.uint<1> + } +} + +// ----- + +// Loop between two RWProbes referring to the same base value. +firrtl.circuit "RefSink" { + firrtl.module @RefSource(out %a_rwref1: !firrtl.rwprobe>, + out %a_rwref2: !firrtl.rwprobe>) { + %a, %_a_rwref = firrtl.wire forceable : !firrtl.uint<1>, + !firrtl.rwprobe> + firrtl.ref.define %a_rwref1, %a_rwref2 : !firrtl.rwprobe> + firrtl.ref.define %a_rwref2, %_a_rwref : !firrtl.rwprobe> + } + +// expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: RefSink.{b <- ... <- refSource.a_rwref2 <- refSource.a_rwref1 <- b}}} + firrtl.module @RefSink( + in %clock: !firrtl.clock, + in %enable: !firrtl.uint<1> + ) { + %c0_ui1 = firrtl.constant 0 : !firrtl.uint<1> + %c1_ui1 = firrtl.constant 1 : !firrtl.uint<1> + %refSource_a_rwref1, %refSource_a_rwref2 = + firrtl.instance refSource @RefSource( + out a_rwref1: !firrtl.rwprobe>, + out a_rwref2: !firrtl.rwprobe> + ) + %a_ref_resolve = + firrtl.ref.resolve %refSource_a_rwref2 : !firrtl.rwprobe> + %b = firrtl.node %a_ref_resolve : !firrtl.uint<1> + firrtl.ref.force_initial %c1_ui1, %refSource_a_rwref1, %b : + !firrtl.uint<1>, !firrtl.uint<1> + } +} + +// ----- + +// Ensure deterministic error messages, in the presence of multiple probes. +firrtl.circuit "RefSink" { + + firrtl.module @RefSource(out %a_rwref1: !firrtl.rwprobe>, + out %a_rwref2: !firrtl.rwprobe>, + out %a_rwref3: !firrtl.rwprobe>, + out %a_rwref4: !firrtl.rwprobe>, + out %a_rwref5: !firrtl.rwprobe>) { + %a, %_a_rwref = firrtl.wire forceable : !firrtl.uint<1>, + !firrtl.rwprobe> + firrtl.ref.define %a_rwref1, %a_rwref2 : !firrtl.rwprobe> + firrtl.ref.define %a_rwref2, %a_rwref3 : !firrtl.rwprobe> + firrtl.ref.define %a_rwref3, %a_rwref4 : !firrtl.rwprobe> + firrtl.ref.define %a_rwref4, %a_rwref5 : !firrtl.rwprobe> + firrtl.ref.define %a_rwref5, %_a_rwref : !firrtl.rwprobe> + } + +// expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: RefSink.{b <- ... <- refSource.a_rwref2 <- refSource.a_rwref1 <- b}}} + firrtl.module @RefSink( + in %clock: !firrtl.clock, + in %enable: !firrtl.uint<1> + ) { + %c0_ui1 = firrtl.constant 0 : !firrtl.uint<1> + %c1_ui1 = firrtl.constant 1 : !firrtl.uint<1> + %rwref1, %rwref2, %rwref3, %rwref4, %rwref5 = + firrtl.instance refSource @RefSource( + out a_rwref1: !firrtl.rwprobe>, + out a_rwref2: !firrtl.rwprobe>, + out a_rwref3: !firrtl.rwprobe>, + out a_rwref4: !firrtl.rwprobe>, + out a_rwref5: !firrtl.rwprobe> + ) + %a_ref_resolve = + firrtl.ref.resolve %rwref2 : !firrtl.rwprobe> + %b = firrtl.node %a_ref_resolve : !firrtl.uint<1> + firrtl.ref.force_initial %c1_ui1, %rwref5, %b : + !firrtl.uint<1>, !firrtl.uint<1> + } +} + +// ----- + +// Incorrect visit of instance op results was resulting in missed cycles. firrtl.circuit "Bug5442" { firrtl.module private @Bar(in %a: !firrtl.uint<1>, out %b: !firrtl.uint<1>) { firrtl.strictconnect %b, %a : !firrtl.uint<1> @@ -617,3 +903,134 @@ firrtl.circuit "Bug5442" { firrtl.strictconnect %baz_a, %bar_b : !firrtl.uint<1> } } + +// ----- + +firrtl.circuit "References" { + firrtl.module private @Child(in %in: !firrtl.probe>, out %out: !firrtl.probe>) { + firrtl.ref.define %out, %in : !firrtl.probe> + } + // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: References.{child0.in <- child0.out <- child0.in}}} + firrtl.module @References() { + %in, %out = firrtl.instance child0 @Child(in in: !firrtl.probe>, out out: !firrtl.probe>) + firrtl.ref.define %in, %out : !firrtl.probe> + } +} + +// ----- + +firrtl.circuit "RefSubLoop" { + firrtl.module private @Child(in %bundle: !firrtl.bundle, b: uint<1>>, out %p: !firrtl.rwprobe, b: uint<1>>>) { + %n, %n_ref = firrtl.node interesting_name %bundle forceable : !firrtl.bundle, b: uint<1>> + firrtl.ref.define %p, %n_ref : !firrtl.rwprobe, b: uint<1>>> + } + // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: RefSubLoop.{c.bundle.b <- ... <- c.p.b <- c.bundle.b}}} + firrtl.module @RefSubLoop(in %x: !firrtl.uint<1>) { + %c_bundle, %c_p = firrtl.instance c interesting_name @Child(in bundle: !firrtl.bundle, b: uint<1>>, out p: !firrtl.rwprobe, b: uint<1>>>) + %0 = firrtl.ref.sub %c_p[1] : !firrtl.rwprobe, b: uint<1>>> + %1 = firrtl.subfield %c_bundle[b] : !firrtl.bundle, b: uint<1>> + %2 = firrtl.subfield %c_bundle[a] : !firrtl.bundle, b: uint<1>> + firrtl.strictconnect %2, %x : !firrtl.uint<1> + %3 = firrtl.ref.resolve %0 : !firrtl.rwprobe> + firrtl.strictconnect %1, %3 : !firrtl.uint<1> + } +} + +// ----- + +firrtl.circuit "Issue4691" { + firrtl.module private @Send(in %val: !firrtl.uint<2>, out %x: !firrtl.probe>) { + %ref_val = firrtl.ref.send %val : !firrtl.uint<2> + firrtl.ref.define %x, %ref_val : !firrtl.probe> + } + // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: Issue4691.{sub.val <- ... <- sub.x <- sub.val}}} + firrtl.module @Issue4691(out %x : !firrtl.uint<2>) { + %sub_val, %sub_x = firrtl.instance sub @Send(in val: !firrtl.uint<2>, out x: !firrtl.probe>) + %res = firrtl.ref.resolve %sub_x : !firrtl.probe> + firrtl.connect %sub_val, %res : !firrtl.uint<2>, !firrtl.uint<2> + firrtl.strictconnect %x, %sub_val : !firrtl.uint<2> + } +} + +// ----- + +firrtl.circuit "Issue5462" { + // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: Issue5462.{n.a <- w.a <- n.a}}} + firrtl.module @Issue5462() attributes {convention = #firrtl} { + %w = firrtl.wire : !firrtl.bundle> + %n = firrtl.node %w : !firrtl.bundle> + %0 = firrtl.subfield %n[a] : !firrtl.bundle> + %1 = firrtl.subfield %w[a] : !firrtl.bundle> + firrtl.strictconnect %1, %0 : !firrtl.uint<8> + } +} + +// ----- + +firrtl.circuit "Issue5462" { + firrtl.module private @Child(in %bundle: !firrtl.bundle, b: uint<1>>, out %p: !firrtl.bundle, b: uint<1>>) { + %n = firrtl.node %bundle : !firrtl.bundle, b: uint<1>> + %0 = firrtl.subfield %n[a] : !firrtl.bundle, b: uint<1>> + %1 = firrtl.subfield %p[a] : !firrtl.bundle, b: uint<1>> + firrtl.strictconnect %1, %0 : !firrtl.uint<1> + %2 = firrtl.subfield %n[b] : !firrtl.bundle, b: uint<1>> + %3 = firrtl.subfield %p[b] : !firrtl.bundle, b: uint<1>> + firrtl.strictconnect %3, %2 : !firrtl.uint<1> + } + // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: Issue5462.{c.bundle.b <- c.p.b <- c.bundle.b}}} + firrtl.module @Issue5462(in %x: !firrtl.uint<1>) attributes {convention = #firrtl} { + %c_bundle, %c_p = firrtl.instance c @Child(in bundle: !firrtl.bundle, b: uint<1>>, out p: !firrtl.bundle, b: uint<1>>) + %0 = firrtl.subfield %c_p[b] : !firrtl.bundle, b: uint<1>> + %1 = firrtl.subfield %c_bundle[b] : !firrtl.bundle, b: uint<1>> + %2 = firrtl.subfield %c_bundle[a] : !firrtl.bundle, b: uint<1>> + firrtl.strictconnect %2, %x : !firrtl.uint<1> + firrtl.strictconnect %1, %0 : !firrtl.uint<1> + } +} + +// ----- + +firrtl.circuit "Issue5462" { + // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: Issue5462.{a <- n.a <- w.a <- a}}} + firrtl.module @Issue5462(in %in_a: !firrtl.uint<8>, out %out_a: !firrtl.uint<8>, in %c: !firrtl.uint<1>) attributes {convention = #firrtl} { + %w = firrtl.wire : !firrtl.bundle> + %n = firrtl.node %w : !firrtl.bundle> + %0 = firrtl.bundlecreate %in_a : (!firrtl.uint<8>) -> !firrtl.bundle> + %1 = firrtl.mux(%c, %n, %0) : (!firrtl.uint<1>, !firrtl.bundle>, !firrtl.bundle>) -> !firrtl.bundle> + %2 = firrtl.subfield %1[a] : !firrtl.bundle> + %3 = firrtl.subfield %w[a] : !firrtl.bundle> + firrtl.strictconnect %3, %2 : !firrtl.uint<8> + %4 = firrtl.subfield %w[a] : !firrtl.bundle> + firrtl.strictconnect %out_a, %4 : !firrtl.uint<8> + } +} + +// ----- +firrtl.circuit "FlipConnect1" { + // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: FlipConnect1.{w.a <- x.a <- w.a}}} + firrtl.module @FlipConnect1(){ + %w = firrtl.wire : !firrtl.bundle> + %x = firrtl.wire : !firrtl.bundle> + // x.a <= w.a + firrtl.connect %w, %x: !firrtl.bundle>, !firrtl.bundle> + // w.a <= x.a + %w_a = firrtl.subfield %w[a] : !firrtl.bundle> + %x_a = firrtl.subfield %x[a] : !firrtl.bundle> + firrtl.strictconnect %w_a, %x_a : !firrtl.uint<8> + } +} + +// ----- + +firrtl.circuit "FlipConnect2" { + // expected-error @below {{detected combinational cycle in a FIRRTL module, sample path: FlipConnect2.{in.a.a <- out.a.a <- in.a.a}}} + firrtl.module @FlipConnect2() { + %in = firrtl.wire : !firrtl.bundle>> + %out = firrtl.wire : !firrtl.bundle>> + firrtl.connect %out, %in : !firrtl.bundle>>, + !firrtl.bundle>> + %0 = firrtl.subfield %in[a] : !firrtl.bundle>> + %1 = firrtl.subfield %out[a] : !firrtl.bundle>> + firrtl.connect %1, %0 : !firrtl.bundle>,!firrtl.bundle> + } +}