Skip to content

Commit ad4aa5d

Browse files
rwy7rwy7
committed
Add domains option
1 parent 6bad34e commit ad4aa5d

File tree

7 files changed

+205
-37
lines changed

7 files changed

+205
-37
lines changed

‎include/circt/Dialect/FIRRTL/Passes.h‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,26 @@ enum class CompanionMode {
6969
Drop,
7070
};
7171

72+
/// The mode for the InferDomains pass.
73+
enum class InferDomainsMode {
74+
/// Check domains with inference for private modules (default).
75+
Infer,
76+
/// Check domains without inference.
77+
Check,
78+
/// Check domains with inference for both public and private modules.
79+
InferAll,
80+
};
81+
82+
/// True if the mode indicates we should infer domains on public modules.
83+
constexpr bool shouldInferPublicModules(InferDomainsMode mode) {
84+
return mode == InferDomainsMode::InferAll;
85+
}
86+
87+
/// True if the mode indicates we should infer domains on private modules.
88+
constexpr bool shouldInferPrivateModules(InferDomainsMode mode) {
89+
return mode == InferDomainsMode::Infer || mode == InferDomainsMode::InferAll;
90+
}
91+
7292
#define GEN_PASS_DECL
7393
#include "circt/Dialect/FIRRTL/Passes.h.inc"
7494

‎include/circt/Dialect/FIRRTL/Passes.td‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -921,8 +921,16 @@ def InferDomains : Pass<"firrtl-infer-domains", "firrtl::CircuitOp"> {
921921
clock domains are specified for signals in the design.
922922
}];
923923
let options = [
924-
Option<"inferPublic", "infer-public", "bool", "false",
925-
"Infer domains on public modules.">
924+
Option<"mode", "mode", "InferDomainsMode", "InferDomainsMode::Infer",
925+
"infer, check, infer-all.",
926+
[{
927+
llvm::cl::values(
928+
clEnumValN(InferDomainsMode::Infer, "infer", "Check domains with inference for private modules"),
929+
clEnumValN(InferDomainsMode::Check, "check", "Check domains without inference"),
930+
clEnumValN(InferDomainsMode::InferAll, "infer-all", "Check domains with inference for both public and private modules")
931+
)
932+
}]
933+
>
926934
];
927935
}
928936

‎include/circt/Firtool/Firtool.h‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,34 @@
2323

