Skip to content

Commit

Permalink
[FIRRTL] Add optional yaml parameter to view intrinsic, op (#8203)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dtzSiFive
dtzSiFive authored Feb 7, 2025
1 parent ab69d42 commit 020bfc4
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 112 deletions.
9 changes: 5 additions & 4 deletions docs/Dialects/FIRRTL/FIRRTLIntrinsics.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,11 @@ This will become a SystemVerilog Interface that is driven by its arguments.
This is _not_ a true SystemVerilog Interface, it is only lowered to one.


| Parameter | Type | Description |
| --------- | ------ | --------------------------------- |
| name | string | Instance name of the view. |
| info | string | JSON encoding the view structure. |
| Parameter | Type | Description |
| --------- | ------ | --------------------------------------- |
| name | string | Instance name of the view. |
| info | string | JSON encoding the view structure. |
| yaml | string | Optional path to emit YAML description. |

| Argument | Type | Description |
| -------- | ------ | ------------------------------- |
Expand Down
4 changes: 2 additions & 2 deletions include/circt/Dialect/FIRRTL/FIRRTLIntrinsics.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,8 @@ def ViewIntrinsicOp : FIRRTLOp<"view", []> {
debugging in a waveform. This is _not_ a true SystemVerilog Interface, it
is only lowered to one.
}];
let arguments = (ins StrAttr:$name, AugmentedBundleType:$augmentedType, Variadic<GroundType>:$inputs);
let assemblyFormat = "$name `,` $augmentedType (`,` $inputs^)? attr-dict (`:` type($inputs)^)?";
let arguments = (ins StrAttr:$name, OptionalAttr<StrAttr>:$yamlFile, AugmentedBundleType:$augmentedType, Variadic<GroundType>:$inputs);
let assemblyFormat = "$name `,` (`yaml` $yamlFile^ `,`)? $augmentedType (`,` $inputs^)? attr-dict (`:` type($inputs)^)?";
}

#endif // CIRCT_DIALECT_FIRRTL_FIRRTLINTRINSICS_TD
7 changes: 4 additions & 3 deletions lib/Dialect/FIRRTL/FIRRTLIntrinsics.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,8 @@ class ViewConverter : public IntrinsicConverter {
GenericIntrinsicOpAdaptor adaptor,
PatternRewriter &rewriter) override {
// Check structure of the intrinsic.
if (gi.hasNoOutput() || gi.namedParam("info") || gi.namedParam("name"))
if (gi.hasNoOutput() || gi.namedParam("info") || gi.namedParam("name") ||
gi.namedParam("yaml", true))
return failure();

// Check operands.
Expand Down Expand Up @@ -944,9 +945,9 @@ class ViewConverter : public IntrinsicConverter {
<< numLeaves << " leaf elements";

// Check complete, convert!

auto yaml = gi.getParamValue<StringAttr>("yaml");
rewriter.replaceOpWithNewOp<ViewIntrinsicOp>(
gi.op, nameAttr.getValue(), augmentedType, adaptor.getOperands());
gi.op, nameAttr.getValue(), yaml, augmentedType, adaptor.getOperands());
return success();
}
};
Expand Down
47 changes: 25 additions & 22 deletions lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,8 @@ struct GrandCentralPass
DenseMap<Attribute, sv::InterfaceOp> interfaceMap;

/// Emit the hierarchy yaml file.
void emitHierarchyYamlFile(SmallVectorImpl<sv::InterfaceOp> &intfs);
void emitHierarchyYamlFile(StringRef yamlPath,
SmallVectorImpl<sv::InterfaceOp> &intfs);
};

} // namespace
Expand Down Expand Up @@ -1915,6 +1916,12 @@ void GrandCentralPass::runOnOperation() {
llvm::dbgs() << "\n";
});

// per-YAML output list of interfaces.
llvm::SmallMapVector<StringAttr, SmallVector<sv::InterfaceOp, 0>, 1>
interfaceYAMLMap;
if (maybeHierarchyFileYAML.has_value())
interfaceYAMLMap[maybeHierarchyFileYAML.value()] = {};

// Scan entire design for View operations.
SmallVector<ViewIntrinsicOp> views;
circuitOp.walk([&views](ViewIntrinsicOp view) { views.push_back(view); });
Expand All @@ -1923,8 +1930,8 @@ void GrandCentralPass::runOnOperation() {
// built exist. However, still generate the YAML file if the annotation for
// this was passed in because some flows expect this.
if (worklist.empty() && views.empty()) {
SmallVector<sv::InterfaceOp, 0> interfaceVec;
emitHierarchyYamlFile(interfaceVec);
for (auto &[yamlPath, intfs] : interfaceYAMLMap)
emitHierarchyYamlFile(yamlPath.getValue(), intfs);
return markAllAnalysesPreserved();
}

Expand Down Expand Up @@ -2254,8 +2261,8 @@ void GrandCentralPass::runOnOperation() {
mod->erase();
}

SmallVector<sv::InterfaceOp, 0> interfaceVec;
emitHierarchyYamlFile(interfaceVec);
for (auto &[yamlPath, intfs] : interfaceYAMLMap)
emitHierarchyYamlFile(yamlPath.getValue(), intfs);
return;
}

