oddball / ipxact2systemverilog

Translates IPXACT XML to synthesizable VHDL or SystemVerilog
GNU General Public License v2.0
56 stars 19 forks source link

Systemverilog code does not compile #27

Closed g-lesssard closed 3 years ago

g-lesssard commented 3 years ago

Hi @oddball ,

I am currently in the process of integrating this generator in a Systemverilog project. We plan on using cocotb alongside verilator to make our test benches and test our code. While trying your tool, I had some issues compiling the generated systemverilog code. If we look at the generated example from this repo, the compiler throws these errors.

  1. The first one is a simple array assignment issue:
    Unsupported: Replication to form 'int$[0:7]' data type: ... In instance example
    22 | const int example_regAddresses [8] = {

The fix is simply to add an apostrophe before the first bracket of the array assignments, like this:

const int example_regAddresses [8] = '{
     reg0_addr,
     reg1_addr,
     reg2_addr,
     reg3_addr,
     reg4_addr,
     reg5_addr,
     reg6_addr,
     reg7_addr};

source: https://verificationguide.com/systemverilog/systemverilog-fixedsize-array/ (see array assignment section)

This change needs to be made for the example_regNames and example_regUnResetedAddresses arrays as well.

  1. The second one is simply a type specification issue.

In the example_regUnResetedAddresses array, you have an array of reg type where you set values like 1 and 0. You need to specify the bit width to insure proper allignment. By default, the compiler doesn't know if the value is on 1 bit, 8, 16, etc...

The fix would be this:

const reg registers_regUnResetedAddresses [8] = '{
   1'b0,
   1'b0,
   1'b0,
   1'b0,
   1'b0,
   1'b1,
   1'b1,
   1'b0};

The 1'bX tells the compiler that is set on one bit.

  1. The last one is bit more complex allignement issue:
Operator ASSIGN expects 32 bits on the Assign RHS, but Assign RHS's SEL generates 6 bits.
                                                                                                                                               : ... In instance example
  127 |          reg2_addr: r = registers.reg2;

The same thing goes for reg7. The issue is that both of these arrays are not the same width of the other registers. All other registers have a width of 32 bits. For example, reg2 is 6 bits, you therefore need to supply where those 6 bits are.

The fix would be something along the lines of this:

function bit [31:0] read_registers(example_struct_type registers,int address);
      bit [31:0]  r;
      case(address)
         reg0_addr: r = registers.reg0;
         reg1_addr: r = registers.reg1;
         reg2_addr: r[$bits(registers.reg2)-1:0] = registers.reg2; <---
         reg3_addr: r = registers.reg3;
         reg4_addr: r = registers.reg4;
         reg5_addr: r = registers.reg5;
         reg6_addr: r = registers.reg6;
         reg7_addr: r = registers.reg7;
        default: r =0;
      endcase
      return r;
endfunction

The $bits() function is something similar to a sizeof in c/c++. This should also be done on the write_registers function on the data part, since it is supplied as 32 bit wide parameter.

g-lesssard commented 3 years ago

Let me know if you have time or plan to make the changes in the near future or have moved on to another project :smile:

oddball commented 3 years ago

Hi!

I will have a look shortly!

oddball commented 3 years ago

@g-lesssard, If I fix the things that you mentioned, does verilator compile the rest?

Been a decade since I designed ASICs for a living, but last I checked, open source hdl compilers were not that great. I no longer have access to commercial compilers, but they did not have any issues with the generated code last I checked.

https://github.com/MikePopoloski/slang has no problem parsing the code

Screenshot 2021-05-14 at 19 34 40

g-lesssard commented 3 years ago

Hmm I see... Verilator is a bit over-the-top over certain warnings and such, maybe slang is a bit more permissive regarding these specific things. Verilator is not the best but it's getting a lot of active support from the community so it should be getting better over time. With these small fixes, everything compiles fine with it though!

oddball commented 3 years ago

@g-lesssard See if you have any success with 1.0.11 !

g-lesssard commented 3 years ago

Thanks for the changes! I tried installing it with pip and got this error:

Collecting ipxact2systemverilog
  Downloading ipxact2systemverilog-1.0.11.tar.gz (44 kB)
     |████████████████████████████████| 44 kB 1.2 MB/s 
    ERROR: Command errored out with exit status 1:
     command: /usr/bin/python3 -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-install-61kx508t/ipxact2systemverilog/setup.py'"'"'; __file__='"'"'/tmp/pip-install-61kx508t/ipxact2systemverilog/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /tmp/pip-install-61kx508t/ipxact2systemverilog/pip-egg-info
         cwd: /tmp/pip-install-61kx508t/ipxact2systemverilog/
    Complete output (5 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-install-61kx508t/ipxact2systemverilog/setup.py", line 9, in <module>
        with open('requirements.txt') as f:
    FileNotFoundError: [Errno 2] No such file or directory: 'requirements.txt'
    ----------------------------------------
ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.

Seems the requirements.txt file is missing...

oddball commented 3 years ago

@g-lesssard unfortunate. Try it again

oddball commented 3 years ago

I assume it is working. Closing