2424
namespace circt {
2525
namespace firtool {
26+
27+
enum class DomainMode {
28+
/// Disable domain checking.
29+
Disable,
30+
/// Check domains with inference for private modules.
31+
Infer,
32+
/// Check domains without inference.
33+
Check,
34+
/// Check domains with inference for both public and private modules.
35+
InferAll,
36+
};
37+
38+
/// Convert the "domain mode" firtool option to a "firrtl::InferDomainsMode",
39+
/// the configuration for a pass.
40+
constexpr std::optional<firrtl::InferDomainsMode>
41+
toInferDomainsPassMode(DomainMode mode) {
42+
switch (mode) {
43+
case DomainMode::Disable:
44+
return std::nullopt;
45+
case DomainMode::Infer:
46+
return firrtl::InferDomainsMode::Infer;
47+
case DomainMode::Check:
48+
return firrtl::InferDomainsMode::Check;
49+
case DomainMode::InferAll:
50+
return firrtl::InferDomainsMode::InferAll;
51+
}
52+
}
53+
2654
//===----------------------------------------------------------------------===//
2755
// FirtoolOptions
2856
//===----------------------------------------------------------------------===//
@@ -144,7 +172,7 @@ class FirtoolOptions {
144172

145173
bool getEmitAllBindFiles() const { return emitAllBindFiles; }
146174

147-
bool shouldInferDomains() const { return inferDomains; }
175+
DomainMode getDomainMode() const { return domainMode; }
148176

149177
// Setters, used by the CAPI
150178
FirtoolOptions &setOutputFilename(StringRef name) {
@@ -395,8 +423,8 @@ class FirtoolOptions {
395423
return *this;
396424
}
397425

398-
FirtoolOptions &setInferDomains(bool value) {
399-
inferDomains = value;
426+
FirtoolOptions &setdomainMode(DomainMode value) {
427+
domainMode = value;
400428
return *this;
401429
}
402430

@@ -454,7 +482,7 @@ class FirtoolOptions {
454482
bool lintStaticAsserts;
455483
bool lintXmrsInDesign;
456484
bool emitAllBindFiles;
457-
bool inferDomains;
485+
DomainMode domainMode;
458486
};
459487

460488
void registerFirtoolCLOptions();

‎lib/Dialect/FIRRTL/Transforms/InferDomains.cpp‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,16 @@ using InstanceIterator = InstanceGraphNode::UseIterator;
4545
using InstanceRange = llvm::iterator_range<InstanceIterator>;
4646
using PortInsertions = SmallVector<std::pair<unsigned, PortInfo>>;
4747

48+
//====--------------------------------------------------------------------------
49+
// Domain Inference mode helper.
50+
//====--------------------------------------------------------------------------
51+
52+
template <typename T>
53+
static bool shouldInfer(T op, InferDomainsMode mode) {
54+
return op.isPublic() ? shouldInferPublicModules(mode)
55+
: shouldInferPrivateModules(mode);
56+
}
57+
4858
//====--------------------------------------------------------------------------
4959
// Helpers for working with module or instance domain info.
5060
//====--------------------------------------------------------------------------
@@ -1375,12 +1385,12 @@ static LogicalResult inferModuleDomains(GlobalState &globals,
13751385
// InferDomainsPass: Top-level pass implementation.
13761386
//===---------------------------------------------------------------------------
13771387

1378-
static LogicalResult runOnModuleLike(bool inferPublic, GlobalState &globals,
1379-
Operation *op) {
1388+
static LogicalResult runOnModuleLike(InferDomainsMode mode,
1389+
GlobalState &globals, Operation *op) {
13801390
if (auto module = dyn_cast<FModuleOp>(op)) {
1381-
if (module.isPublic() && !inferPublic)
1382-
return checkModuleDomains(globals, module);
1383-
return inferModuleDomains(globals, module);
1391+
if (shouldInfer(module, mode))
1392+
return inferModuleDomains(globals, module);
1393+
return checkModuleDomains(globals, module);
13841394
}
13851395

13861396
if (auto extModule = dyn_cast<FExtModuleOp>(op)) {
@@ -1406,7 +1416,7 @@ void InferDomainsPass::runOnOperation() {
14061416
DenseSet<InstanceGraphNode *> visited;
14071417
for (auto *root : instanceGraph) {
14081418
for (auto *node : llvm::post_order_ext(root, visited)) {
1409-
if (failed(runOnModuleLike(inferPublic, globals, node->getModule()))) {
1419+
if (failed(runOnModuleLike(mode, globals, node->getModule()))) {
14101420
signalPassFailure();
14111421
return;
14121422
}

‎lib/Firtool/Firtool.cpp‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ LogicalResult firtool::populatePreprocessTransforms(mlir::PassManager &pm,
4949
pm.nest<firrtl::CircuitOp>().nest<firrtl::FModuleOp>().addPass(
5050
firrtl::createLowerIntrinsics());
5151

52-
if (opt.shouldInferDomains())
53-
pm.nest<firrtl::CircuitOp>().addPass(firrtl::createInferDomains());
54-
52+
if (auto mode = toInferDomainsPassMode(opt.getDomainMode())) {
53+
pm.nest<firrtl::CircuitOp>().addPass(firrtl::createInferDomains({*mode}));
54+
}
5555
return success();
5656
}
5757

@@ -761,10 +761,19 @@ struct FirtoolCmdOptions {
761761
llvm::cl::desc("Emit bindfiles for private modules"),
762762
llvm::cl::init(false)};
763763

764-
llvm::cl::opt<bool> inferDomains{
765-
"infer-domains",
766-
llvm::cl::desc("Enable domain inference and checking"),
767-
llvm::cl::init(false)};
764+
llvm::cl::opt<firtool::DomainMode> domainMode{
765+
"domain-mode", llvm::cl::desc("Enable domain inference and checking"),
766+
llvm::cl::init(firtool::DomainMode::Disable),
767+
llvm::cl::values(
768+
clEnumValN(firtool::DomainMode::Disable, "disable",
769+
"Disable domain checking"),
770+
clEnumValN(firtool::DomainMode::Infer, "infer",
771+
"Check domains with inference for private modules"),
772+
clEnumValN(firtool::DomainMode::Check, "check",
773+
"Check domains without inference"),
774+
clEnumValN(firtool::DomainMode::InferAll, "infer-all",
775+
"Check domains with inference for both public and private "
776+
"modules"))};
768777

769778
//===----------------------------------------------------------------------===
770779
// Lint options
@@ -817,7 +826,8 @@ circt::firtool::FirtoolOptions::FirtoolOptions()
817826
disableCSEinClasses(false), selectDefaultInstanceChoice(false),
818827
symbolicValueLowering(verif::SymbolicValueLowering::ExtModule),
819828
disableWireElimination(false), lintStaticAsserts(true),
820-
lintXmrsInDesign(true), emitAllBindFiles(false), inferDomains(false) {
829+
lintXmrsInDesign(true), emitAllBindFiles(false),
830+
domainMode(DomainMode::Disable) {
821831
if (!clOptions.isConstructed())
822832
return;
823833
outputFilename = clOptions->outputFilename;
@@ -870,5 +880,5 @@ circt::firtool::FirtoolOptions::FirtoolOptions()
870880
lintStaticAsserts = clOptions->lintStaticAsserts;
871881
lintXmrsInDesign = clOptions->lintXmrsInDesign;
872882
emitAllBindFiles = clOptions->emitAllBindFiles;
873-
inferDomains = clOptions->inferDomains;
883+
domainMode = clOptions->domainMode;
874884
}

‎test/Dialect/FIRRTL/infer-domains-errors.mlir‎

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: circt-opt -pass-pipeline='builtin.module(firrtl.circuit(firrtl-infer-domains{infer-public=true}))' %s --verify-diagnostics --split-input-file
1+
// RUN: circt-opt -pass-pipeline='builtin.module(firrtl.circuit(firrtl-infer-domains{mode=infer-all}))' %s --verify-diagnostics --split-input-file
22

33
// Port annotated with same domain type twice.
44
firrtl.circuit "DomainCrossOnPort" {
@@ -112,14 +112,34 @@ firrtl.circuit "Top" {
112112

113113
// -----
114114

115-
// Domain not exported like it should be.
115+
// Domain exported multiple times. Which do we choose?
116+
117+
firrtl.circuit "DoubleExportOfDomain" {
118+
firrtl.domain @ClockDomain
119+
120+
firrtl.module @DoubleExportOfDomain(
121+
// expected-note @below {{candidate association "DI"}}
122+
in %DI : !firrtl.domain of @ClockDomain,
123+
// expected-note @below {{candidate association "DO"}}
124+
out %DO : !firrtl.domain of @ClockDomain,
125+
in %i : !firrtl.uint<1> domains [%DO],
126+
// expected-error @below {{ambiguous "ClockDomain" association for port "o"}}
127+
out %o : !firrtl.uint<1> domains []
128+
) {
129+
// DI and DO are aliases
130+
firrtl.domain.define %DO, %DI
131+
132+
// o is on same domain as i
133+
firrtl.matchingconnect %o, %i : !firrtl.uint<1>
134+
}
135+
}
116136

117137
// -----
118138

119-
// Domain exported multiple times. Which do we choose?
139+
// Domain exported multiple times, this time with one input and one output. Which do we choose?
120140

121141
firrtl.circuit "DoubleExportOfDomain" {
122-
firrtl.domain @ClockDomain
142+
firrtl.domain @ClockDomain
123143

124144
firrtl.module @DoubleExportOfDomain(
125145
// expected-note @below {{candidate association "DI"}}
@@ -138,3 +158,29 @@ firrtl.circuit "DoubleExportOfDomain" {
138158
}
139159
}
140160

161+
// -----
162+
163+
// InstanceChoice: Each module has different domains inferred.
164+
165+
firrtl.circuit "ConflictingInstanceChoiceDomains" {
166+
firrtl.domain @ClockDomain
167+
168+
firrtl.option @Option {
169+
firrtl.option_case @X
170+
firrtl.option_case @Y
171+
}
172+
173+
firrtl.module @Foo(in %in1: !firrtl.uint<1>, in %in2: !firrtl.uint<1>, out %out: !firrtl.uint<1>) {
174+
firrtl.connect %out, %in1 : !firrtl.uint<1>
175+
}
176+
177+
firrtl.module @Bar(in %in1: !firrtl.uint<1>, in %in2: !firrtl.uint<1>, out %out: !firrtl.uint<1>) {
178+
firrtl.connect %out, %in2 : !firrtl.uint<1>
179+
}
180+
181+
firrtl.module @ConflictingInstanceChoiceDomains(in %in1: !firrtl.uint<1>, in %in2: !firrtl.uint<1>) {
182+
%inst_in1, %inst_in2, %inst_out = firrtl.instance_choice inst @Foo alternatives @Option { @X -> @Foo, @Y -> @Bar } (in in1: !firrtl.uint<1>, in in2: !firrtl.uint<1>, out out: !firrtl.uint<1>)
183+
firrtl.connect %inst_in1, %in1 : !firrtl.uint<1>, !firrtl.uint<1>
184+
firrtl.connect %inst_in2, %in2 : !firrtl.uint<1>, !firrtl.uint<1>
185+
}
186+
}

0 commit comments

Comments
 (0)