[ExportVerilog] Add ModportType support for non-extern hw.module ports#10030
[ExportVerilog] Add ModportType support for non-extern hw.module ports#10030dmlockhart wants to merge 2 commits intollvm:mainfrom
Conversation
Add support for !sv.modport<> typed ports on non-extern hw.module: - printPackedTypeImpl: emit InterfaceName.ModportName for ModportType. - Port direction: suppress input/output/inout keywords for modport ports. - Wire keyword: suppress 'wire' keyword for modport ports. Modport-typed ports (e.g., MyBundle.sink) already encode their direction in the interface modport definition, so the direction and wire keywords are suppressed when emitting these ports.
There was a problem hiding this comment.
Generally looks good to me. Thank you for sending a PR! !sv.modport hasn't been used much so it may not have good coverage. Please feel free to open an issue if you encounter any problems.
Strictly speaking I think it's necessary to add more complicated logic around // Determine the width of the widest type we have to print so everything lines up nicely. to compute the accurate number of whitespace padding, i.e:
Current output (startOfNamePos=len("output") + len("MyBundle.sink") + 1):
input [31:0] data_in,
MyBundle.sink intf_port,
output [31:0] data_out
Ideal Output (startOfNamePos=max(len("output"), len("MyBundle.sink")) + 1):
input [31:0] data_in,
MyBundle.sink intf_port,
output [31:0] data_out,
However currently the logic is a bit hardcoded around the assumption that ports are only input or output so I'm fine with merging this as is for now and cleaning up the hardcoded assumptions in a follow-up.
| size_t totalWidth = dirWidth + maxTypeWidth; | ||
| ps << portTypeStrings[portIdx]; | ||
| if (portTypeStrings[portIdx].size() < totalWidth) | ||
| ps.nbsp(totalWidth - portTypeStrings[portIdx].size()); |
There was a problem hiding this comment.
Please remove dirWidth and totalWidth.
| ps.nbsp(totalWidth - portTypeStrings[portIdx].size()); | |
| ps << portTypeStrings[portIdx]; | |
| if (portTypeStrings[portIdx].size() < startOfNamePos) | |
| ps.nbsp(startOfNamePos - portTypeStrings[portIdx].size()); |
uenoku
left a comment
There was a problem hiding this comment.
Please format and remove redundant variables, otherwise LGTM!
Add support for
!sv.modport<>typed ports on non-externhw.module:printPackedTypeImpl: emitInterfaceName.ModportNameforModportType.input/output/inoutkeywords for modport ports.wirekeyword for modport ports.Modport-typed ports (e.g.,
MyBundle.sink) already encode their direction in the interface modport definition, so the direction and wire keywords are suppressed when emitting these ports.This enables using SystemVerilog interface modports as port types on regular (non-extern) modules, producing correct Verilog output like: