From 8e3d29ffa3a510015a7eb7dd4048621d1c9bbae6 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Mon, 29 Jan 2024 16:40:55 -0500 Subject: [PATCH] [FIRRTL] Add parser/emitter support for layer-associated probes (#6552) * [FIRRTL] Parse Layer-Associated Probes Add support to the FIRRTL parse to handle probes associated with layers. Signed-off-by: Schuyler Eldridge * [FIRRTL] Emit Layer-associated Probes Add emitter support for layer-associated probes. Signed-off-by: Schuyler Eldridge * fixup! [FIRRTL] Parse Layer-Associated Probes * Add a fir version check for colored probes --------- Signed-off-by: Schuyler Eldridge Co-authored-by: Robert Young --- lib/Dialect/FIRRTL/Export/FIREmitter.cpp | 22 ++++++++- lib/Dialect/FIRRTL/Import/FIRParser.cpp | 35 ++++++++++++-- test/Dialect/FIRRTL/emit-basic.mlir | 36 +++++++++++++- test/Dialect/FIRRTL/parse-basic.fir | 6 ++- test/Dialect/FIRRTL/parse-errors.fir | 61 ++++++++++++++++++++++++ 5 files changed, 152 insertions(+), 8 deletions(-) diff --git a/lib/Dialect/FIRRTL/Export/FIREmitter.cpp b/lib/Dialect/FIRRTL/Export/FIREmitter.cpp index 82b00f6f9f18..53ed33c572b5 100644 --- a/lib/Dialect/FIRRTL/Export/FIREmitter.cpp +++ b/lib/Dialect/FIRRTL/Export/FIREmitter.cpp @@ -253,6 +253,18 @@ struct Emitter { [&](Value v) { emitSubExprIBox2(v); }); } + /// Emit a (potentially nested) symbol reference as `A.B.C`. + void emitSymbol(SymbolRefAttr symbol) { + ps.ibox(2, IndentStyle::Block); + ps << symbol.getRootReference(); + for (auto nested : symbol.getNestedReferences()) { + ps.zerobreak(); + ps << "."; + ps << nested.getAttr(); + } + ps.end(); + } + private: /// Emit an error and remark that emission failed. InFlightDiagnostic emitError(Operation *op, const Twine &message) { @@ -1377,8 +1389,16 @@ void Emitter::emitType(Type type, bool includeConst) { if (type.getForceable()) ps << "RW"; ps << "Probe<"; + ps.cbox(2, IndentStyle::Block); + ps.zerobreak(); emitType(type.getType()); - ps << ">"; + if (auto layer = type.getLayer()) { + ps << ","; + ps.space(); + emitSymbol(type.getLayer()); + } + ps << BreakToken(0, -2) << ">"; + ps.end(); }) .Case([&](AnyRefType type) { ps << "AnyRef"; }) .Case([&](StringType type) { ps << "String"; }) diff --git a/lib/Dialect/FIRRTL/Import/FIRParser.cpp b/lib/Dialect/FIRRTL/Import/FIRParser.cpp index 4d2a7f8e9d14..d44b96c40f53 100644 --- a/lib/Dialect/FIRRTL/Import/FIRParser.cpp +++ b/lib/Dialect/FIRRTL/Import/FIRParser.cpp @@ -934,13 +934,30 @@ ParseResult FIRParser::parseType(FIRRTLType &result, const Twine &message) { auto kind = getToken().getKind(); auto loc = getToken().getLoc(); consumeToken(); - FIRRTLType type; + // Inner Type + FIRRTLType type; if (parseToken(FIRToken::less, "expected '<' in reference type") || - parseType(type, "expected probe data type") || - parseToken(FIRToken::greater, "expected '>' in reference type")) + parseType(type, "expected probe data type")) return failure(); + // Probe Color + SmallVector layers; + if (getToken().getKind() == FIRToken::identifier) { + if (requireFeature({3, 2, 0}, "colored probes")) + return failure(); + do { + StringRef layer; + loc = getToken().getLoc(); + if (parseId(layer, "expected layer name")) + return failure(); + layers.push_back(layer); + } while (consumeIf(FIRToken::period)); + } + + if (!consumeIf(FIRToken::greater)) + return emitError(loc, "expected '>' to end reference type"); + bool forceable = kind == FIRToken::kw_RWProbe; auto innerType = type_dyn_cast(type); @@ -953,7 +970,17 @@ ParseResult FIRParser::parseType(FIRRTLType &result, const Twine &message) { if (forceable && innerType.containsConst()) return emitError(loc, "rwprobe cannot contain const"); - result = RefType::get(innerType, forceable); + SymbolRefAttr layer; + if (!layers.empty()) { + auto nestedLayers = + llvm::map_range(ArrayRef(layers).drop_front(), [&](StringRef a) { + return FlatSymbolRefAttr::get(getContext(), a); + }); + layer = SymbolRefAttr::get(getContext(), layers.front(), + llvm::to_vector(nestedLayers)); + } + + result = RefType::get(innerType, forceable, layer); break; } diff --git a/test/Dialect/FIRRTL/emit-basic.mlir b/test/Dialect/FIRRTL/emit-basic.mlir index 5989c26fdc9e..c77e52c99786 100644 --- a/test/Dialect/FIRRTL/emit-basic.mlir +++ b/test/Dialect/FIRRTL/emit-basic.mlir @@ -704,13 +704,18 @@ firrtl.circuit "Foo" { } } // CHECK: module ModuleWithGroups : - // CHECK-NEXT: layerblock GroupA : + // CHECK-NEXT: output a : Probe, GroupA> + // CHECK-NEXT: output b : RWProbe, GroupA.GroupB> + // CHECK: layerblock GroupA : // CHECK-NEXT: layerblock GroupB : // CHECK-NEXT: layerblock GroupC : // CHECK-NEXT: layerblock GroupD : // CHECK-NEXT: layerblock GroupE : // CHECK-NEXT: layerblock GroupF : - firrtl.module @ModuleWithGroups() { + firrtl.module @ModuleWithGroups( + out %a: !firrtl.probe, @GroupA>, + out %b: !firrtl.rwprobe, @GroupA::@GroupB> + ) { firrtl.layerblock @GroupA { firrtl.layerblock @GroupA::@GroupB { firrtl.layerblock @GroupA::@GroupB::@GroupC { @@ -725,6 +730,33 @@ firrtl.circuit "Foo" { } } + // Test line-breaks for very large layer associations. + firrtl.layer @Group1234567890 bind { + firrtl.layer @Group1234567890 bind { + firrtl.layer @Group1234567890 bind { + firrtl.layer @Group1234567890 bind { + firrtl.layer @Group1234567890 bind { + firrtl.layer @Group1234567890 bind { + firrtl.layer @Group1234567890 bind { + firrtl.layer @Group1234567890 bind {} + } + } + } + } + } + } + } + + // CHECK: module ModuleWithLongProbeColor + // CHECK-NEXT: output o : Probe< + // CHECK-NEXT: UInt<1>, + // CHECK-NEXT: Group1234567890.Group1234567890.Group1234567890.Group1234567890 + // CHECK-NEXT: .Group1234567890.Group1234567890.Group1234567890.Group1234567890 + // CHECK-NEXT: > + firrtl.module @ModuleWithLongProbeColor( + out %o: !firrtl.probe, @Group1234567890::@Group1234567890::@Group1234567890::@Group1234567890::@Group1234567890::@Group1234567890::@Group1234567890::@Group1234567890> + ) {} + // CHECK: module RWProbe : // CHECK-NEXT: input in : { a : UInt<1> }[2] // CHECK-NEXT: output p : RWProbe> diff --git a/test/Dialect/FIRRTL/parse-basic.fir b/test/Dialect/FIRRTL/parse-basic.fir index fd2aaa44fa98..284b721bd222 100644 --- a/test/Dialect/FIRRTL/parse-basic.fir +++ b/test/Dialect/FIRRTL/parse-basic.fir @@ -1671,9 +1671,13 @@ circuit Layers: ; CHECK-NEXT: } ; CHECK-NEXT: } - ; CHECK: firrtl.module @Layers + ; CHECK: firrtl.module @Layers + ; CHECK-SAME: out %b: !firrtl.probe, @A> + ; CHECK-SAME: out %c: !firrtl.rwprobe, @A::@B> module Layers: input a: UInt<1> + output b: Probe, A> + output c: RWProbe, A.B> layerblock A: node A_a = a diff --git a/test/Dialect/FIRRTL/parse-errors.fir b/test/Dialect/FIRRTL/parse-errors.fir index 3e7e323fd2ca..a9b7e103cccb 100644 --- a/test/Dialect/FIRRTL/parse-errors.fir +++ b/test/Dialect/FIRRTL/parse-errors.fir @@ -1253,3 +1253,64 @@ circuit Foo: option Platform: FPGA FPGA + +;// ----- +FIRRTL version 3.1.0 +circuit Foo: + + module Foo: + ; expected-error @below {{colored probes are a FIRRTL 3.2.0+ feature, but the specified FIRRTL version was 3.1.0}} + output a: Probe, A> + +;// ----- +FIRRTL version 4.0.0 +circuit Foo: + layer A bind: + module Foo: + output a: Probe< + ; expected-error @below {{expected probe data type}} +;// ----- +FIRRTL version 4.0.0 +circuit Foo: + layer A bind: + module Foo: + ; expected-error @below {{expected '<' in reference type}} + output a: Probe X + +;// ----- +FIRRTL version 4.0.0 +circuit Foo: + layer A bind: + module Foo: + output a: Probe + ; expected-error @below {{expected '<' in reference type}} +;// ----- +FIRRTL version 4.0.0 +circuit Foo: + layer A bind: + module Foo: + ; expected-error @below {{expected probe data type}} + output a: Probe<> + +;// ----- +FIRRTL version 4.0.0 +circuit Foo: + layer A bind: + module Foo: + ; expected-error @below {{expected '>' to end reference type}} + output a: Probe +;// ----- +FIRRTL version 4.0.0 +circuit Foo: + layer A bind: + module Foo: + ; expected-error @below {{expected '>' to end reference type}} + output a: Probe A + +;// ----- +FIRRTL version 4.0.0 +circuit Foo: + layer A bind: + module Foo: + ; expected-error @below {{expected layer name}} + output a: Probe A.>