Skip to content

Commit

Permalink
[FIRRTL][ProbesToSignals] RWProbe support (#7706)
Browse files Browse the repository at this point in the history 
* Define type conversion mapping rwprobe<T> -> passive(T)
* Forceable support (passive read of data result)
* RWProbeOp support (materialize target, to passive)
* Reject {force,release}{,_initial}

Add some basic tests.
  • Loading branch information
@dtzSiFive
dtzSiFive authored Oct 15, 2024
1 parent dce37cc commit 5a6a561
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 35 deletions.
96 changes: 84 additions & 12 deletions lib/Dialect/FIRRTL/Transforms/ProbesToSignals.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
// * Inference passes, especially width inference. Probes infer slightly
// differently than non-probes do (must have same width along the chain).
//
// Forceable and colored probes are not supported.
// Colored probes are not supported.
// Specialize layers on or off to remove colored probes first.
//
// Debug ports on FIRRTL memories are not currently supported,
Expand Down Expand Up @@ -66,7 +66,7 @@ namespace {

class ProbeVisitor : public FIRRTLVisitor<ProbeVisitor, LogicalResult> {
public:
ProbeVisitor() = default;
ProbeVisitor(hw::InnerRefNamespace &irn) : irn(irn) {}

/// Entrypoint.
LogicalResult visit(FModuleLike mod);
Expand All @@ -91,9 +91,6 @@ class ProbeVisitor : public FIRRTLVisitor<ProbeVisitor, LogicalResult> {
if (!refType)
return Type();

if (refType.getForceable())
return err("rwprobe not supported");

if (refType.getLayer())
return err("layer-colored probes not supported");

Expand Down Expand Up @@ -141,39 +138,58 @@ class ProbeVisitor : public FIRRTLVisitor<ProbeVisitor, LogicalResult> {

/// Check declarations specifically before forwarding to unhandled.
LogicalResult visitUnhandledDecl(Operation *op) {
// Check for and reject forceable declarations explicitly.
// Check for and handle active forceable declarations.
if (auto fop = dyn_cast<Forceable>(op); fop && fop.isForceable())
return fop.emitError("forceable declaration not supported");
return visitActiveForceableDecl(fop);
return visitUnhandledOp(op);
}

// Declarations

LogicalResult visitDecl(MemOp op);
LogicalResult visitDecl(WireOp op);
LogicalResult visitActiveForceableDecl(Forceable fop);

LogicalResult visitInstanceLike(Operation *op);
LogicalResult visitDecl(InstanceOp op) { return visitInstanceLike(op); }
LogicalResult visitDecl(InstanceChoiceOp op) { return visitInstanceLike(op); }

// Probe operations.

LogicalResult visitExpr(RWProbeOp op) {
return op.emitError("rwprobe not supported");
}
LogicalResult visitExpr(RWProbeOp op);
LogicalResult visitExpr(RefCastOp op);
LogicalResult visitExpr(RefResolveOp op);
LogicalResult visitExpr(RefSendOp op);
LogicalResult visitExpr(RefSubOp op);

LogicalResult visitStmt(RefDefineOp op);

// Force and release operations: reject as unsupported.
LogicalResult visitStmt(RefForceOp op) {
return op.emitError("force not supported");
}
LogicalResult visitStmt(RefForceInitialOp op) {
return op.emitError("force_initial not supported");
}
LogicalResult visitStmt(RefReleaseOp op) {
return op.emitError("release not supported");
}
LogicalResult visitStmt(RefReleaseInitialOp op) {
return op.emitError("release_initial not supported");
}

private:
/// Map from probe-typed Value's to their non-probe equivalent.
DenseMap<Value, Value> probeToHWMap;

/// Forceable operations to demote.
SmallVector<Forceable> forceables;

/// Operations to delete.
SmallVector<Operation *> toDelete;

/// Read-only copy of inner-ref namespace for resolving inner refs.
hw::InnerRefNamespace &irn;
};

} // end namespace
Expand Down Expand Up @@ -260,6 +276,10 @@ LogicalResult ProbeVisitor::visit(FModuleLike mod) {
for (auto *op : llvm::reverse(toDelete))
op->erase();

// Demote forceable's.
for (auto fop : forceables)
firrtl::detail::replaceWithNewForceability(fop, false);

return success();
}

Expand Down Expand Up @@ -384,7 +404,7 @@ LogicalResult ProbeVisitor::visitDecl(MemOp op) {

LogicalResult ProbeVisitor::visitDecl(WireOp op) {
if (op.isForceable())
return op.emitError("forceable declaration not supported");
return visitActiveForceableDecl(op);

auto conv = convertType(op.getDataRaw().getType(), op.getLoc());
if (failed(conv))
Expand All @@ -402,6 +422,28 @@ LogicalResult ProbeVisitor::visitDecl(WireOp op) {
return success();
}

LogicalResult ProbeVisitor::visitActiveForceableDecl(Forceable fop) {
assert(fop.isForceable() && "must be called on active forceables");
// Map rw ref result to normal result.
auto data = fop.getData();
auto conv = mapType(fop.getDataRef().getType(), fop.getLoc());
if (failed(conv))
return failure();
auto newType = *conv;
forceables.push_back(fop);

assert(newType == data.getType().getPassiveType());
if (newType != data.getType()) {
ImplicitLocOpBuilder builder(fop.getLoc(), fop);
builder.setInsertionPointAfterValue(data);
auto wire = builder.create<WireOp>(newType);
emitConnect(builder, wire.getData(), data);
data = wire.getData();
}
probeToHWMap[fop.getDataRef()] = data;
return success();
}

LogicalResult ProbeVisitor::visitInstanceLike(Operation *op) {
SmallVector<Type> newTypes;
auto needsConv = mapRange(op->getResultTypes(), op->getLoc(), newTypes);
Expand Down Expand Up @@ -461,6 +503,33 @@ LogicalResult ProbeVisitor::visitStmt(RefDefineOp op) {
return success();
}

LogicalResult ProbeVisitor::visitExpr(RWProbeOp op) {
// Handle similar to ref.send but lookup the target
// and materialize a value for it (indexing).
auto conv = mapType(op.getType(), op.getLoc());
if (failed(conv))
return failure();
auto newType = *conv;
toDelete.push_back(op);

auto ist = irn.lookup(op.getTarget());
assert(ist);
auto ref = getFieldRefForTarget(ist);

ImplicitLocOpBuilder builder(op.getLoc(), op);
builder.setInsertionPointAfterValue(ref.getValue());
auto data = getValueByFieldID(builder, ref.getValue(), ref.getFieldID());
assert(cast<FIRRTLBaseType>(data.getType()).getPassiveType() ==
op.getType().getType());
if (newType != data.getType()) {
auto wire = builder.create<WireOp>(newType);
emitConnect(builder, wire.getData(), data);
data = wire.getData();
}
probeToHWMap[op.getResult()] = data;
return success();
}

LogicalResult ProbeVisitor::visitExpr(RefCastOp op) {
auto input = probeToHWMap.at(op.getInput());
// Insert wire of the new type, and connect to it.
Expand Down Expand Up @@ -546,8 +615,11 @@ void ProbesToSignalsPass::runOnOperation() {

SmallVector<Operation *, 0> ops(getOperation().getOps<FModuleLike>());

hw::InnerRefNamespace irn{getAnalysis<SymbolTable>(),
getAnalysis<hw::InnerSymbolTableCollection>()};

auto result = failableParallelForEach(&getContext(), ops, [&](Operation *op) {
ProbeVisitor visitor;
ProbeVisitor visitor(irn);
return visitor.visit(cast<FModuleLike>(op));
});

Expand Down
28 changes: 6 additions & 22 deletions test/Dialect/FIRRTL/probes-to-signals-errors.mlir
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,5 @@
// RUN: circt-opt --firrtl-probes-to-signals --verify-diagnostics --split-input-file %s

// CHECK-LABEL: "ProbeAndRWProbe"
firrtl.circuit "ProbeAndRWProbe" {
// expected-error @below {{rwprobe not supported, cannot convert type '!firrtl.rwprobe<uint<2>>'}}
firrtl.extmodule private @Probes(out ro : !firrtl.probe<uint<1>>, out rw : !firrtl.rwprobe<uint<2>>)
firrtl.module @ProbeAndRWProbe() {}
}

// -----

firrtl.circuit "InternalPath" {
// expected-error @below {{cannot convert module with internal path}}
firrtl.extmodule @InternalPath(
Expand Down Expand Up @@ -37,20 +28,13 @@ firrtl.circuit "RefProducer" {

// -----

firrtl.circuit "Forceable" {
firrtl.module @Forceable() {
// expected-error @below {{forceable declaration not supported}}
%w, %w_f = firrtl.wire forceable : !firrtl.uint<2>, !firrtl.rwprobe<uint<2>>
}
}

// -----

firrtl.circuit "RWProbeOp" {
firrtl.module @RWProbeOp() {
firrtl.circuit "RejectForce" {
firrtl.module @RejectForce(in %clock: !firrtl.clock, in %val : !firrtl.uint<2>, out %p : !firrtl.rwprobe<uint<2>>) {
%w = firrtl.wire sym @sym : !firrtl.uint<2>
// expected-error @below {{rwprobe not supported}}
%rwprobe = firrtl.ref.rwprobe <@RWProbeOp::@sym> : !firrtl.rwprobe<uint<2>>
%rwprobe = firrtl.ref.rwprobe <@RejectForce::@sym> : !firrtl.rwprobe<uint<2>>
%c1_ui1 = firrtl.constant 1 : !firrtl.const.uint<1>
// expected-error @below {{force not supported}}
firrtl.ref.force %clock, %c1_ui1, %rwprobe, %val : !firrtl.clock, !firrtl.const.uint<1>, !firrtl.rwprobe<uint<2>>, !firrtl.uint<2>
}
}

Expand Down
63 changes: 62 additions & 1 deletion test/Dialect/FIRRTL/probes-to-signals.mlir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ firrtl.circuit "TestP" {

// -----

// Extmodule
// Extmodule using alias

// CHECK-LABEL: "ExtModule"
firrtl.circuit "ExtModule" {
Expand All @@ -75,6 +75,17 @@ firrtl.circuit "ExtModule" {

// -----

// Extmodule using rwprobe

// CHECK-LABEL: "ExtModuleRW"
firrtl.circuit "ExtModuleRW" {
// CHECK: out ro: !firrtl.uint<1>
// CHECK-SAME: out rw: !firrtl.uint<2>
firrtl.extmodule @ExtModuleRW(out ro: !firrtl.probe<uint<1>>, out rw: !firrtl.rwprobe<uint<2>>)
}

// -----

// CHIRRTL debug port

// CHECK-LABEL: "DbgsMemPort"
Expand Down Expand Up @@ -109,3 +120,53 @@ firrtl.circuit "DbgsMemPort" {
// CHECK: matchingconnect %_a, %[[W]]
}
}

// -----
// RWProbe exported out top should be supported.

// CHECK-LABEL: "ForceableRWProbeExport"
firrtl.circuit "ForceableRWProbeExport" {
// CHECK: @ForceableRWProbeExport(out %p: !firrtl.uint<2>)
firrtl.module @ForceableRWProbeExport(out %p : !firrtl.rwprobe<uint<2>>) {
// CHECK-NEXT: %w = firrtl.wire : !firrtl.uint<2>
// CHECK-NEXT: firrtl.matchingconnect %p, %w
// CHECK-NEXT: }
%w, %w_f = firrtl.wire forceable : !firrtl.uint<2>, !firrtl.rwprobe<uint<2>>
firrtl.ref.define %p, %w_f : !firrtl.rwprobe<uint<2>>
}
}

// -----

// Test use of forceable + rwprobe + read works.
// Forceable is handled by using passive copy of data result.

// CHECK-LABEL: "ForceableToRead"
firrtl.circuit "ForceableToRead" {
// CHECK: @ForceableToRead(
firrtl.module @ForceableToRead(out %r : !firrtl.uint<2>) {
// CHECK-NEXT: %w = firrtl.wire : !firrtl.uint<2>
// CHECK-NEXT: firrtl.matchingconnect %r, %w
%w, %w_f = firrtl.wire forceable : !firrtl.uint<2>, !firrtl.rwprobe<uint<2>>
%data = firrtl.ref.resolve %w_f : !firrtl.rwprobe<uint<2>>
firrtl.matchingconnect %r, %data : !firrtl.uint<2>
}
}

// -----

// Check rwprobe operation.

// CHECK-LABEL: "RWProbeOp"
firrtl.circuit "RWProbeOp" {
// CHECK: @RWProbeOp(out %p: !firrtl.uint<2>)
firrtl.module @RWProbeOp(out %p: !firrtl.rwprobe<uint<2>>) {
// CHECK-NEXT: %w = firrtl.wire sym @sym
// CHECK-NEXT: firrtl.matchingconnect %p, %w
// CHECK-NEXT: }
%w = firrtl.wire sym @sym : !firrtl.uint<2>
%rwprobe = firrtl.ref.rwprobe <@RWProbeOp::@sym> : !firrtl.rwprobe<uint<2>>
firrtl.ref.define %p, %rwprobe : !firrtl.rwprobe<uint<2>>
}
}

0 comments on commit 5a6a561

Please sign in to comment.