Skip to content

[ImportVerilog] Resolve modport member accesses through interface ports#10398

Open
micprog wants to merge 2 commits intollvm:mainfrom
micprog:micprog/modport_member_access
Open

[ImportVerilog] Resolve modport member accesses through interface ports#10398
micprog wants to merge 2 commits intollvm:mainfrom
micprog:micprog/modport_member_access

Conversation

@micprog
Copy link
Copy Markdown

@micprog micprog commented May 6, 2026

In-body accesses like bus.member (where bus is an Iface.modport port) are produced by slang as HierarchicalValueExpressions. They were mishandled in two ways:

  1. HierarchicalNames.cpp recorded them as cross-instance hierarchical references, adding spurious hierPath inputs to the module signature that the instance call site never filled in. This produced unsupported port whenever such a module was instantiated.

  2. The expression's symbol is the ModportPortSymbol, not the interface body variable. Body-level lookups via valueSymbols, which keyed on the body variable, missed it.

Fix: skip via-iface-port refs in HierarchicalNames.cpp using slang's isViaIfacePort(), and thread the ModportPortSymbol through FlattenedIfacePort so Structure.cpp registers both keys (body variable + modport port symbol) in valueSymbols. Slang already enforces modport directionality during elaboration, so the fix does not bypass any access checks.

Also add a regression test exercising in-body read of a modport input and write of a modport output.

Assisted-by: Claude Code:claude-opus-4-7

micprog added 2 commits May 5, 2026 15:44
In-body accesses like `bus.member` (where `bus` is an `Iface.modport`
port) are produced by slang as `HierarchicalValueExpression`s. They were
mishandled in two ways:

1. `HierarchicalNames.cpp` recorded them as cross-instance hierarchical
   references, adding spurious `hierPath` inputs to the module
   signature that the instance call site never filled in. This produced
   `unsupported port` whenever such a module was instantiated.

2. The expression's symbol is the `ModportPortSymbol`, not the
   interface body variable. Body-level lookups via `valueSymbols`,
   which keyed on the body variable, missed it.

Fix: skip via-iface-port refs in `HierarchicalNames.cpp` using slang's
`isViaIfacePort()`, and thread the `ModportPortSymbol` through
`FlattenedIfacePort` so `Structure.cpp` registers both keys (body
variable + modport port symbol) in `valueSymbols`. Slang already
enforces modport directionality during elaboration, so the fix does
not bypass any access checks.

Also add a regression test exercising in-body read of a modport input
and write of a modport output.

Assisted-by: Claude Code:claude-opus-4-7
Copy link
Copy Markdown
Contributor

@Scheremo Scheremo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got a nit, but otherwise LGTM!

Comment on lines +1310 to +1314
if (auto *mppSym = fp.modportPortSym
? fp.modportPortSym->as_if<slang::ast::ValueSymbol>()
: nullptr;
mppSym && mppSym != valueSym)
valueSymbols.insert(mppSym, portValue);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (auto *mppSym = fp.modportPortSym
? fp.modportPortSym->as_if<slang::ast::ValueSymbol>()
: nullptr;
mppSym && mppSym != valueSym)
valueSymbols.insert(mppSym, portValue);
slang::ast::ValueSymbol *mppSym = nullptr;
if (fp.modportPortSym) {
mppSym = fp.modportPortSym->as_if<slang::ast::ValueSymbol>();
}
if (mppSym && mppSym != valueSym) {
valueSymbols.insert(mppSym, portValue);
}

nit: Might be nice to take this apart, looks like that's three checks rolled into one statement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants