ijor / fx68k

FX68K 68000 cycle accurate SystemVerilog core
GNU General Public License v3.0
141 stars 31 forks source link

Not compatible with iVerilog #2

Closed jotego closed 2 years ago

jotego commented 5 years ago

Icarus Verilog cannot be used for simulation with fx68k, even when using the -g2005-sv option. As Icarus Verilog is the main FOSS tool for verilog simulation, it would be good to be able to use it.

zerkman commented 5 years ago

Hi, fx68k does not work with Verilator either. This would also be very helpful.

sorgelig commented 5 years ago

SystemVerilog is NOT Verilog, although it encapsulates it. I believe none of simulators support SystemVerilog (although some simulators support SystemVerilog for test benches).

jotego commented 5 years ago

These simulators support a subset of system verilog and are important tools for the community. For a non commercial core compatibility with FOSS is vital.

El El jue, 7 nov 2019 a las 11:07, sorgelig notifications@github.com escribió:

SystemVerilog is NOT Verilog, although it encapsulates it. I believe none of simulators support SystemVerilog.

So i think this issue is irrelevant.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ijor/fx68k/issues/2?email_source=notifications&email_token=AAOG27D7ZC4XKB7JTU7WFWDQSPZGJA5CNFSM4IZB5BNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDMBUEY#issuecomment-551033363, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOG27CMV6O5EUGHLWNT6C3QSPZGJANCNFSM4IZB5BNA .

a1exh commented 5 years ago

All commercial simulators support synthesisable SV. FOSS need to be encouraged to support the SystemVerilog constructs which are synthesisable. SystemVerilog interfaces, multidimensional arrays in ports, increment/decrement and assign, loop variable declaration and increment/decrement are but a few of the features the RTL guys have embraced and are unlikely to give up.

a1exh commented 5 years ago

What are the error messages? Perhaps it is borderline legal code that can be restructured?

sorgelig commented 5 years ago

Problem is not in FX68K. Problem is in iVerilog. Then why complain here?

jotego commented 5 years ago

There were a couple of things only. The most cumbersome one is the use of interfaces. That’s not supported at all. I have a branch of this in my fork where I can simulate with it. But it may contain bugs. I don’t have the test benches needed to verify it. That’s why I haven’t requested a pull. You can see the changes I made at

https://github.com/jotego/fx68k/tree/fix2?files=1

a1exh commented 5 years ago

I could try and use Formality RTL->RTL and formally prove they are equivalent? You could try with FOSS SymbiYosis but it might not support SV Interfaces.

sorgelig commented 5 years ago

So it's just fix of couple lines?

jotego commented 5 years ago

Yes, if you could use formality to check it that would be great. I have no experience with formal checkers, though (shame on me).

If I recall correctly I had to remove some unique statements and the interfaces. It took me 1 hour maybe. I only use mine for simulation. For synthesis I use the original branch.

ijor commented 5 years ago

FX68K uses the basic SV features only. There are no SV interfaces in FX68k. May be you mean "struct" types, and not interfaces.

Simulators should really support "struct". They are so useful for encapsulating signals passed between modules and also helps to make the code more self documented. The free (but of course, not open source) simulators provided by the main FPGA vendors do support struct types.

I have been told that Verilator does support struct types, but currently not correctly.

Removing UNIQUE, at least blindly, is dangerous and not a very good idea. The compiler might generate less efficient code using priority encoders that are really not needed. In some cases it might also make the simulator to produce a different behavior than synthesized code.

a1exh commented 5 years ago

Screenshot at 2019-11-08 10-24-18 Formality showed that Ijor master is identical to jotego fix2 (I had to update locally my fix2 with the latest commit on master "Some PC relative instructions had wrong FC encoding")

a1exh commented 5 years ago

Formally identical for the parameters as they are, I have forgotton how to make formality test all parameter combinations.

a1exh commented 5 years ago

I'm a little rusty on the subtleties of RTL->RTL formality scripts but this is a strong indicator the conversion is ok.

jotego commented 5 years ago

Thank you very much for the formal comparison. I still made something wrong as I cannot synthesize with the modfied code:

Error (10166): SystemVerilog RTL Coding error at fx68kAlu.sv(780): always_comb construct does not infer purely combinational logic. File: /home/jtejada/github/jt_gng/modules/fx68k/fx68kAlu.sv Line: 780

And another question. Even if I made this exercise. Maybe this is not the shape you'd like for your core so it is not mergeable. I wonder if you prefer a different approach.

a1exh commented 5 years ago

Try uncommenting line 804

a1exh commented 5 years ago

If anyone is curious what Not Compared : Unread means. They are assignments which are made but never read which are not compared. Usually "unused" bits of a logic vector.

` (Net ) r:/WORK/fx68k/excUnit/alu/rowDecoder/197_Undef_Inst.200_Undef_Inst.stype[0] (no reader)

(Net )  r:/WORK/fx68k/excUnit/alu/rowDecoder/197_Undef_Inst.200_Undef_Inst.stype[1]  (no reader)

(Net )  r:/WORK/fx68k/excUnit/alu/ccrCore[1]  (no reader)

(Net )  r:/WORK/fx68k/excUnit/alu/ccrCore[3]  (no reader)

(Net )  r:/WORK/fx68k/excUnit/alu/ccrCore[4]  (no reader)

(Net )  r:/WORK/fx68k/busControl/dataOe  (no reader)

(Net )  r:/WORK/fx68k/nanoLatch[23]  (no reader)

(Net )  r:/WORK/fx68k/nanoLatch[2]  (no reader)

(Net )  i:/WORK/fx68k/excUnit/alu/rowDecoder/197_Undef_Inst.200_Undef_Inst.stype[0]  (no reader)

(Net )  i:/WORK/fx68k/excUnit/alu/rowDecoder/197_Undef_Inst.200_Undef_Inst.stype[1]  (no reader)

(Net )  i:/WORK/fx68k/excUnit/alu/ccrCore[1]  (no reader)

(Net )  i:/WORK/fx68k/excUnit/alu/ccrCore[3]  (no reader)

(Net )  i:/WORK/fx68k/excUnit/alu/ccrCore[4]  (no reader)

(Net )  i:/WORK/fx68k/busControl/dataOe  (no reader)

(Net )  i:/WORK/fx68k/nanoLatch[23]  (no reader)

(Net )  i:/WORK/fx68k/nanoLatch[2]  (no reader)`
jotego commented 2 years ago

We have made a fork that works both in synthesis and simulation (iverilog/verilator). It's here in branch fix6.

The edits are kind of ugly as we have just done the minimum required to make verilator work. So they are not worth a PR (I would reject it myself). Nonetheless, they solve the simulation problem.

We will still use the original version for synthesis.