Skip to content

Commit 61b18e7

Browse files
authored
[FIRRTL] Clock gate extraction work w/ prefixing (#7946)
Weaken the check used by the `ExtractInstances` pass to determine what is a clock gate. Change this from requiring that the defname of an external module is exactly "EICG_wrapper" to instead only needing to end with "EICG_wrapper". This is done so that this will work with Chisel-time prefixing and that we would actually _like_ to prefix these clock gates. Commentary: this string matching is terrible. I realize that. However, this is a legacy API that is intending to be replaced. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
1 parent 2896a1e commit 61b18e7

File tree

3 files changed

+53
-9
lines changed

3 files changed

+53
-9
lines changed

docs/Dialects/FIRRTL/FIRRTLAnnotations.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -862,9 +862,9 @@ Example:
862862
| group | string | Name of an optional wrapper module under which to group extracted instances |
863863

864864
This annotation causes the `ExtractInstances` pass to move instances of
865-
extmodules with defname `EICG_wrapper` upwards in the hierarchy, either out of
866-
the DUT if `group` is omitted or empty, or into a submodule of the DUT with the
867-
name given in `group`. The wiring prefix is hard-coded to `clock_gate`.
865+
extmodules whose defnames end in `EICG_wrapper` upwards in the hierarchy, either
866+
out of the DUT if `group` is omitted or empty, or into a submodule of the DUT
867+
with the name given in `group`. The wiring prefix is hard-coded to `clock_gate`.
868868

869869
Applies to the circuit.
870870

lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -352,14 +352,15 @@ void ExtractInstancesPass::collectAnnos() {
352352
collectAnno(inst, instAnnos[0]);
353353
});
354354

355-
// If clock gate extraction is requested, find instances of extmodules with
356-
// the corresponding `defname` and mark them as to be extracted.
357-
// TODO: This defname really shouldn't be hardcoded here. Make this at least
358-
// somewhat configurable.
355+
// If clock gate extraction is requested, find instances of extmodules which
356+
// have a defname that ends with "EICG_wrapper". This also allows this to
357+
// compose with Chisel-time module prefixing.
358+
//
359+
// TODO: This defname matching is a terrible hack and should be replaced with
360+
// something better.
359361
if (!clkgateFileName.empty()) {
360-
auto clkgateDefNameAttr = StringAttr::get(&getContext(), "EICG_wrapper");
361362
for (auto module : circuit.getOps<FExtModuleOp>()) {
362-
if (module.getDefnameAttr() != clkgateDefNameAttr)
363+
if (!module.getDefnameAttr().getValue().ends_with("EICG_wrapper"))
363364
continue;
364365
LLVM_DEBUG(llvm::dbgs()
365366
<< "Clock gate `" << module.getModuleName() << "`\n");

test/Dialect/FIRRTL/extract-instances.mlir

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -751,3 +751,46 @@ firrtl.circuit "Plop_Foo" attributes {annotations = [{class = "sifive.enterprise
751751
firrtl.instance core sym @core @Plop_Bar()
752752
}
753753
}
754+
755+
// Test that clock gate extraction composes with Chisel-time module prefixing.
756+
// Chisel may add any number of prefixes to the clock gates. Ensure that this
757+
// will not block extraction. Additionally, test that suffixes will block
758+
// extraction.
759+
//
760+
// CHECK-LABEL: firrtl.circuit "PrefixedClockGate"
761+
firrtl.circuit "PrefixedClockGate" attributes {
762+
annotations = [
763+
{
764+
class = "sifive.enterprise.firrtl.ExtractClockGatesFileAnnotation",
765+
filename = "ckgates.txt",
766+
group = "ClockGates"
767+
}
768+
]
769+
} {
770+
firrtl.extmodule private @Prefix_EICG_wrapper() attributes {
771+
defname = "Prefix_EICG_wrapper"
772+
}
773+
firrtl.extmodule private @Prefix_EICG_wrapper_Suffix() attributes {
774+
defname = "Prefix_EICG_wrapper_Suffix"
775+
}
776+
// CHECK: firrtl.module private @ClockGates() {
777+
// CHECK-NEXT: firrtl.instance clockGate_0 @Prefix_EICG_wrapper()
778+
// CHECK-NOT: firrtl.instance clockGate_1 @Prefix_EICG_wrapper_Suffix()
779+
//
780+
// CHECK: firrtl.module @Foo
781+
// CHECK-NEXT: firrtl.instance ClockGates {{.*}} @ClockGates()
782+
// CHECK-NEXT: firrtl.instance clockGate_1 @Prefix_EICG_wrapper_Suffix()
783+
firrtl.module @Foo() attributes {
784+
annotations = [
785+
{
786+
class = "sifive.enterprise.firrtl.MarkDUTAnnotation"
787+
}
788+
]
789+
} {
790+
firrtl.instance clockGate_0 @Prefix_EICG_wrapper()
791+
firrtl.instance clockGate_1 @Prefix_EICG_wrapper_Suffix()
792+
}
793+
firrtl.module @PrefixedClockGate() {
794+
firrtl.instance foo @Foo()
795+
}
796+
}

0 commit comments

Comments
 (0)