From 6ec6b740ff0690563e35797199234f13b67a954d Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Thu, 5 Dec 2024 13:49:26 -0800 Subject: [PATCH] [InstanceChoice] Add a default override for unspecified options --- include/circt-c/Firtool/Firtool.h | 3 ++ include/circt/Dialect/FIRRTL/Passes.h | 3 +- include/circt/Dialect/FIRRTL/Passes.td | 4 +++ include/circt/Firtool/Firtool.h | 9 ++++++ lib/CAPI/Firtool/Firtool.cpp | 5 +++ .../FIRRTL/Transforms/SpecializeOption.cpp | 32 ++++++++++++------- lib/Firtool/Firtool.cpp | 12 +++++-- test/firtool/instchoice.fir | 4 ++- 8 files changed, 56 insertions(+), 16 deletions(-) diff --git a/include/circt-c/Firtool/Firtool.h b/include/circt-c/Firtool/Firtool.h index b088bc047903..90d37de97c02 100644 --- a/include/circt-c/Firtool/Firtool.h +++ b/include/circt-c/Firtool/Firtool.h @@ -222,6 +222,9 @@ MLIR_CAPI_EXPORTED void circtFirtoolOptionsSetDisableCSEinClasses(CirctFirtoolFirtoolOptions options, bool value); +MLIR_CAPI_EXPORTED void circtFirtoolOptionsSetSelectDefaultInstanceChoice( + CirctFirtoolFirtoolOptions options, bool value); + //===----------------------------------------------------------------------===// // Populate API. //===----------------------------------------------------------------------===// diff --git a/include/circt/Dialect/FIRRTL/Passes.h b/include/circt/Dialect/FIRRTL/Passes.h index 6562510a7ca0..b6bb48266736 100644 --- a/include/circt/Dialect/FIRRTL/Passes.h +++ b/include/circt/Dialect/FIRRTL/Passes.h @@ -200,7 +200,8 @@ std::unique_ptr createProbesToSignalsPass(); std::unique_ptr createSpecializeLayersPass(); -std::unique_ptr createSpecializeOptionPass(); +std::unique_ptr +createSpecializeOptionPass(bool selectDefault = false); std::unique_ptr createCreateCompanionAssume(); diff --git a/include/circt/Dialect/FIRRTL/Passes.td b/include/circt/Dialect/FIRRTL/Passes.td index bc80c936b374..991682046fb4 100644 --- a/include/circt/Dialect/FIRRTL/Passes.td +++ b/include/circt/Dialect/FIRRTL/Passes.td @@ -876,6 +876,10 @@ def SpecializeOption : Pass<"firrtl-specialize-option", "firrtl::CircuitOp"> { }]; let constructor = "circt::firrtl::createSpecializeOptionPass()"; + let options = [ + Option<"selectDefault", "specialize-to-default-if-no-override", "bool", "false", + "Specialize instance choice to default, if no option selected.">, + ]; let statistics = [ Statistic<"numInstances", "num-instances", "Number of instances specialized"> diff --git a/include/circt/Firtool/Firtool.h b/include/circt/Firtool/Firtool.h index 0f59df059356..baed0b88fa67 100644 --- a/include/circt/Firtool/Firtool.h +++ b/include/circt/Firtool/Firtool.h @@ -132,6 +132,9 @@ class FirtoolOptions { bool shouldFixupEICGWrapper() const { return fixupEICGWrapper; } bool shouldAddCompanionAssume() const { return addCompanionAssume; } bool shouldDisableCSEinClasses() const { return disableCSEinClasses; } + bool shouldSelectDefaultInstanceChoice() const { + return selectDefaultInstanceChoice; + } // Setters, used by the CAPI FirtoolOptions &setOutputFilename(StringRef name) { @@ -362,6 +365,11 @@ class FirtoolOptions { return *this; } + FirtoolOptions &setSelectDefaultInstanceChoice(bool value) { + selectDefaultInstanceChoice = value; + return *this; + } + private: std::string outputFilename; bool disableAnnotationsUnknown; @@ -409,6 +417,7 @@ class FirtoolOptions { bool fixupEICGWrapper; bool addCompanionAssume; bool disableCSEinClasses; + bool selectDefaultInstanceChoice; }; void registerFirtoolCLOptions(); diff --git a/lib/CAPI/Firtool/Firtool.cpp b/lib/CAPI/Firtool/Firtool.cpp index 1f319217b4ae..0d96c13bbcde 100644 --- a/lib/CAPI/Firtool/Firtool.cpp +++ b/lib/CAPI/Firtool/Firtool.cpp @@ -317,6 +317,11 @@ void circtFirtoolOptionsSetDisableCSEinClasses( unwrap(options)->setDisableCSEinClasses(value); } +void circtFirtoolOptionsSetSelectDefaultInstanceChoice( + CirctFirtoolFirtoolOptions options, bool value) { + unwrap(options)->setSelectDefaultInstanceChoice(value); +} + //===----------------------------------------------------------------------===// // Populate API. //===----------------------------------------------------------------------===// diff --git a/lib/Dialect/FIRRTL/Transforms/SpecializeOption.cpp b/lib/Dialect/FIRRTL/Transforms/SpecializeOption.cpp index 54aff680b1a2..5996cde38eea 100644 --- a/lib/Dialect/FIRRTL/Transforms/SpecializeOption.cpp +++ b/lib/Dialect/FIRRTL/Transforms/SpecializeOption.cpp @@ -27,6 +27,7 @@ namespace { struct SpecializeOptionPass : public circt::firrtl::impl::SpecializeOptionBase { using SpecializeOptionBase::numInstances; + using SpecializeOptionBase::selectDefault; void runOnOperation() override { auto circuit = getOperation(); @@ -66,20 +67,25 @@ struct SpecializeOptionPass &getContext(), circuit.getOps(), [&](auto module) { module.walk([&](firrtl::InstanceChoiceOp inst) { auto it = selected.find(inst.getOptionNameAttr()); + FlatSymbolRefAttr target; if (it == selected.end()) { - inst.emitError("missing specialization for option ") - << inst.getOptionNameAttr(); - failed = true; - return; - } + if (!selectDefault) { + inst.emitError("missing specialization for option ") + << inst.getOptionNameAttr(); + failed = true; + return; + } + target = inst.getDefaultTargetAttr(); + } else + target = inst.getTargetOrDefaultAttr(it->second); ImplicitLocOpBuilder builder(inst.getLoc(), inst); auto newInst = builder.create( - inst->getResultTypes(), inst.getTargetOrDefaultAttr(it->second), - inst.getNameAttr(), inst.getNameKindAttr(), - inst.getPortDirectionsAttr(), inst.getPortNamesAttr(), - inst.getAnnotationsAttr(), inst.getPortAnnotationsAttr(), - builder.getArrayAttr({}), UnitAttr{}, inst.getInnerSymAttr()); + inst->getResultTypes(), target, inst.getNameAttr(), + inst.getNameKindAttr(), inst.getPortDirectionsAttr(), + inst.getPortNamesAttr(), inst.getAnnotationsAttr(), + inst.getPortAnnotationsAttr(), builder.getArrayAttr({}), + UnitAttr{}, inst.getInnerSymAttr()); inst.replaceAllUsesWith(newInst); inst.erase(); @@ -101,6 +107,8 @@ struct SpecializeOptionPass }; } // namespace -std::unique_ptr firrtl::createSpecializeOptionPass() { - return std::make_unique(); +std::unique_ptr firrtl::createSpecializeOptionPass(bool selectDefault) { + auto pass = std::make_unique(); + pass->selectDefault = selectDefault; + return pass; } diff --git a/lib/Firtool/Firtool.cpp b/lib/Firtool/Firtool.cpp index 958f46e54f30..d08ecbd1ebb0 100644 --- a/lib/Firtool/Firtool.cpp +++ b/lib/Firtool/Firtool.cpp @@ -133,7 +133,8 @@ LogicalResult firtool::populateCHIRRTLToLowFIRRTL(mlir::PassManager &pm, // Must run the specialize instance-choice and layers passes after all // diagnostic passes have run, otherwise it can hide errors. - pm.addNestedPass(firrtl::createSpecializeOptionPass()); + pm.addNestedPass(firrtl::createSpecializeOptionPass( + opt.shouldSelectDefaultInstanceChoice())); pm.addNestedPass(firrtl::createSpecializeLayersPass()); // Run after inference, layer specialization. @@ -733,6 +734,12 @@ struct FirtoolCmdOptions { "add-companion-assume", llvm::cl::desc("Add companion assumes to assertions"), llvm::cl::init(false)}; + + llvm::cl::opt selectDefaultInstanceChoice{ + "specialize-to-default-if-no-override", + llvm::cl::desc( + "Specialize instance choice to default, if no option selected"), + llvm::cl::init(false)}; }; } // namespace @@ -769,7 +776,7 @@ circt::firtool::FirtoolOptions::FirtoolOptions() ckgEnableName("en"), ckgTestEnableName("test_en"), ckgInstName("ckg"), exportModuleHierarchy(false), stripFirDebugInfo(true), stripDebugInfo(false), fixupEICGWrapper(false), addCompanionAssume(false), - disableCSEinClasses(false) { + disableCSEinClasses(false), selectDefaultInstanceChoice(false) { if (!clOptions.isConstructed()) return; outputFilename = clOptions->outputFilename; @@ -818,4 +825,5 @@ circt::firtool::FirtoolOptions::FirtoolOptions() stripDebugInfo = clOptions->stripDebugInfo; fixupEICGWrapper = clOptions->fixupEICGWrapper; addCompanionAssume = clOptions->addCompanionAssume; + selectDefaultInstanceChoice = clOptions->selectDefaultInstanceChoice; } diff --git a/test/firtool/instchoice.fir b/test/firtool/instchoice.fir index 56810db4974a..9fd0259ea097 100644 --- a/test/firtool/instchoice.fir +++ b/test/firtool/instchoice.fir @@ -1,5 +1,6 @@ ; RUN: firtool %s --parse-only --select-instance-choice=Platform=ASIC | FileCheck %s ; RUN: firtool %s --ir-hw --disable-opt --select-instance-choice=Platform=ASIC | FileCheck %s --check-prefix=ASIC +; RUN: firtool %s --ir-hw --disable-opt --specialize-to-default-if-no-override | FileCheck %s --check-prefixes=DEFAULT FIRRTL version 4.1.0 ; CHECK: firrtl.circuit "Foo" attributes {select_inst_choice = ["Platform=ASIC"]} { @@ -28,7 +29,8 @@ circuit Foo: ; CHECK-SAME: @FPGA -> @FPGATarget, ; CHECK-SAME: @ASIC -> @ASICTarget ; CHECK-SAME: } (in clock: !firrtl.clock) - ; ASIC: hw.instance "inst" @ASICTarget(clock: %clock: !seq.clock) -> () + ; ASIC: hw.instance "inst" @ASICTarget + ; DEFAULT: hw.instance "inst" @DefaultTarget instchoice inst of DefaultTarget, Platform : FPGA => FPGATarget ASIC => ASICTarget