[ExportVerilog] Support sv.func.* op emission #7015
Conversation
uenoku
force-pushed
the
dev/hidetou/func-export
branch
from
ba2358e
to
3dbbeb7
Compare
| if (dyn_cast_or_null<HWInstanceLike>(op.getSrc().getDefiningOp())) | ||
| // prepare assigns wires to instance outputs and function results, but these | ||
| // are logically handled in the port binding list when outputing an instance. | ||
| if (isa_and_nonnull<HWInstanceLike, FuncCallOp>(op.getSrc().getDefiningOp())) |
There was a problem hiding this comment.
nit: Is the second type argument really necessary?
There was a problem hiding this comment.
Yes, it's necessary. FuncCall op emissions follows a similar emission implementation to HWInstanceOp. The assignments are emitted in FuncCallOp emission so we need to skip here.
| auto printArg = [&](Value value) { | ||
| if (needsComma) | ||
| ps << "," << PP::space; | ||
| emitExpression(value, ops); | ||
| needsComma = true; | ||
| }; |
There was a problem hiding this comment.
won't this lead to a trailing comma? Could you maybe do something using the argument index instead to figure out if you need a comma or not?
There was a problem hiding this comment.
Good question! I think it will not produce training comma since a comma is always emitted before an operand.
| ps << *linkageName << PP::nbsp << "=" << PP::nbsp; | ||
| auto op = | ||
| cast<FuncOp>(state.symbolCache.getDefinition(importOp.getCalleeAttr())); | ||
| assert(op.isDeclaration() && "function must be a declaration"); |
There was a problem hiding this comment.
I don't think I've seen this way of adding assert messages before lol
There was a problem hiding this comment.
Yes, this is per LLVM coding style: https://llvm.org/docs/CodingStandards.html#assert-liberally
| startStatement(); | ||
| ps.addCallback({func, true}); | ||
| // A function is moduled as an automatic function. | ||
| emitFunctionSignature(*this, ps, func, /*isAutomatic=*/true); |
There was a problem hiding this comment.
Adding an argument name as a comment is encouraged for boolean parameter: https://llvm.org/docs/CodingStandards.html#comment-formatting
uenoku
force-pushed
the
dev/hidetou/dpi-import
branch
from
3390da4
to
75cdb7d
Compare
uenoku
changed the base branch from
dev/hidetou/dpi-import
to
dev/hidetou/dpi-import-old
…eForEmission accordingly
uenoku
force-pushed
the
dev/hidetou/func-export
branch
from
b5bfc8c
to
0724cf4
Compare
uenoku
requested review from
teqdruid,
mortbopet,
blakep-msft,
maerhart,
Dinistro,
fabianschuiki,
seldridge,
andrewb1999 and
mikeurbach
as code owners
uenoku
changed the base branch from
dev/hidetou/dpi-import-old
to
dev/hidetou/prepare-nfc
uenoku
removed request for
fabianschuiki,
mikeurbach,
Dinistro,
mortbopet,
andrewb1999,
maerhart and
blakep-msft
uenoku
force-pushed
the
dev/hidetou/func-export
branch
2 times, most recently
from
ef06699
to
3767a5b
Compare
Co-authored-by: Will Dietz <will.dietz@sifive.com>
uenoku
changed the title
|
I'd like to merge. Post-merge review is also appreciated! |
This PR (separated from #6977) implements ExportVerilog support of sv.func, sv.func.dpi.import, sv.func.call and sv.func.call.procedural.
sv.func emission is similar to hw.module but one difference is a return value. Surprisingly in SV a function name is used as a placeholder for a return value so name legalization properly sets
hw.verilogNameof a returned value to a function name.Result names of sv.func.call.procedural are attached to temporary registers created in PrepareForEmission. This is similar implementation approach as hw.instance.
A part of PrepareForEmission and ExportVerilog is ported from #4856