search

zachjs / sv2v

SystemVerilog to Verilog conversion
BSD 3-Clause "New" or "Revised" License
497 stars 50 forks source link

Initial support for system tasks #276

Open sifferman opened 5 months ago

sifferman commented 5 months ago

Fixes https://github.com/zachjs/sv2v/issues/273

sifferman commented 5 months ago

I'm not currently sure how to include the filename, line number, and hierarchical scope, so I left it out. I can look into it later, but I may wait for guidance.

sifferman commented 5 months ago

Thanks for the code review!

  1. I added --exclude=SeverityTasks
  2. I've thought about the hierarchical scope more, and I'm not sure including it is important since it's not guaranteed the simulation will follow the same hierarchy as what was passed to sv2v. I think this could just be skipped.
  3. I'm not certain the best way to do this. Pointers would be helpful! 😃
sifferman commented 3 months ago

Just fixed the merge conflicts. Happy to make changes as needed!

zachjs commented 1 month ago

Scope and location information should be available via the Scoper module, the it's not super well documented, so I can take a stab at that if you'd like.

sifferman commented 1 month ago

Scope and location information should be available via the Scoper module, the it's not super well documented, so I can take a stab at that if you'd like.

Up to you. These were my further thoughts:

I'm not sure including it is important since it's not guaranteed the simulation will follow the same hierarchy as what was passed to sv2v. I think this could just be skipped.

Also, what if a module is called multiple times? It may be better just to have the module name and that's it

zachjs commented 2 weeks ago

I'm sorry for the continued delay in getting this PR in. I want to reiterate that I think this is a great contribution! I'm working on getting my IR changes merged now. Probably the AST should have used a representation like this for these tasks from the beginning. Then I'll iterate a bit on the items above: A) adding scope and location information if appropriate; and B) simplifying the $fatal elaboration.

zachjs commented 1 week ago

I think this is now pretty close to complete. @sifferman, what do you think of the current implementation? Please feel free to suggest any changes. I have no particular attachment to the current message format.