Expand Down Expand Up @@ -2311,7 +2318,6 @@ void GrandCentralPass::runOnOperation() {
// will use XMRs to drive the interface. If extraction info is available,
// then the top-level instantiate interface will be marked for extraction via
// a SystemVerilog bind.
SmallVector<sv::InterfaceOp, 2> interfaceVec;
SmallDenseMap<FModuleLike, SmallVector<InterfaceElemsBuilder>>
companionToInterfaceMap;
auto compareInterfaceSignal = [&](InterfaceElemsBuilder &lhs,
Expand Down Expand Up @@ -2481,7 +2487,8 @@ void GrandCentralPass::runOnOperation() {

++numViews;

interfaceVec.push_back(topIface);
if (maybeHierarchyFileYAML.has_value())
interfaceYAMLMap[maybeHierarchyFileYAML.value()].push_back(topIface);

// Instantiate the interface inside the companion.
builder.setInsertionPointToStart(companionModule.getBodyBlock());
Expand Down Expand Up @@ -2567,6 +2574,7 @@ void GrandCentralPass::runOnOperation() {
sv::InterfaceOp topIface;
auto containingOutputFileAttr =
viewParentMod->getAttrOfType<hw::OutputFileAttr>("output_file");
auto yamlPath = view.getYamlFileAttr();
for (const auto &ifaceBuilder : interfaceBuilder) {
auto builder = OpBuilder::atBlockEnd(getOperation().getBodyBlock());
auto loc = getOperation().getLoc();
Expand Down Expand Up @@ -2599,7 +2607,7 @@ void GrandCentralPass::runOnOperation() {
// If we need to generate a YAML representation of this interface,
// then add an attribute indicating that this `sv::VerbatimOp` is
// actually a description.
if (maybeHierarchyFileYAML)
if (yamlPath)
descriptionOp->setAttr("firrtl.grandcentral.yaml.type",
builder.getStringAttr("description"));
}
Expand All @@ -2609,7 +2617,7 @@ void GrandCentralPass::runOnOperation() {

// If we need to generate a YAML representation of the interface, then
// add attributes that describe what this `sv::VerbatimOp` is.
if (maybeHierarchyFileYAML) {
if (yamlPath) {
if (str->instantiation)
instanceOp->setAttr("firrtl.grandcentral.yaml.type",
builder.getStringAttr("instance"));
Expand All @@ -2634,7 +2642,8 @@ void GrandCentralPass::runOnOperation() {

++numViews;

interfaceVec.push_back(topIface);
if (yamlPath)
interfaceYAMLMap[yamlPath].push_back(topIface);

// Instantiate the interface before the view and the XMR's we inserted
// above.
Expand All @@ -2646,7 +2655,8 @@ void GrandCentralPass::runOnOperation() {
view.erase();
}

emitHierarchyYamlFile(interfaceVec);
for (auto &[yamlPath, intfs] : interfaceYAMLMap)
emitHierarchyYamlFile(yamlPath.getValue(), intfs);

// Signal pass failure if any errors were found while examining circuit
// annotations.
Expand All @@ -2656,13 +2666,7 @@ void GrandCentralPass::runOnOperation() {
}

void GrandCentralPass::emitHierarchyYamlFile(
SmallVectorImpl<sv::InterfaceOp> &intfs) {
// If a `GrandCentralHierarchyFileAnnotation` was passed in, generate a YAML
// representation of the interfaces that we produced with the filename that
// that annotation provided.
if (!maybeHierarchyFileYAML)
return;

StringRef yamlPath, SmallVectorImpl<sv::InterfaceOp> &intfs) {
CircuitOp circuitOp = getOperation();

std::string yamlString;
Expand All @@ -2673,10 +2677,9 @@ void GrandCentralPass::emitHierarchyYamlFile(

auto builder = OpBuilder::atBlockBegin(circuitOp.getBodyBlock());
builder.create<sv::VerbatimOp>(builder.getUnknownLoc(), yamlString)
->setAttr("output_file",
hw::OutputFileAttr::getFromFilename(
&getContext(), maybeHierarchyFileYAML->getValue(),
/*excludeFromFileList=*/true));
->setAttr("output_file", hw::OutputFileAttr::getFromFilename(
&getContext(), yamlPath,
/*excludeFromFileList=*/true));
LLVM_DEBUG({ llvm::dbgs() << "Generated YAML:" << yamlString << "\n"; });
}

Expand Down
58 changes: 9 additions & 49 deletions test/Dialect/FIRRTL/grand-central-view.mlir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,7 @@
// types. All the interfaces share a common, simple circuit that provides two
// signals, "foo" and "bar".

firrtl.circuit "InterfaceGroundType" attributes {
annotations = [
{
class = "sifive.enterprise.grandcentral.ExtractGrandCentralAnnotation",
directory = "gct-dir",
filename = "bindings.sv"
},
{
class = "sifive.enterprise.grandcentral.GrandCentralHierarchyFileAnnotation",
filename = "gct.yaml"
}
]
} {
firrtl.circuit "InterfaceGroundType" {
firrtl.module @Companion() {
// These are dummy references created for the purposes of the test.
%_ui0 = firrtl.verbatim.expr "???" : () -> !firrtl.uint<0>
Expand All @@ -38,7 +26,7 @@ firrtl.circuit "InterfaceGroundType" attributes {
%c0_ui1 = firrtl.constant 0 : !firrtl.uint<1>
%c-1_si2 = firrtl.constant -1 : !firrtl.sint<2>

firrtl.view "GroundView", <{
firrtl.view "GroundView", yaml "gct.yaml", <{
class = "sifive.enterprise.grandcentral.AugmentedBundleType",
defName = "GroundView",
elements = [
Expand All @@ -55,7 +43,7 @@ firrtl.circuit "InterfaceGroundType" attributes {
]
}>, %foo, %bar : !firrtl.uint<1>, !firrtl.uint<2>

firrtl.view "VectorView", <{
firrtl.view "VectorView", yaml "gct.yaml", <{
class = "sifive.enterprise.grandcentral.AugmentedBundleType",
defName = "VectorView",
elements = [
Expand All @@ -77,7 +65,7 @@ firrtl.circuit "InterfaceGroundType" attributes {
}>, %foo, %foo : !firrtl.uint<1>, !firrtl.uint<1>


firrtl.view "BundleView", <{
firrtl.view "BundleView", yaml "gct.yaml", <{
class = "sifive.enterprise.grandcentral.AugmentedBundleType",
defName = "BundleView",
elements = [
Expand All @@ -99,7 +87,7 @@ firrtl.circuit "InterfaceGroundType" attributes {
]
}>, %foo, %bar : !firrtl.uint<1>, !firrtl.uint<2>

firrtl.view "VectorOfBundleView", <{
firrtl.view "VectorOfBundleView", yaml "gct.yaml", <{
class = "sifive.enterprise.grandcentral.AugmentedBundleType",
defName = "VectorOfBundleView",
elements = [
Expand Down Expand Up @@ -127,7 +115,7 @@ firrtl.circuit "InterfaceGroundType" attributes {
]
}>, %foo, %bar : !firrtl.uint<1>, !firrtl.uint<2>

firrtl.view "VectorOfVectorView", <{
firrtl.view "VectorOfVectorView", yaml "gct.yaml", <{
class = "sifive.enterprise.grandcentral.AugmentedBundleType",
defName = "VectorOfVectorView",
elements = [
Expand Down Expand Up @@ -178,7 +166,7 @@ firrtl.circuit "InterfaceGroundType" attributes {
]
}>, %bar, %bar, %bar, %bar, %bar, %bar : !firrtl.uint<2>, !firrtl.uint<2>, !firrtl.uint<2>, !firrtl.uint<2>, !firrtl.uint<2>, !firrtl.uint<2>

firrtl.view "ConstantView", <{
firrtl.view "ConstantView", yaml "gct.yaml", <{
class = "sifive.enterprise.grandcentral.AugmentedBundleType",
defName = "ConstantView",
elements = [
Expand All @@ -194,7 +182,7 @@ firrtl.circuit "InterfaceGroundType" attributes {
]
}>, %c0_ui1, %c-1_si2 : !firrtl.uint<1>, !firrtl.sint<2>

firrtl.view "ZeroWidthView", <{
firrtl.view "ZeroWidthView", yaml "gct.yaml", <{
class = "sifive.enterprise.grandcentral.AugmentedBundleType",
defName = "ZeroWidthView",
elements = [
Expand All @@ -210,13 +198,6 @@ firrtl.circuit "InterfaceGroundType" attributes {
}
}

// All AugmentedBundleType annotations are removed from the circuit.
//
// CHECK-LABEL: firrtl.circuit "InterfaceGroundType" {{.+}} {annotations =
// CHECK-SAME: class = "sifive.enterprise.grandcentral.ExtractGrandCentralAnnotation"
// CHECK-NOT: class = "sifive.enterprise.grandcentral.AugmentedBundleType"
// CHECK-SAME: {

// Check YAML Output.
//
// Note: Built-in vector serialization works slightly differently than
Expand Down Expand Up @@ -420,15 +401,7 @@ firrtl.circuit "InterfaceGroundType" attributes {

// -----

firrtl.circuit "Top" attributes {
annotations = [
{
class = "sifive.enterprise.grandcentral.ExtractGrandCentralAnnotation",
directory = ".",
filename = "bindings.sv"
}
]
} {
firrtl.circuit "Top" {
firrtl.module private @Companion_w1(in %_gen_uint: !firrtl.uint<1>) {
%view_uintrefPort = firrtl.node %_gen_uint : !firrtl.uint<1>
firrtl.view "View_w1", <{
Expand Down Expand Up @@ -501,19 +474,6 @@ firrtl.circuit "Top" attributes {

// -----

firrtl.circuit "NoInterfaces" attributes {
annotations = [
{class = "sifive.enterprise.grandcentral.GrandCentralHierarchyFileAnnotation",
filename = "gct.yaml"}]} {
firrtl.module @NoInterfaces() {}
}

// CHECK-LABEL: module {
// CHECK: sv.verbatim
// CHECK-SAME: []

// -----

firrtl.circuit "SetOutputDir" {
firrtl.module @SetOutputDir() attributes {output_file = #hw.output_file<"path/">} {
firrtl.view "view", <{
Expand Down
4 changes: 4 additions & 0 deletions test/Dialect/FIRRTL/lower-intrinsics.mlir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,5 +228,9 @@ firrtl.circuit "Foo" {
// CHECK-SAME: }>,
// Check operands and types.
// CHECK-SAME: %4, %3, %2, %1, %0 : !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>

firrtl.int.generic "circt_view" <name: none = "view2", yaml: none = "views.yml", info: none = "{\"class\":\"sifive.enterprise.grandcentral.AugmentedBundleType\",\"defName\":\"ViewName\",\"elements\":[]}"> : () -> ()

// CHECK: firrtl.view "view2", yaml "views.yml", <{
}
}
49 changes: 49 additions & 0 deletions test/firtool/views-directories-yaml.fir
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
; RUN: firtool %s | FileCheck %s
; RUN: firtool %s | FileCheck %s --check-prefix=YAML

FIRRTL version 4.2.0
circuit ViewInLayer:
layer Views, bind, "views/":
layer Other, bind, "other/":
layer A, bind, "other/A/":
layer B, inline:

public module ViewInLayer:
input x : UInt<1>
input y : UInt<1>
intrinsic(circt_view<name="view", info="{\"class\":\"sifive.enterprise.grandcentral.AugmentedBundleType\", \"defName\": \"Root\", \"elements\": [{\"name\": \"foo\", \"tpe\":{\"class\":\"sifive.enterprise.grandcentral.AugmentedGroundType\"}}]}">, x)

layerblock Views:
intrinsic(circt_view<name="view", info="{\"class\":\"sifive.enterprise.grandcentral.AugmentedBundleType\", \"defName\": \"MyView\", \"elements\": [{\"name\": \"foo\", \"tpe\":{\"class\":\"sifive.enterprise.grandcentral.AugmentedGroundType\"}}]}">, x)

layerblock Other:
node n = and(x, y)
layerblock A:
; TODO: Maybe the YAML path should be relative to governing output directory?
intrinsic(circt_view<name="view_a", yaml="other/views.yml", info="{\"class\":\"sifive.enterprise.grandcentral.AugmentedBundleType\", \"defName\": \"AView\", \"elements\": [{\"name\": \"foo\", \"tpe\":{\"class\":\"sifive.enterprise.grandcentral.AugmentedGroundType\"}}]}">, n)
layerblock B:
intrinsic(circt_view<name="view_b", yaml="other/views.yml", info="{\"class\":\"sifive.enterprise.grandcentral.AugmentedBundleType\", \"defName\": \"BView\", \"elements\": [{\"name\": \"foo\", \"tpe\":{\"class\":\"sifive.enterprise.grandcentral.AugmentedGroundType\"}}]}">, n)

; CHECK-NOT: FILE
; CHECK: module ViewInLayer
; CHECK-NOT: FILE
; CHECK: interface Root

; CHECK-DAG: FILE "views{{[/\]}}MyView.sv"
; CHECK-DAG: FILE "other{{[/\]}}A{{[/\]}}AView.sv"
; CHECK-DAG: FILE "other{{[/\]}}BView.sv"
; CHECK-DAG: FILE "other{{[/\]}}views.yml"

; YAML: FILE "other{{[/\]}}views.yml"
; YAML: - name: AView
; YAML-NEXT: fields:
; YAML-NEXT: - name: foo
; YAML-NEXT: dimensions: [ ]
; YAML-NEXT: width: 1
; YAML-NEXT: instances: []
; YAML-NEXT: - name: BView
; YAML-NEXT: fields:
; YAML-NEXT: - name: foo
; YAML-NEXT: dimensions: [ ]
; YAML-NEXT: width: 1
; YAML-NEXT: instances: []
Loading

0 comments on commit 020bfc4

Please sign in to comment.