diff --git a/include/circt/Dialect/FIRRTL/FIRRTLDeclarations.td b/include/circt/Dialect/FIRRTL/FIRRTLDeclarations.td index d4bb8dc2494c..fd538979b78e 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLDeclarations.td +++ b/include/circt/Dialect/FIRRTL/FIRRTLDeclarations.td @@ -572,7 +572,10 @@ def RegResetOp : HardwareDeclOp<"regreset", [Forceable]> { let hasVerifier = 1; } -def WireOp : HardwareDeclOp<"wire", [Forceable]> { +def WireOp : HardwareDeclOp<"wire", [ + Forceable, + DeclareOpInterfaceMethods +]> { let summary = "Define a new wire"; let description = [{ Declare a new wire: diff --git a/include/circt/Dialect/FIRRTL/FIRRTLExpressions.td b/include/circt/Dialect/FIRRTL/FIRRTLExpressions.td index ea80c82ef339..7918c5cfca72 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLExpressions.td +++ b/include/circt/Dialect/FIRRTL/FIRRTLExpressions.td @@ -1220,6 +1220,7 @@ def RefCastOp : FIRRTLOp<"ref.cast", let results = (outs RefType:$result); let hasFolder = 1; + let hasVerifier = 1; let assemblyFormat = "$input attr-dict `:` functional-type($input, $result)"; @@ -1238,6 +1239,7 @@ def RefResolveOp: FIRRTLExprOp<"ref.resolve", let results = (outs FIRRTLBaseType:$result); let hasCanonicalizer = true; + let hasVerifier = 1; let assemblyFormat = "$ref attr-dict `:` qualified(type($ref))"; } diff --git a/lib/Dialect/FIRRTL/FIRRTLOps.cpp b/lib/Dialect/FIRRTL/FIRRTLOps.cpp index ac742f2fbbf2..73e4014bff19 100644 --- a/lib/Dialect/FIRRTL/FIRRTLOps.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLOps.cpp @@ -1448,28 +1448,55 @@ LogicalResult FIntModuleOp::verify() { return success(); } +static LogicalResult verifyProbeType(RefType refType, Location loc, + CircuitOp circuitOp, + SymbolTableCollection &symbolTable, + Twine start) { + auto layer = refType.getLayer(); + if (!layer) + return success(); + auto *layerOp = symbolTable.lookupSymbolIn(circuitOp, layer); + if (!layerOp) + return emitError(loc) << start << " associated with layer '" << layer + << "', but this layer was not defined"; + if (!isa(layerOp)) { + auto diag = emitError(loc) + << start << " associated with layer '" << layer + << "', but symbol '" << layer << "' does not refer to a '" + << LayerOp::getOperationName() << "' op"; + return diag.attachNote(layerOp->getLoc()) << "symbol refers to this op"; + } + return success(); +} + static LogicalResult verifyPortSymbolUses(FModuleLike module, SymbolTableCollection &symbolTable) { - auto circuitOp = module->getParentOfType(); - // verify types in ports. + auto circuitOp = module->getParentOfType(); for (size_t i = 0, e = module.getNumPorts(); i < e; ++i) { auto type = module.getPortType(i); - auto classType = dyn_cast(type); - if (!classType) + + if (auto refType = type_dyn_cast(type)) { + if (failed(verifyProbeType( + refType, module.getPortLocation(i), circuitOp, symbolTable, + Twine("probe port '") + module.getPortName(i) + "' is"))) + return failure(); continue; + } - // verify that the class exists. - auto className = classType.getNameAttr(); - auto classOp = dyn_cast_or_null( - symbolTable.lookupSymbolIn(circuitOp, className)); - if (!classOp) - return module.emitOpError() << "references unknown class " << className; + if (auto classType = dyn_cast(type)) { + auto className = classType.getNameAttr(); + auto classOp = dyn_cast_or_null( + symbolTable.lookupSymbolIn(circuitOp, className)); + if (!classOp) + return module.emitOpError() << "references unknown class " << className; - // verify that the result type agrees with the class definition. - if (failed(classOp.verifyType(classType, - [&]() { return module.emitOpError(); }))) - return failure(); + // verify that the result type agrees with the class definition. + if (failed(classOp.verifyType(classType, + [&]() { return module.emitOpError(); }))) + return failure(); + continue; + } } return success(); @@ -3045,6 +3072,16 @@ void WireOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) { std::optional WireOp::getTargetResultIndex() { return 0; } +LogicalResult WireOp::verifySymbolUses(SymbolTableCollection &symbolTable) { + auto refType = type_dyn_cast(getType(0)); + if (!refType) + return success(); + + return verifyProbeType( + refType, getLoc(), getOperation()->getParentOfType(), + symbolTable, Twine("'") + getOperationName() + "' op is"); +} + void ObjectOp::build(OpBuilder &builder, OperationState &state, ClassLike klass, StringRef name) { build(builder, state, klass.getInstanceType(), @@ -3366,6 +3403,41 @@ LogicalResult RefDefineOp::verify() { "destination reference cannot be a cast of another reference"); } + SymbolRefAttr layer; + auto layerBlockOp = (*this)->getParentOfType(); + if (layerBlockOp) + layer = layerBlockOp.getLayerName(); + + // The dest layer must be the same as the source layer or a parent of it. + SymbolRefAttr layerPointer = layer; + auto destLayer = getDest().getType().getLayer(); + for (;;) { + if (layerPointer == destLayer) + break; + + if (!layerPointer) { + if (!layer) + return emitOpError() + << "defines to a layer-colored probe from outside a layerblock"; + auto diag = emitOpError() + << "defines to a probe colored with layer '" << destLayer + << "' from a layerblock associated with layer '" << layer + << "'. The define op must be in a layerblock associated with " + "or a child of layer '" + << destLayer << "'."; + return diag.attachNote(layerBlockOp.getLoc()) + << "the layerblock was declared here"; + } + + if (layerPointer.getNestedReferences().empty()) { + layerPointer = {}; + continue; + } + layerPointer = + SymbolRefAttr::get(layerPointer.getRootReference(), + layerPointer.getNestedReferences().drop_back()); + } + return success(); } @@ -5678,7 +5750,7 @@ FIRRTLType RefSubOp::inferReturnType(ValueRange operands, return RefType::get( vectorType.getElementType().getConstType( vectorType.isConst() || vectorType.getElementType().isConst()), - refType.getForceable()); + refType.getForceable(), refType.getLayer()); return emitInferRetTypeError(loc, "out of range index '", fieldIdx, "' in RefType of vector type ", refType); } @@ -5691,13 +5763,93 @@ FIRRTLType RefSubOp::inferReturnType(ValueRange operands, return RefType::get(bundleType.getElement(fieldIdx).type.getConstType( bundleType.isConst() || bundleType.getElement(fieldIdx).type.isConst()), - refType.getForceable()); + refType.getForceable(), refType.getLayer()); } return emitInferRetTypeError( loc, "ref.sub op requires a RefType of vector or bundle base type"); } +LogicalResult RefCastOp::verify() { + + SymbolRefAttr layer; + auto layerBlockOp = (*this)->getParentOfType(); + if (layerBlockOp) + layer = layerBlockOp.getLayerName(); + + // The dest layer must be the same as the source layer or a parent of it. + SymbolRefAttr layerPointer = layer; + auto destLayer = getType().getLayer(); + for (;;) { + if (layerPointer == destLayer) + break; + + if (!layerPointer) { + if (!layer) + return emitOpError() + << "cannot cast to a layer from outside a layerblock"; + auto diag = emitOpError() + << "casts to a probe associated with layer '" << destLayer + << "' from a layerblock associated with layer '" << layer + << "'. The cast op must be in a layerblock associated with " + "or a child of layer '" + << destLayer << "'."; + return diag.attachNote(layerBlockOp.getLoc()) + << "the layerblock was declared here"; + } + + if (layerPointer.getNestedReferences().empty()) { + layerPointer = {}; + continue; + } + layerPointer = + SymbolRefAttr::get(layerPointer.getRootReference(), + layerPointer.getNestedReferences().drop_back()); + } + + return success(); +} + +LogicalResult RefResolveOp::verify() { + + SymbolRefAttr layer; + auto layerBlockOp = (*this)->getParentOfType(); + if (layerBlockOp) + layer = layerBlockOp.getLayerName(); + + // The dest layer must be the same as the source layer or a parent of it. + SymbolRefAttr layerPointer = layer; + auto destLayer = getRef().getType().getLayer(); + for (;;) { + if (layerPointer == destLayer) + break; + + if (!layerPointer) { + if (!layer) + return emitOpError() << "cannot read from a layer-colored probe from " + "outside a layerblock"; + auto diag = emitOpError() + << "reads from a probe colored with layer '" << destLayer + << "' from a layerblock associated with layer '" << layer + << "'. The resolve op must be in a layerblock associated " + "with or a child of layer '" + << destLayer << "'."; + return diag.attachNote(layerBlockOp.getLoc()) + << "the layerblock was declared here"; + } + + if (layerPointer.getNestedReferences().empty()) { + layerPointer = {}; + continue; + } + layerPointer = + SymbolRefAttr::get(layerPointer.getRootReference(), + layerPointer.getNestedReferences().drop_back()); + } + + return success(); +} + LogicalResult RWProbeOp::verifyInnerRefs(hw::InnerRefNamespace &ns) { auto targetRef = getTarget(); if (targetRef.getModule() != @@ -5793,16 +5945,10 @@ LogicalResult LayerBlockOp::verify() { // Any value captured from the current layer block is fine. if (operand.getParentBlock() == body) continue; - // Capture of a non-base type, e.g., reference is illegal. + // Capture of a non-base type, e.g., reference, is allowed. FIRRTLBaseType baseType = dyn_cast(operand.getType()); - if (!baseType) { - auto diag = emitOpError() - << "captures an operand which is not a FIRRTL base type"; - diag.attachNote(operand.getLoc()) << "operand is defined here"; - diag.attachNote(op->getLoc()) << "operand is used here"; - failed = true; + if (!baseType) return WalkResult::advance(); - } // Capturing a non-passive type is illegal. if (!baseType.isPassive()) { auto diag = emitOpError() diff --git a/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp b/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp index ddaa1a69b9f1..aa3e43f3b982 100644 --- a/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp +++ b/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp @@ -1432,7 +1432,10 @@ bool TypeLoweringVisitor::visitExpr(RefCastOp op) { auto clone = [&](const FlatBundleFieldEntry &field, ArrayAttr attrs) -> Value { auto input = getSubWhatever(op.getInput(), field.index); - return builder->create(RefType::get(field.type), input); + return builder->create(RefType::get(field.type, + op.getType().getForceable(), + op.getType().getLayer()), + input); }; return lowerProducer(op, clone); } diff --git a/test/Dialect/FIRRTL/errors.mlir b/test/Dialect/FIRRTL/errors.mlir index f407fe861e71..b694a408e4d1 100644 --- a/test/Dialect/FIRRTL/errors.mlir +++ b/test/Dialect/FIRRTL/errors.mlir @@ -1869,22 +1869,6 @@ firrtl.circuit "WrongLayerBlockNesting" { // ----- -// A layer block captures a type which is not a FIRRTL base type. -firrtl.circuit "NonBaseTypeCapture" { - firrtl.layer @A bind {} - firrtl.module @NonBaseTypeCapture(in %in: !firrtl.uint<1>) { - // expected-note @below {{operand is defined here}} - %ref = firrtl.ref.send %in : !firrtl.uint<1> - // expected-error @below {{'firrtl.layerblock' op captures an operand which is not a FIRRTL base type}} - firrtl.layerblock @A { - // expected-note @below {{operand is used here}} - %b = firrtl.ref.resolve %ref : !firrtl.probe> - } - } -} - -// ----- - // A layer block captures a non-passive type. firrtl.circuit "NonPassiveCapture" { firrtl.layer @A bind {} @@ -1922,6 +1906,114 @@ firrtl.circuit "LayerBlockDrivesSinksOutside" { // ----- +firrtl.circuit "IllegalRefDefine" { + firrtl.layer @A bind {} + firrtl.module @Bar(out %a: !firrtl.probe, @A>) {} + firrtl.module @IllegalRefDefine(out %a: !firrtl.probe, @A>) { + %bar_a = firrtl.instance bar interesting_name @Bar(out a: !firrtl.probe, @A>) + // expected-error @below {{defines to a layer-colored probe from outside a layerblock}} + firrtl.ref.define %a, %bar_a : !firrtl.probe, @A> + } +} + +// ----- + +firrtl.circuit "IllegalRefCastDestModule" { + firrtl.module @IllegalRefCastDestModule() { + %a = firrtl.wire : !firrtl.uint<1> + %0 = firrtl.ref.send %a : !firrtl.uint<1> + // expected-error @below {{cannot cast to a layer from outside a layerblock}} + %1 = firrtl.ref.cast %0 : (!firrtl.probe>) -> !firrtl.probe, @A> + } +} + +// ----- + +firrtl.circuit "IllegalRefCastDestLayerBlock" { + firrtl.layer @A bind { + } + firrtl.layer @B bind { + } + firrtl.module @IllegalRefCastDestLayerBlock() { + // expected-note @below {{the layerblock was declared here}} + firrtl.layerblock @A { + %a = firrtl.wire : !firrtl.uint<1> + %0 = firrtl.ref.send %a : !firrtl.uint<1> + // expected-error @below {{'firrtl.ref.cast' op casts to a probe associated with layer '@B' from a layerblock associated with layer '@A'.}} + %1 = firrtl.ref.cast %0 : (!firrtl.probe>) -> !firrtl.probe, @B> + } + } +} + +// ----- + +firrtl.circuit "IllegalRefResolve_NotInLayerBlock" { + firrtl.layer @A bind { + } + firrtl.module @IllegalRefResolve_NotInLayerBlock() { + %0 = firrtl.wire : !firrtl.probe, @A> + // expected-error @below {{'firrtl.ref.resolve' op cannot read from a layer-colored probe from outside a layerblock}} + %1 = firrtl.ref.resolve %0 : !firrtl.probe, @A> + } +} + +// ----- + +firrtl.circuit "IllegalRefResolve_IllegalLayer" { + firrtl.layer @A bind { + } + firrtl.layer @B bind { + } + firrtl.module @IllegalRefResolve_IllegalLayer() { + %0 = firrtl.wire : !firrtl.probe, @A> + // expected-note @below {{the layerblock was declared here}} + firrtl.layerblock @B { + // expected-error @below {{'firrtl.ref.resolve' op reads from a probe colored with layer '@A' from a layerblock associated with layer '@B'. The resolve op must be in a layerblock associated with or a child of layer '@A'.}} + %1 = firrtl.ref.resolve %0 : !firrtl.probe, @A> + } + } +} + +// ----- + +firrtl.circuit "InvalidProbeAssociationPort_SymbolDoesNotExist" { + // expected-error @below {{probe port 'a' is associated with layer '@B', but this layer was not defined}} + firrtl.module @InvalidProbeAssociationPort_SymbolDoesNotExist(out %a: !firrtl.probe, @B>) { + } +} + +// ----- + +firrtl.circuit "InvalidProbeAssociationPort_SymbolIsNotALayer" { + // expected-note @below {{symbol refers to this op}} + firrtl.module @B() {} + // expected-error @below {{probe port 'a' is associated with layer '@B', but symbol '@B' does not refer to a 'firrtl.layer' op}} + firrtl.module @InvalidProbeAssociationPort_SymbolIsNotALayer(out %a: !firrtl.probe, @B>) { + } +} + +// ----- + +firrtl.circuit "InvalidProbeAssociationWire_SymbolDoesNotExist" { + firrtl.module @InvalidProbeAssociationWire_SymbolDoesNotExist() { + // expected-error @below {{'firrtl.wire' op is associated with layer '@B', but this layer was not defined}} + %a = firrtl.wire : !firrtl.probe, @B> + } +} + +// ----- + +firrtl.circuit "InvalidProbeAssociationWire_SymbolIsNotALayer" { + // expected-note @below {{symbol refers to this op}} + firrtl.module @B() {} + firrtl.module @InvalidProbeAssociationWire_SymbolIsNotALayer() { + // expected-error @below {{'firrtl.wire' op is associated with layer '@B', but symbol '@B' does not refer to a 'firrtl.layer' op}} + %a = firrtl.wire : !firrtl.probe, @B> + } +} + +// ----- + firrtl.circuit "RWProbeRemote" { firrtl.module @Other() { %w = firrtl.wire sym @x : !firrtl.uint<1>