diff --git a/include/circt/Dialect/Arc/ArcPasses.h b/include/circt/Dialect/Arc/ArcPasses.h index 94782ff3b505..468d5635ee26 100644 --- a/include/circt/Dialect/Arc/ArcPasses.h +++ b/include/circt/Dialect/Arc/ArcPasses.h @@ -33,7 +33,6 @@ std::unique_ptr createDedupPass(); std::unique_ptr createGroupResetsAndEnablesPass(); std::unique_ptr createInferMemoriesPass(const InferMemoriesOptions &options = {}); -std::unique_ptr createInferStatePropertiesPass(); std::unique_ptr createInlineArcsPass(); std::unique_ptr createInlineModulesPass(); std::unique_ptr createIsolateClocksPass(); diff --git a/include/circt/Dialect/Arc/ArcPasses.td b/include/circt/Dialect/Arc/ArcPasses.td index ab0d3171a083..c14f9f3ee58b 100644 --- a/include/circt/Dialect/Arc/ArcPasses.td +++ b/include/circt/Dialect/Arc/ArcPasses.td @@ -114,8 +114,21 @@ def InlineModules : Pass<"arc-inline-modules", "mlir::ModuleOp"> { def InferStateProperties : Pass<"arc-infer-state-properties", "mlir::ModuleOp"> { let summary = "Add resets and enables explicitly to the state operations"; - let constructor = "circt::arc::createInferStatePropertiesPass()"; let dependentDialects = ["circt::hw::HWDialect", "circt::comb::CombDialect"]; + let options = [ + Option<"detectEnables", "enables", "bool", "true", "Infer enable signals">, + Option<"detectResets", "resets", "bool", "true", "Infer reset signals">, + ]; + let statistics = [ + Statistic<"addedEnables", "added-enables", + "Enables added explicitly to a StateOp">, + Statistic<"addedResets", "added-resets", + "Resets added explicitly to a StateOp">, + Statistic<"missedEnables", "missed-enables", + "Detected enables that could not be added explicitly to a StateOp">, + Statistic<"missedResets", "missed-resets", + "Detected resets that could not be added explicitly to a StateOp">, + ]; } def IsolateClocks : Pass<"arc-isolate-clocks", "mlir::ModuleOp"> { diff --git a/lib/Dialect/Arc/Transforms/InferStateProperties.cpp b/lib/Dialect/Arc/Transforms/InferStateProperties.cpp index 8513ad440a9f..0a75a2ec9286 100644 --- a/lib/Dialect/Arc/Transforms/InferStateProperties.cpp +++ b/lib/Dialect/Arc/Transforms/InferStateProperties.cpp @@ -380,25 +380,12 @@ EnableInfo computeEnableInfoFromPattern(OpOperand &output, StateOp stateOp) { namespace { struct InferStatePropertiesPass - : public arc::impl::InferStatePropertiesBase { - InferStatePropertiesPass() = default; - InferStatePropertiesPass(const InferStatePropertiesPass &pass) - : InferStatePropertiesPass() {} + : public impl::InferStatePropertiesBase { + using InferStatePropertiesBase::InferStatePropertiesBase; void runOnOperation() override; void runOnStateOp(arc::StateOp stateOp, arc::DefineOp arc, DenseMap &resetConditionMap); - - Statistic addedEnables{this, "added-enables", - "Enables added explicitly to a StateOp"}; - Statistic addedResets{this, "added-resets", - "Resets added explicitly to a StateOp"}; - Statistic missedEnables{ - this, "missed-enables", - "Detected enables that could not be added explicitly to a StateOp"}; - Statistic missedResets{ - this, "missed-resets", - "Detected resets that could not be added explicitly to a StateOp"}; }; } // namespace @@ -422,56 +409,55 @@ void InferStatePropertiesPass::runOnStateOp( return; auto outputOp = cast(arc.getBodyBlock().getTerminator()); - const unsigned visitedNoChange = -1; - - // Check for reset patterns, we only have to do this once per arc::DefineOp - // and store the result for later arc::StateOps referring to the same arc. - if (!resetConditionMap.count(arc)) { - SmallVector resetInfos; - int numResets = 0; - ; - for (auto &output : outputOp->getOpOperands()) { - auto resetInfo = computeResetInfoFromPattern(output); - resetInfos.push_back(resetInfo); - if (resetInfo) - ++numResets; - } - - // Rewrite the arc::DefineOp if valid - auto result = applyResetTransformation(arc, resetInfos); - if ((succeeded(result) && resetInfos[0])) - resetConditionMap[arc] = resetInfos[0].condition.getArgNumber(); - else - resetConditionMap[arc] = visitedNoChange; + static constexpr unsigned visitedNoChange = -1; + + if (detectResets) { + // Check for reset patterns, we only have to do this once per arc::DefineOp + // and store the result for later arc::StateOps referring to the same arc. + if (!resetConditionMap.count(arc)) { + SmallVector resetInfos; + int numResets = 0; + for (auto &output : outputOp->getOpOperands()) { + auto resetInfo = computeResetInfoFromPattern(output); + resetInfos.push_back(resetInfo); + if (resetInfo) + ++numResets; + } - if (failed(result)) - missedResets += numResets; - } + // Rewrite the arc::DefineOp if valid + auto result = applyResetTransformation(arc, resetInfos); + if ((succeeded(result) && resetInfos[0])) + resetConditionMap[arc] = resetInfos[0].condition.getArgNumber(); + else + resetConditionMap[arc] = visitedNoChange; - // Apply resets to the state operation. - if (resetConditionMap.count(arc) && - resetConditionMap[arc] != visitedNoChange) { - setResetOperandOfStateOp(stateOp, resetConditionMap[arc]); - ++addedResets; - } + if (failed(result)) + missedResets += numResets; + } - // Check for enable patterns. - SmallVector enableInfos; - int numEnables = 0; - for (OpOperand &output : outputOp->getOpOperands()) { - auto enableInfo = computeEnableInfoFromPattern(output, stateOp); - enableInfos.push_back(enableInfo); - if (enableInfo) - ++numEnables; + // Apply resets to the state operation. + if (resetConditionMap.count(arc) && + resetConditionMap[arc] != visitedNoChange) { + setResetOperandOfStateOp(stateOp, resetConditionMap[arc]); + ++addedResets; + } } - // Apply enable patterns. - if (!failed(applyEnableTransformation(arc, stateOp, enableInfos))) - ++addedEnables; - else - missedEnables += numEnables; -} + if (detectEnables) { + // Check for enable patterns. + SmallVector enableInfos; + int numEnables = 0; + for (OpOperand &output : outputOp->getOpOperands()) { + auto enableInfo = computeEnableInfoFromPattern(output, stateOp); + enableInfos.push_back(enableInfo); + if (enableInfo) + ++numEnables; + } -std::unique_ptr arc::createInferStatePropertiesPass() { - return std::make_unique(); + // Apply enable patterns. + if (!failed(applyEnableTransformation(arc, stateOp, enableInfos))) + ++addedEnables; + else + missedEnables += numEnables; + } } diff --git a/tools/arcilator/arcilator.cpp b/tools/arcilator/arcilator.cpp index 560f89a49f54..c5b3ee3414e2 100644 --- a/tools/arcilator/arcilator.cpp +++ b/tools/arcilator/arcilator.cpp @@ -108,6 +108,16 @@ static cl::opt shouldInline("inline", cl::desc("Inline arcs"), static cl::opt shouldDedup("dedup", cl::desc("Deduplicate arcs"), cl::init(true), cl::cat(mainCategory)); +static cl::opt shouldDetectEnables( + "detect-enables", + cl::desc("Infer enable conditions for states to avoid computation"), + cl::init(true), cl::cat(mainCategory)); + +static cl::opt shouldDetectResets( + "detect-resets", + cl::desc("Infer reset conditions for states to avoid computation"), + cl::init(false), cl::cat(mainCategory)); + static cl::opt shouldMakeLUTs("lookup-tables", cl::desc("Optimize arcs into lookup tables"), cl::init(true), @@ -236,6 +246,12 @@ static void populatePipeline(PassManager &pm) { pm.addPass(arc::createSplitLoopsPass()); if (shouldDedup) pm.addPass(arc::createDedupPass()); + { + arc::InferStatePropertiesOptions opts; + opts.detectEnables = shouldDetectEnables; + opts.detectResets = shouldDetectResets; + pm.addPass(arc::createInferStateProperties(opts)); + } pm.addPass(createCSEPass()); pm.addPass(arc::createArcCanonicalizerPass()); if (shouldMakeLUTs) @@ -243,12 +259,6 @@ static void populatePipeline(PassManager &pm) { pm.addPass(createCSEPass()); pm.addPass(arc::createArcCanonicalizerPass()); - // TODO: the following is commented out because the backend does not support - // StateOp resets yet. - // pm.addPass(arc::createInferStatePropertiesPass()); - // InferStateProperties does not remove all ops it bypasses and inserts a lot - // of constant ops that should be uniqued - // pm.addPass(createSimpleCanonicalizerPass()); // Now some arguments may be unused because reset conditions are not passed as // inputs anymore pm.addPass(arc::createRemoveUnusedArcArgumentsPass()); // Because we replace a lot of StateOp inputs with constants in the enable