Skip to content

Conversation

@jpienaar
Copy link
Member

@jpienaar jpienaar commented Jul 8, 2025

The conversion seems to work and gets to Verilog. It seems correct, if not most efficient lowered form if there is only one state. Not sure if there was a different reason for this restriction (beyond, this would be better as just HWModule, although didn't see a pattern that would do that).

@jpienaar jpienaar requested a review from mortbopet July 8, 2025 12:07
Copy link
Contributor

@teqdruid teqdruid left a comment

Choose a reason for hiding this comment

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

Could you add some tests for this edge case?

@jpienaar
Copy link
Member Author

jpienaar commented Aug 18, 2025

Sure, sorry for delay. I'm not sure where to put it, I can do a unit test of just the conversion to SV, but it feels like seeing the verilog shows no further issues. But could also stop at this point and just test output of this pass.

I could also split it in 2: do one here and on in export pass that uses the output of the first two as input.

@jpienaar jpienaar requested a review from teqdruid August 20, 2025 14:32
Copy link
Contributor

@teqdruid teqdruid left a comment

Choose a reason for hiding this comment

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

But could also stop at this point and just test output of this pass.

This please. Just ensure that there's really no reason for that edge case detection to be there.

The conversion seems to work and gets to Verilog. It seems correct, if not most efficient lowered form if there is only one state. Not sure if there was a different reason for this restriction (beyond, this would be better as just HWModule, although didn't see a pattern that would do that).
@jpienaar jpienaar merged commit bb20a77 into main Aug 29, 2025
7 checks passed
@jpienaar jpienaar deleted the jpienaar/fsmtosv branch August 29, 2025 11:53
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.

3 participants