From ab5a748b14284d2ff38c641c50ebfbb24f93356b Mon Sep 17 00:00:00 2001 From: Will Dietz Date: Tue, 11 Apr 2023 10:38:15 -0500 Subject: [PATCH] [FIRRTL][FIRParser] Parse RWProbe and rwprobe(). (#4972) With the IR pieces in place, add parsing support instead of faking it. * Parse RWProbe into firrtl.rwprobe . * Add `rwprobe()` support leveraging `Forceable`. Eagerly create declarations with the rwprobe ref result enabled, to ensure it's available if it's needed. Cleanup any declarations not forced (most of them) after parsing. --- lib/Dialect/FIRRTL/Import/FIRParser.cpp | 51 ++++++++++++++++++------- test/Dialect/FIRRTL/parse-basic.fir | 14 ++++--- 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/lib/Dialect/FIRRTL/Import/FIRParser.cpp b/lib/Dialect/FIRRTL/Import/FIRParser.cpp index e52877f81b0b..f178237d38e8 100644 --- a/lib/Dialect/FIRRTL/Import/FIRParser.cpp +++ b/lib/Dialect/FIRRTL/Import/FIRParser.cpp @@ -701,9 +701,7 @@ ParseResult FIRParser::parseType(FIRRTLType &result, const Twine &message) { parseToken(FIRToken::greater, "expected '>' in reference type")) return failure(); - if (kind == FIRToken::kw_RWProbe) - emitWarning(translateLocation(loc), - "RWProbe not yet supported, converting to Probe"); + bool forceable = kind == FIRToken::kw_RWProbe; auto innerType = dyn_cast(type); if (!innerType) // TODO: "innerType.containsReference()" @@ -712,7 +710,7 @@ ParseResult FIRParser::parseType(FIRRTLType &result, const Twine &message) { if (!innerType.isPassive()) return emitError(loc, "probe inner type must be passive"); - result = RefType::get(innerType); + result = RefType::get(innerType, forceable); break; } @@ -2458,7 +2456,19 @@ ParseResult FIRStmtParser::parseRWProbe(Value &result) { staticRef.getDefiningOp())) return emitError(startTok.getLoc(), "cannot probe memories or their ports"); - result = builder.create(staticRef); + // TODO: Support for non-public ports. + if (isa(staticRef)) + return emitError(startTok.getLoc(), "rwprobe of port not yet supported"); + auto *op = staticRef.getDefiningOp(); + if (!op) + return emitError(startTok.getLoc(), + "rwprobe value must be defined by an operation"); + auto forceable = dyn_cast(op); + if (!forceable || !forceable.isForceable() /* e.g., is/has const type*/) + return emitError(startTok.getLoc(), "rwprobe target not forceable") + .attachNote(op->getLoc()); + + result = forceable.getDataRef(); return success(); } @@ -2830,8 +2840,11 @@ ParseResult FIRStmtParser::parseNode() { auto annotations = getConstants().emptyArrayAttr; StringAttr sym = {}; - auto result = builder.create( - initializer, id, NameKindEnum::InterestingName, annotations, sym); + // TODO: isConst -> hasConst. + bool forceable = !isConst(initializer.getType()); + auto result = + builder.create(initializer, id, NameKindEnum::InterestingName, + annotations, sym, forceable); return moduleContext.addSymbolEntry(id, result.getResult(), startTok.getLoc()); } @@ -2857,8 +2870,10 @@ ParseResult FIRStmtParser::parseWire() { auto annotations = getConstants().emptyArrayAttr; StringAttr sym = {}; + // TODO: isConst -> hasConst. + bool forceable = !isConst(type); auto result = builder.create(type, id, NameKindEnum::InterestingName, - annotations, sym); + annotations, sym, forceable); return moduleContext.addSymbolEntry(id, result.getResult(), startTok.getLoc()); } @@ -2948,16 +2963,18 @@ ParseResult FIRStmtParser::parseRegister(unsigned regIndent) { ArrayAttr annotations = getConstants().emptyArrayAttr; Value result; StringAttr sym = {}; + // TODO: isConst -> hasConst. + bool forceable = !isConst(type); if (resetSignal) - result = - builder - .create(type, clock, resetSignal, resetValue, id, - NameKindEnum::InterestingName, annotations, sym) - .getResult(); + result = builder + .create(type, clock, resetSignal, resetValue, id, + NameKindEnum::InterestingName, annotations, + sym, forceable) + .getResult(); else result = builder .create(type, clock, id, NameKindEnum::InterestingName, - annotations, sym) + annotations, sym, forceable) .getResult(); return moduleContext.addSymbolEntry(id, result, startTok.getLoc()); } @@ -3436,6 +3453,12 @@ FIRCircuitParser::parseModuleBody(DeferredModuleToParse &deferredModule) { return result; } + // Demote any forceable operations that aren't being forced. + deferredModule.moduleOp.walk([](Forceable fop) { + if (fop.isForceable() && fop.getDataRef().use_empty()) + firrtl::detail::replaceWithNewForceability(fop, false); + }); + return success(); } diff --git a/test/Dialect/FIRRTL/parse-basic.fir b/test/Dialect/FIRRTL/parse-basic.fir index 3192254cb65d..c6cd32ac78d4 100644 --- a/test/Dialect/FIRRTL/parse-basic.fir +++ b/test/Dialect/FIRRTL/parse-basic.fir @@ -1157,26 +1157,27 @@ circuit MyModule : ; CHECK: firrtl.circuit "MyModule" { ; CHECK-LABEL: module private @RefsChild( ; CHECK-SAME: out %r: !firrtl.probe> - ; CHECK-SAME: out %rw: !firrtl.probe> + ; CHECK-SAME: out %rw: !firrtl.rwprobe> module RefsChild : input in : UInt<1> output r : Probe> - output rw : RWProbe> ; expected-warning {{RWProbe not yet supported}} + output rw : RWProbe> - ; CHECK-NEXT: %[[NODE:.+]] = firrtl.node + ; CHECK-NEXT: %[[NODE:.+]], %[[NODE_RWREF:.+]] = firrtl.node + ; CHECK-SAME: forceable node n = in ; CHECK-NEXT: %[[REF:.+]] = firrtl.ref.send %[[NODE]] ; CHECK-NEXT: ref.define %r, %[[REF]] define r = probe(n) - ; CHECK-NEXT: %[[RWREF:.+]] = firrtl.ref.send %[[NODE]] - ; CHECK-NEXT: ref.define %rw, %[[RWREF]] + ; CHECK-NOT: ref.send + ; CHECK-NEXT: ref.define %rw, %[[NODE_RWREF]] define rw = rwprobe(n) ; CHECK-LABEL: module private @Refs( module Refs : input in : UInt<1> output r : Probe> - output rw : RWProbe> ; expected-warning {{RWProbe not yet supported}} + output rw : RWProbe> output out : UInt<1> output out2 : UInt<1> output out3 : UInt<3> @@ -1190,6 +1191,7 @@ circuit MyModule : ; CHECK: firrtl.circuit "MyModule" { ; CHECK-NEXT: ref.define %r, %[[OUTREF]] define r = probe(out) ; CHECK-NEXT: ref.define %rw, %[[RC_RW]] + ; CHECK-SAME: rwprobe define rw = rc.rw ; CHECK-NEXT: %[[READ_RC_R:.+]] = firrtl.ref.resolve %[[RC_R]]