Add interpolated_symbols logic to inline verilog #44
Conversation
| @@ -191,6 +191,9 @@ std::unique_ptr<BlockComment> Transformer::visit( | |||
|
|
|||
| std::unique_ptr<InlineVerilog> Transformer::visit( | |||
| std::unique_ptr<InlineVerilog> node) { | |||
| for (auto&& symbol : node->interpolated_symbols) { | |||
There was a problem hiding this comment.
From my understanding of this code, it seems like you're basically relying on the fact that a transformer always transforms an expression in the same way (i.e. it is a "pure function", at least for this subset of expressions) and apply the transformation to the expression to be interpolated.
Presumably this works if we only have simple transformers and simple expressions, but this seems pretty fragile to me, both (a) for the reason that transformers can do very general things, and (b) because the referenced expression may not even exist post-transformation.
The most robust thing to do seems like keeping another symbol table that is updated by all transformations. Or providing a unified way for each transformation to maintain that table.
Out of curiosity, how would the following work:
add add_inst(...);
assert property (... add_inst.O ...);and then add_inst gets inlined into x + y? Are interpolated symbols restricted to expressions only?
| std::string result = value; | ||
| for (auto &&it : this->interpolated_symbols) { | ||
| result = std::regex_replace( | ||
| result, std::regex("\\{" + it.first + "\\}"), it.second->toString()); |
There was a problem hiding this comment.
Think we should use {{ }} for templating since { } could definitely appear for verilog concat. Even {{ }} could appear as verilog but think it's significantly less likely.
Also ideally, a templating engine would be used instead of regex. http://goog-ctemplate.sourceforge.net/ is one good example.
|
It turns out this isn't necessary for now, so I'll wait on this feature until it's needed later on (the issue I was running into with inlining can be handled at the CoreIR level, so we don't need to crack this nut for now) |
This extends inline verilog nodes to contain a list of symbols to be interpolated, to be used by rdaly525/coreir#864
The goal is to provide a mapping from symbol to Identifier for interpolation, since these identifiers (e.g. a wire reference) might be inlined in a transformer, so the transformer should update the symbol mapping for the inlining logic. (Effectively this stages interpolation until after inline has been run, so we get the final name for various references).