Skip to content

Commit 106815a

Browse files
seldridgeseldridge
authored
[FIRRTL] Preserve/Tap all "Named" Nodes or Wires (#2676)
Change end-to-end FIRRTL compilation behavior to preserve (via tapping) all nodes and wires which are "named". A "named" node or wire is one whose name does not begin with an underscore. Tapping is done by creating a "shadow node" that is assigned the value of the actual wire and marked "don't touch". This is done to enable better debug-ability of Chisel designs by enabling users to always have references to named things they define in Chisel. More specifically, anytime a Chisel user defines a `val foo = <expression>`, CIRCT will now produce a wire called "foo" in the output Verilog. Add a parser option for controlling whether or not "named" wires and nodes will be preserved from FIRRTL to Verilog. Add a firtool command-line option for disabling name preservation during FIRRTL parsing. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
1 parent 727db57 commit 106815a

File tree

8 files changed

+89
-9
lines changed

8 files changed

+89
-9
lines changed

‎include/circt/Dialect/FIRRTL/FIRParser.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ struct FIRParserOptions {
3838
/// This, along with numOMIRFiles provides structure to the buffers in the
3939
/// source manager.
4040
unsigned numAnnotationFiles;
41+
/// If true, then the parser will NOT generate debug taps for "named" wires
42+
/// and nodes. A "named" wire/node is one whose name does NOT beging with a
43+
/// leading "_". If false, debug-ability is greatly increased. If true, much
44+
/// more compact Verilog will be generated.
45+
bool disableNamePreservation = true;
4146
};
4247

4348
mlir::OwningOpRef<mlir::ModuleOp> importFIRFile(llvm::SourceMgr &sourceMgr,

‎lib/Dialect/FIRRTL/Import/FIRParser.cpp‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2996,6 +2996,8 @@ ParseResult FIRStmtParser::parseMem(unsigned memIndent) {
29962996
return moduleContext.addSymbolEntry(id, entryID, startTok.getLoc());
29972997
}
29982998

2999+
static bool isNamed(StringRef id) { return !id.startswith("_"); }
3000+
29993001
/// node ::= 'node' id '=' exp info?
30003002
ParseResult FIRStmtParser::parseNode() {
30013003
auto startTok = consumeToken(FIRToken::kw_node);
@@ -3040,8 +3042,19 @@ ParseResult FIRStmtParser::parseNode() {
30403042
moduleContext.targetsInModule, initializerType);
30413043

30423044
auto sym = getSymbolIfRequired(annotations, id);
3043-
auto result = builder.create<NodeOp>(initializer.getType(), initializer, id,
3045+
auto isNamed =
3046+
!getConstants().options.disableNamePreservation && ::isNamed(id);
3047+
auto result = builder.create<NodeOp>(initializer.getType(), initializer,
3048+
isNamed ? (Twine("_") + id).str() : id,
30443049
annotations, sym);
3050+
// If the node is named, then add a debug tap.
3051+
//
3052+
// TODO: Change this once the FIRRTL spec supports "named" vs. "unnamed"
3053+
// nodes.
3054+
if (isNamed)
3055+
builder.create<NodeOp>(
3056+
initializer.getType(), result, id, getConstants().emptyArrayAttr,
3057+
StringAttr::get(annotations.getContext(), modNameSpace.newName(id)));
30453058
return moduleContext.addSymbolEntry(id, result, startTok.getLoc());
30463059
}
30473060

@@ -3070,7 +3083,26 @@ ParseResult FIRStmtParser::parseWire() {
30703083
moduleContext.targetsInModule, type);
30713084

30723085
auto sym = getSymbolIfRequired(annotations, id);
3073-
auto result = builder.create<WireOp>(type, id, annotations, sym);
3086+
auto isNamed =
3087+
!getConstants().options.disableNamePreservation && ::isNamed(id);
3088+
auto result = builder.create<WireOp>(
3089+
type, isNamed ? (Twine("_") + id).str() : id, annotations, sym);
3090+
// If the wire is named, then add a debug tap.
3091+
//
3092+
// TODO: Change this once the FIRRTL spec supports "named" vs. "unnamed"
3093+
// wires.
3094+
if (isNamed) {
3095+
if (type.isPassive())
3096+
builder.create<NodeOp>(
3097+
type, result, id, getConstants().emptyArrayAttr,
3098+
StringAttr::get(annotations.getContext(), modNameSpace.newName(id)));
3099+
else {
3100+
auto debug = builder.create<WireOp>(
3101+
type.getPassiveType(), id, getConstants().emptyArrayAttr,
3102+
StringAttr::get(annotations.getContext(), modNameSpace.newName(id)));
3103+
builder.create<ConnectOp>(debug, result);
3104+
}
3105+
}
30743106
return moduleContext.addSymbolEntry(id, result, startTok.getLoc());
30753107
}
30763108

‎test/Dialect/FIRRTL/SFCTests/GrandCentralInterfaces/Wire.fir‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
; RUN: firtool --firrtl-grand-central --verilog --annotation-file %S/Wire.anno.json --annotation-file %S/Extract.anno.json %s | FileCheck %s --check-prefixes CHECK,EXTRACT
2-
; RUN: firtool --firrtl-grand-central --verilog --annotation-file %S/Wire.anno.json %s | FileCheck %s --check-prefixes CHECK,NOEXTRACT
3-
; RUN: firtool --firrtl-grand-central --verilog --annotation-file %S/Wire.anno.json --annotation-file %S/Extract.anno.json %s --annotation-file %S/YAML.anno.json | FileCheck %s --check-prefixes YAML
4-
; RUN: firtool --firrtl-grand-central --split-verilog --annotation-file %S/Wire.anno.json %s -o %t.folder > %t && cat %t.folder/MyView_companion.sv | FileCheck %s --check-prefixes MYVIEW_COMPANION
1+
; RUN: firtool --firrtl-grand-central --disable-name-preservation --verilog --annotation-file %S/Wire.anno.json --annotation-file %S/Extract.anno.json %s | FileCheck %s --check-prefixes CHECK,EXTRACT
2+
; RUN: firtool --firrtl-grand-central --disable-name-preservation --verilog --annotation-file %S/Wire.anno.json %s | FileCheck %s --check-prefixes CHECK,NOEXTRACT
3+
; RUN: firtool --firrtl-grand-central --disable-name-preservation --verilog --annotation-file %S/Wire.anno.json --annotation-file %S/Extract.anno.json %s --annotation-file %S/YAML.anno.json | FileCheck %s --check-prefixes YAML
4+
; RUN: firtool --firrtl-grand-central --disable-name-preservation --split-verilog --annotation-file %S/Wire.anno.json %s -o %t.folder > %t && cat %t.folder/MyView_companion.sv | FileCheck %s --check-prefixes MYVIEW_COMPANION
55

66
circuit Top :
77
module Submodule :

‎test/Dialect/FIRRTL/SFCTests/data-taps-nlas.fir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
; RUN: firtool --firrtl-grand-central --verilog %s | FileCheck %s
1+
; RUN: firtool --firrtl-grand-central --disable-name-preservation --verilog %s | FileCheck %s
22
; See https://github.com/llvm/circt/issues/2691
33

44
circuit Top : %[[{

‎test/Dialect/FIRRTL/SFCTests/signal-mappings.fir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
; RUN: firtool %s --annotation-file %S/signal-mappings-subCircuit.json --firrtl-grand-central --verilog | FileCheck %s
1+
; RUN: firtool %s --annotation-file %S/signal-mappings-subCircuit.json --disable-name-preservation --firrtl-grand-central --verilog | FileCheck %s
22

33
; Subcircuit:
44

‎test/Dialect/FIRRTL/SFCTests/width-spec-errors.fir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
; RUN: firtool --split-input-file --verify-diagnostics %s
1+
; RUN: firtool --disable-name-preservation --split-input-file --verify-diagnostics %s
22
; Tests extracted from:
33
; - test/scala/firrtlTests/WidthSpec.scala
44

‎test/firtool/name-preservation.fir‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
; RUN: firtool %s | FileCheck %s --check-prefixes=CHECK,NAMES
2+
; RUN: firtool %s -disable-name-preservation | FileCheck %s --check-prefixes=CHECK,NO_NAMES
3+
4+
circuit Foo:
5+
; CHECK-LABEL: module Foo
6+
module Foo:
7+
input a: {a: UInt<1>, flip b: UInt<1>}
8+
output b: {a: UInt<1>, flip b: UInt<1>}
9+
10+
; Unnamed wires are always removed.
11+
; CHECK-NOT: wire _x_a;
12+
; CHECK-NOT: wire _x_b;
13+
14+
wire _x: {a: UInt<1>, flip b: UInt<1>}
15+
_x <= a
16+
17+
; Default behavior is to preserve named wires.
18+
; NAMES: wire x_a;
19+
; NAMES: wire x_b;
20+
; With -disable-name-preservation, named wires are removed.
21+
; NO_NAMES-NOT: wire x_b;
22+
; NO_NAMES-NOT: wire x_a;
23+
wire x: {a: UInt<1>, flip b: UInt<1>}
24+
x <= _x
25+
26+
; Unnamed nodes are always removed.
27+
; CHECK-NOT: wire _y_a;
28+
node _y_a = x.a
29+
30+
; Default behavior is to preserve named nodes.
31+
; NAMES: wire y;
32+
; With -disable-name-preservation, named nodes are removed.
33+
; NO-NAMES-NOT: wire y;
34+
node y = _y_a
35+
36+
b.a <= y
37+
x.b <= b.b

‎tools/firtool/firtool.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,11 @@ static cl::opt<bool> disableAnnotationsUnknown(
102102
"disable-annotation-unknown",
103103
cl::desc("Ignore unknown annotations when parsing"), cl::init(false));
104104

105+
static cl::opt<bool> disableNamePreservation(
106+
"disable-name-preservation",
107+
cl::desc("Don't generate debug taps for named FIRRTL wires and nodes"),
108+
cl::init(false));
109+
105110
static cl::opt<bool>
106111
emitMetadata("emit-metadata",
107112
cl::desc("emit metadata for metadata annotations"),
@@ -381,6 +386,7 @@ processBuffer(MLIRContext &context, TimingScope &ts, llvm::SourceMgr &sourceMgr,
381386
options.ignoreInfoLocators = ignoreFIRLocations;
382387
options.rawAnnotations = newAnno;
383388
options.numAnnotationFiles = numAnnotationFiles;
389+
options.disableNamePreservation = disableNamePreservation;
384390
module = importFIRFile(sourceMgr, &context, options);
385391
} else {
386392
auto parserTimer = ts.nest("MLIR Parser");

0 commit comments

Comments
 (0)