tgingold-cern / cheby

GNU General Public License v3.0
8 stars 4 forks source link

SystemVerilog Improvements #31

Closed lorenzschmid closed 10 months ago

lorenzschmid commented 10 months ago

Adds improvement for the SystemVerilog generation, namely:

Introduces a new global variable, hdl_lang inside cheby.hdl.globals.gconfig to distinguish between Verilog and SystemVerilog generation.

tgingold-cern commented 10 months ago

Ok for me (once the failure is fixed: I don't think you can put an event control inside an always_comb).

lorenzschmid commented 10 months ago

So I fixed it by introducing two "dialects", one for Verilog (old behavior) and one for SystemVerilog (new behavior). This introduces a new global variable, hdl_lang inside cheby.hdl.globals.gconfig to distinguish between the two.

As far as I am aware, cheby does not put any events insides its combinatorial process, no? The error shown in the Github Action above, originates from the fact that always_comb does not support sensitivity lists.

Let me know if you agree on the new global variable or if you prefer choosing a different approach.

tgingold-cern commented 10 months ago

It's OK with me.

lorenzschmid commented 10 months ago

Sorry, forget to remove the draft status. Ready to be merged now.

stefanlippuner commented 10 months ago

I think it would make sense to also switch the SV generation & elaboration tests to SystemVerilog.

Overall, I'm not too sure how useful the 'verilog' dialect is. I saw quite a few files that were using 'interface' ports. As far as I know, this is not part of the Verilog spec. Maybe it would be easier and cleaner to only have the SystemVerilog version?

tgingold-cern commented 10 months ago

Unfortunately, we still need verilog. We have old Xilinx fpga in use, which require ISE. And SV support in ISE is not very good. We simply don't try to support everything with plain old verilog.