Skip to content

Commit

Permalink
[FIRRTL][FIRParser] Parse RWProbe<T> and rwprobe(). (#4972)
Browse files Browse the repository at this point in the history 
With the IR pieces in place, add parsing support instead of faking it.

* Parse RWProbe into firrtl.rwprobe<T> .
* 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.
  • Loading branch information
@dtzSiFive
dtzSiFive authored Apr 11, 2023
1 parent 72adc14 commit ab5a748
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 20 deletions.
51 changes: 37 additions & 14 deletions lib/Dialect/FIRRTL/Import/FIRParser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<FIRRTLBaseType>(type);
if (!innerType) // TODO: "innerType.containsReference()"
Expand All @@ -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;
}

Expand Down Expand Up @@ -2458,7 +2456,19 @@ ParseResult FIRStmtParser::parseRWProbe(Value &result) {
staticRef.getDefiningOp()))
return emitError(startTok.getLoc(), "cannot probe memories or their ports");

result = builder.create<RefSendOp>(staticRef);
// TODO: Support for non-public ports.
if (isa<BlockArgument>(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<Forceable>(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();
}
Expand Down Expand Up @@ -2830,8 +2840,11 @@ ParseResult FIRStmtParser::parseNode() {

auto annotations = getConstants().emptyArrayAttr;
StringAttr sym = {};
auto result = builder.create<NodeOp>(
initializer, id, NameKindEnum::InterestingName, annotations, sym);
// TODO: isConst -> hasConst.
bool forceable = !isConst(initializer.getType());
auto result =
builder.create<NodeOp>(initializer, id, NameKindEnum::InterestingName,
annotations, sym, forceable);
return moduleContext.addSymbolEntry(id, result.getResult(),
startTok.getLoc());
}
Expand All @@ -2857,8 +2870,10 @@ ParseResult FIRStmtParser::parseWire() {
auto annotations = getConstants().emptyArrayAttr;
StringAttr sym = {};

// TODO: isConst -> hasConst.
bool forceable = !isConst(type);
auto result = builder.create<WireOp>(type, id, NameKindEnum::InterestingName,
annotations, sym);
annotations, sym, forceable);
return moduleContext.addSymbolEntry(id, result.getResult(),
startTok.getLoc());
}
Expand Down Expand Up @@ -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<RegResetOp>(type, clock, resetSignal, resetValue, id,
NameKindEnum::InterestingName, annotations, sym)
.getResult();
result = builder
.create<RegResetOp>(type, clock, resetSignal, resetValue, id,
NameKindEnum::InterestingName, annotations,
sym, forceable)
.getResult();
else
result = builder
.create<RegOp>(type, clock, id, NameKindEnum::InterestingName,
annotations, sym)
annotations, sym, forceable)
.getResult();
return moduleContext.addSymbolEntry(id, result, startTok.getLoc());
}
Expand Down Expand Up @@ -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();
}

Expand Down
14 changes: 8 additions & 6 deletions test/Dialect/FIRRTL/parse-basic.fir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1157,26 +1157,27 @@ circuit MyModule : ; CHECK: firrtl.circuit "MyModule" {

; CHECK-LABEL: module private @RefsChild(
; CHECK-SAME: out %r: !firrtl.probe<uint<1>>
; CHECK-SAME: out %rw: !firrtl.probe<uint<1>>
; CHECK-SAME: out %rw: !firrtl.rwprobe<uint<1>>
module RefsChild :
input in : UInt<1>
output r : Probe<UInt<1>>
output rw : RWProbe<UInt<1>> ; expected-warning {{RWProbe not yet supported}}
output rw : RWProbe<UInt<1>>

; 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<UInt<1>>
output rw : RWProbe<UInt<1>> ; expected-warning {{RWProbe not yet supported}}
output rw : RWProbe<UInt<1>>
output out : UInt<1>
output out2 : UInt<1>
output out3 : UInt<3>
Expand All @@ -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]]
Expand Down

0 comments on commit ab5a748

Please sign in to comment.