Allow more constructs in a generate loop in VAST #3324
Conversation
mikex-oss
left a comment
mikex-oss
left a comment
There was a problem hiding this comment.
Overall looks fine to me, aside from my other comment.
Would like someone with more VAST experience to give it a look.
xls/codegen/vast/vast.h
Outdated
| }; | ||
|
|
||
| using GenerateLoopMember = | ||
| std::variant<LocalParam*, Statement*, AlwaysComb*, AlwaysFf*, AlwaysFlop*, |
There was a problem hiding this comment.
Isn't Statement too general? Shouldn't this be some subset of ModuleMember and explicitly include things like GenerateLoop and ContinuousAssignment?
There was a problem hiding this comment.
Yeah you're right, it's too general or more precisely kinda wrong. I think actually having it hold ModuleMember is probably the right thing to do since the body of the generate loop is effectively at the module scope. The only potentially weird thing is ModuleSection, but that is not really part of the AST and is instead a transparent container which makes it easier to construct VAST as you can add stuff to different points in the module rather than having to build everything sequentially. And there is no reason why the loop body couldn't use ModuleSection for the same purpose. I'll start on that change.
grebe
left a comment
grebe
left a comment
There was a problem hiding this comment.
Modulo @mikex-oss's comment, LGTM
Previously the generate loop contained a StatementBlock which limited what could be put inside of loops to statements. However, other constructs can be put in loops like always blocks, etc. This change expands what may be placed in the body of a GenerateLoop.
meheff
force-pushed
the
meheff/2025-10-27-vast-api
branch
from
4c77ace to
8aa5d96
Compare
|
Updated so generate loop bodies hold module members. |
|
Anything else you need from me on this PR? |
3 participants
Previously the generate loop contained a StatementBlock which limited what could be put inside of loops to statements. However, other constructs can be put in loops like always blocks, etc. This change expands what may be placed in the body of a GenerateLoop.