[FIRRTL][WireDFT] Wire test-en to ClockGateIntrinsic. #6488
Conversation
|
Curious, local clang-tidy doesn't report that (seems like it should). Welp. |
| @@ -381,7 +402,7 @@ void WireDFTPass::runOnOperation() { | |||
|
|
|||
| auto wireUp = [&](Value startSignal, FModuleOp signalModule, | |||
| StringAttr portName, StringRef portNameFriendly, | |||
| unsigned targetPortNo, auto &targets) -> LogicalResult { | |||
| auto &dests) -> LogicalResult { | |||
There was a problem hiding this comment.
Templated lambdas are magic 🌈 🙌
| auto testEnableDests = llvm::map_to_vector(clockGates, [&](auto *rec) { | ||
| return cast<InstanceOp>(*rec->getInstance()).getResult(testEnPortNo); | ||
| }); | ||
| llvm::append_range( | ||
| testEnableDests, | ||
| llvm::map_range(cgIntrinsics, [&](ClockGateIntrinsicOp cgOp) { | ||
| // Insert wire to run test-enable signal to, | ||
| // and change the CG intrinsic to use it as such. | ||
| auto builder = ImplicitLocOpBuilder(cgOp.getLoc(), cgOp); | ||
| auto wire = builder.create<WireOp>(enableSignal.getType()); | ||
| cgOp.getTestEnableMutable().assign(wire.getResult()); | ||
| return wire.getResult(); | ||
| })); |
There was a problem hiding this comment.
Clever! I was wondering how you're going to deal with the intrinsic having an actual operand instead of a wire-y thing. This is a nice low-friction way to do it!
There was a problem hiding this comment.
Yeah, it will add unnecessary wires in common case where the test-enable comes from above, but needed for some cases where it comes from below or in same module (without dominance checking). The wiring logic here isn't as robust as WiringProblem's and that has some limitations too IIRC in some corner cases. Anyway, once we don't have to handle both forms it'd be good to rework this to handle this better but low-friction indeed 👍 .
Extend WireDFT to support wiring the test-enable signal to ClockGateIntrinsicOp's in same manner it does extmodules magically matched as the test-enable wiring targets.