diff --git a/lib/Conversion/ImportVerilog/HierarchicalNames.cpp b/lib/Conversion/ImportVerilog/HierarchicalNames.cpp index 97ef8399768e..d4bdd09e9fa5 100644 --- a/lib/Conversion/ImportVerilog/HierarchicalNames.cpp +++ b/lib/Conversion/ImportVerilog/HierarchicalNames.cpp @@ -43,6 +43,14 @@ struct InstBodyVisitor if (currentInstBody == outermostInstBody) return; + // References resolved via an interface port (e.g. `bus.member` where `bus` + // is an `Iface.modport` port) are not cross-instance hierarchical accesses; + // they are handled by the interface port lowering machinery in + // Structure.cpp. Recording them here would add a spurious hierPath input + // to the module signature that nothing fills in at the instance site. + if (expr.ref.isViaIfacePort()) + return; + auto hierName = builder.getStringAttr(expr.symbol.name); const slang::ast::InstanceBodySymbol *parentInstBody = nullptr; diff --git a/lib/Conversion/ImportVerilog/ImportVerilogInternals.h b/lib/Conversion/ImportVerilog/ImportVerilogInternals.h index 7b440bfb20dd..f60d80daa59d 100644 --- a/lib/Conversion/ImportVerilog/ImportVerilogInternals.h +++ b/lib/Conversion/ImportVerilog/ImportVerilogInternals.h @@ -62,6 +62,12 @@ struct FlattenedIfacePort { /// The connected interface instance backing this port (if any). This enables /// materializing virtual interface handles from interface ports. const slang::ast::InstanceSymbol *ifaceInstance = nullptr; + /// For modport-typed iface ports, the ModportPortSymbol this was flattened + /// from. Slang resolves in-body accesses like `bus.member` to a + /// `HierarchicalValueExpression` whose symbol is the `ModportPortSymbol`, + /// not the underlying interface body variable, so we register both keys in + /// `valueSymbols` to make the lookup find this port's `BlockArgument`. + const slang::ast::Symbol *modportPortSym = nullptr; }; /// Lowering information for an expanded interface instance. Maps each interface diff --git a/lib/Conversion/ImportVerilog/Structure.cpp b/lib/Conversion/ImportVerilog/Structure.cpp index d815016b3306..5a32d930f712 100644 --- a/lib/Conversion/ImportVerilog/Structure.cpp +++ b/lib/Conversion/ImportVerilog/Structure.cpp @@ -1119,7 +1119,7 @@ Context::convertModuleHeader(const slang::ast::InstanceBodySymbol *module) { } lowering.ifacePorts.push_back({name, dir, type, portLoc, arg, &ifacePort, mpp->internalSymbol, - ifaceInst}); + ifaceInst, mpp}); } } else { // No modport: iterate interface body for all variables and nets. @@ -1291,18 +1291,26 @@ Context::convertModuleBody(const slang::ast::InstanceBodySymbol *module) { if (!valueSym) continue; + Value portValue; if (fp.direction == hw::ModulePort::Output) { // Output interface ports are not referenceable within the module body. // Create internal variables for them and return their value through the // module terminator. - auto varOp = moore::VariableOp::create( + portValue = moore::VariableOp::create( builder, fp.loc, moore::RefType::get(cast(fp.type)), fp.name, Value()); - valueSymbols.insert(valueSym, varOp); } else { - valueSymbols.insert(valueSym, fp.arg); + portValue = fp.arg; } + valueSymbols.insert(valueSym, portValue); + // Slang resolves in-body accesses (e.g. `bus.r`) through the + // ModportPortSymbol rather than the interface body's variable. Register + // both so the body-level expression lookup finds this port. + if (fp.modportPortSym) + if (auto *mppSym = fp.modportPortSym->as_if()) + if (mppSym != valueSym) + valueSymbols.insert(mppSym, portValue); if (!fp.ifaceInstance) continue; diff --git a/test/Conversion/ImportVerilog/interface-modport-body-access.sv b/test/Conversion/ImportVerilog/interface-modport-body-access.sv new file mode 100644 index 000000000000..63a8e82f1ba4 --- /dev/null +++ b/test/Conversion/ImportVerilog/interface-modport-body-access.sv @@ -0,0 +1,35 @@ +// RUN: circt-verilog --ir-moore %s | FileCheck %s +// REQUIRES: slang + +// Test for in-body access to modport members of a modport-typed interface port. +// The receiving module reads `bus.x` (modport input) and writes `bus.y` +// (modport output). Slang resolves these as HierarchicalValueExpressions whose +// symbol is the ModportPortSymbol; ImportVerilog must resolve them through the +// flattened interface port instead of registering them as cross-instance +// hierPath inputs (which would add spurious unfilled module ports). +// UNSUPPORTED: valgrind + +interface IfaceModportBody; + logic [7:0] x; + logic [7:0] y; + modport mp (input x, output y); +endinterface + +// CHECK-LABEL: moore.module private @ModportBodyUse(in %bus_x : !moore.l8, out bus_y : !moore.l8) { +// CHECK: moore.output %bus_x : !moore.l8 +// CHECK: } +module ModportBodyUse(IfaceModportBody.mp bus); + assign bus.y = bus.x; +endmodule + +// CHECK-LABEL: moore.module @TopModportBodyUse() { +// CHECK: %ifc_x = moore.variable : +// CHECK: %ifc_y = moore.assigned_variable %dut.bus_y : l8 +// CHECK: [[X:%.+]] = moore.read %ifc_x : +// CHECK: %dut.bus_y = moore.instance "dut" @ModportBodyUse(bus_x: [[X]]: !moore.l8) -> (bus_y: !moore.l8) +// CHECK: moore.output +// CHECK: } +module TopModportBodyUse; + IfaceModportBody ifc(); + ModportBodyUse dut(.bus(ifc)); +endmodule