[FIRRTL] Add a pass to specialize instance choices #6507
Conversation
nandor
requested review from
darthscsi,
dtzSiFive and
seldridge
as code owners
nandor
force-pushed
the
dev/nandor/specialize-choices
branch
from
120bd0b
to
e869858
Compare
| if (selectedOption.empty() || selectedCase.empty()) { | ||
| markAllAnalysesPreserved(); | ||
| return; | ||
| } | ||
|
|
||
| auto optionGroup = circuit.lookupSymbol<OptionOp>(selectedOption); | ||
| if (!optionGroup) { | ||
| markAllAnalysesPreserved(); | ||
| return; | ||
| } | ||
|
|
||
| auto optionCase = optionGroup.lookupSymbol<OptionCaseOp>(selectedCase); | ||
| if (!optionCase) { | ||
| markAllAnalysesPreserved(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
I don't think this is a great API---if these fail, then there's a user error and things arguably shouldn't proceed. Specifically, for something like the user specifies an option/case that doesn't exist or misspells it. If the pass silently does a no-op, then it's difficult to debug as it looks like the pass did the wrong thing.
tl;dr: more signalPassFailure() and tests of this behavior. 😉
There was a problem hiding this comment.
We might set up a flow for all designs and ask for specialization for a target, but a smaller input might have no use of options at all, in which case the pass would fail. I think it is reasonable to turn this into a no-op if there is no option to specialize.
To better reflect this, I added warning emission if an option is not found.
| @@ -850,4 +850,24 @@ def Lint : | |||
| let constructor = "circt::firrtl::createLintingPass()"; | |||
| } | |||
|
|
|||
| def SpecializeOption : | |||
| Pass<"firrtl-specialize-option", "firrtl::CircuitOp"> { | |||
There was a problem hiding this comment.
This could be a FModuleOp pass by my reading of https://mlir.llvm.org/docs/PassManagement/#operation-pass. What does the proposed error handling look like in that case?
There was a problem hiding this comment.
We'd get a bazillion copies of the error message? I think it's clearer handle parallelism within the pass in this case.
nandor
force-pushed
the
dev/nandor/firrtl-instance-choice
branch
from
ebef73b
to
f23cd10
Compare
nandor
force-pushed
the
dev/nandor/specialize-choices
branch
4 times, most recently
from
7ad0c0d
to
035c097
Compare
The `SpecializeOptions` pass eliminates instance choices and replaces them with instances targeting the modules associated with a case provided through the options.
nandor
force-pushed
the
dev/nandor/specialize-choices
branch
from
035c097
to
4b6ec9f
Compare
The
SpecializeOptionspass eliminates instance choices and replaces them with instances targeting the modules associated with a case provided through the options.