Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIRRTL][ProbesToSignals] RWProbe support #7706

Merged
merged 1 commit into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 84 additions & 12 deletions lib/Dialect/FIRRTL/Transforms/ProbesToSignals.cpp
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
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
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>>
}
}

Loading