Project

General

Profile

[logo] 
 
Home
News
Activity
About/Contact
Major Tools
  Dinotrace
  Verilator
  Verilog-mode
  Verilog-Perl
Other Tools
  BugVise
  CovVise
  Force-Gate-Sim
  Gspice
  IPC::Locker
  Rsvn
  SVN::S4
  Voneline
  WFH
General Info
  Papers

Issue #1333

Support LXT2 file format natively

Added by Yu Sheng Lin 4 months ago. Updated 3 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Unsupported
% Done:

0%


Description

I make a issue about this development thread .

0001-Add-basic-LXT2-support.patch View - The patch implement LXT2 support (123 KB) Yu Sheng Lin, 08/26/2018 04:22 PM

History

#1 Updated by Wilson Snyder 4 months ago

  • Status changed from New to Feature
  • Assignee set to Yu Sheng Lin

I used this repo rather than the patch: https://github.com/johnjohnlin/verilator_fork/tree/lxt2 no need to attach patches if you have a public git repo.

Great work! Here's the feedback on your patches. If any of these are hard/non-obvious I can clean them up for you.

  • I noted the lxt2_* functions are copyright 2016, that's ok by itself but did you check you have the most recent versions?
  • Please add documentation for --trace-lxt2 to bin/verilator.
  • >"Also LXT2 format requires zlib, so add this to the Verilator flags."

Probably better to this automatically when --trace-lxt2 is specified with

addLdLibs("-lz");
  • The ./Makefile.in will need some changes to install the new include/lxt2 files.
  • Please change include/verilated_lxt2* to use the Verilator standard indentation. (No tabs, GNU style. You can diff with verilated_vcd* to see it.) If you don't have Emacs to do this for you I can.
  • Thanks for making an enum for the trace type. Instead of a standard enum please use an enum class TraceFormat similar to how AstNumeric (in V3Ast.h) is done, then the related methods traceClassBase traceSourceName can become members of that.
  • Good idea to use lxt2vcd. Can you make this just

    lxt2vcd("$Self->{obj_dir}/simx.lxt2", "$Self->{obj_dir}/simx.vcd");

then add a new "sub lxt2vcd" to driver.pl? This can then give an appropriate error if it isn't installed (similar "sub vcd_identical").

  • Instead of a custom t_trace_ltx2.cpp, please have driver.pl create the appropriate code automatically when it sees the --trace-lxt2 switch. Also note you can't use C++11 f\ eatures (constexpr).
  • Probably all of the existing t_trace_* tests can get new t_trace_*_lxt2 versions which just point at the existing verilog (for the VCD version) with:

    top_filename("t/t_trace_WHATEVER.v");

Once that is done the t_trace_lxt2.v might not have anything special it in and can be removed.

I suspect once you do this you'll see you need the other full/chg functions ;)

  • The t_trace_lxt2 test for me gives this error:
        lxt2vcd obj_vlt/t_trace_lxt2/simx.lxt2 -o obj_vlt/t_trace_lxt2/simx.vcd
        LXTLOAD | 14 facilities
        LXTLOAD | Read 1 block header OK
        LXTLOAD | [0] start time
        LXTLOAD | [2002] end time
        LXTLOAD |
        LXTLOAD | block [0] processing 0 / 2002
        ***ERROR Unknown token '^S\200' on line 0 of obj_vlt/t_trace_lxt2/simx.lxt2
        ***ERROR Unknown token 'QT\$\301\246' on line 0 of obj_vlt/t_trace_lxt2/simx.lxt2
        ...

#2 Updated by Yu Sheng Lin 4 months ago

Thanks for reviewing this patch. I just pushed a new version and fixed most of the problems except for these two:

  • The ./Makefile.in will need some changes to install the new include/lxt2 files.

It seems that it is not necessary, the Makefile installs include/*.[vch]*.

  • please have driver.pl create the appropriate code automatically when it sees the --trace-lxt2 switch

Sorry I don't know how to do that because I did not write Perl before.

  • Probably all of the existing t_trace_* tests can get new t_trace_*_lxt2 versions

Yes, it should be added in the future.

#3 Updated by Wilson Snyder 4 months ago

Please pull from the bug1333_lxt2 branch of the http://git.veripool.org/git/verilator repository as I made some cleanups and additions to your version.

Then please fix the following tests

test_regress/t/t_trace_array_lxt2.pl  --vlt
test_regress/t/t_trace_complex_lxt2.pl  --vlt
test_regress/t/t_trace_complex_params_lxt2.pl  --vlt
test_regress/t/t_trace_complex_structs_lxt2.pl  --vlt
test_regress/t/t_trace_packed_struct_lxt2.pl  --vlt

Also separately, for the changes you made to the lxt2 sources (changing the include to "", and the casts) can you please file a bug with Bybell to get these upstreamed to gtkWave.

Getting close, thanks!

#4 Updated by Yu Sheng Lin 4 months ago

Hello, the new version is updated to this branch . Also, the lxt functions were C code, which can be compiled gcc. I will report it to GTKWave but they might not accept the change.

#5 Updated by Wilson Snyder 4 months ago

  • Category set to Unsupported
  • Status changed from Feature to Resolved

Excellent, made a few more minor cleanups and pushed to git towards 4.000.

Thanks for your good work.

#6 Updated by Frederic Requin 4 months ago

Hello, the linker fails if I trace a vector with more than 32 bits.

I have an undefined reference to "VerilatedLxt2::quad2Str(unsigned long, int)"
g++    main.o verilated_dpi.o verilated.o verilated_lxt2_c.o Vecp5_jtag_spi_top__ALL.a   -lz  -o Vecp5_jtag_spi_top -lm -lstdc++
Vecp5_jtag_spi_top__ALL.a(Vecp5_jtag_spi_top__ALLsup.o):Vecp5_jtag_spi_top__ALLsup.cpp:(.text+0x1821) : référence indéfinie vers « VerilatedLxt2::quad2Str(unsigned long, int) »
Vecp5_jtag_spi_top__ALL.a(Vecp5_jtag_spi_top__ALLsup.o):Vecp5_jtag_spi_top__ALLsup.cpp:(.text+0x1821): relocalisation tronquée pour concorder avec la taille : R_X86_64_PC32 vers le symbole indéfini VerilatedLxt2::quad2Str(unsigned long, int)
Vecp5_jtag_spi_top__ALL.a(Vecp5_jtag_spi_top__ALLsup.o):Vecp5_jtag_spi_top__ALLsup.cpp:(.text+0x3466) : référence indéfinie vers « VerilatedLxt2::quad2Str(unsigned long, int) »
Vecp5_jtag_spi_top__ALL.a(Vecp5_jtag_spi_top__ALLsup.o):Vecp5_jtag_spi_top__ALLsup.cpp:(.text+0x3466): relocalisation tronquée pour concorder avec la taille : R_X86_64_PC32 vers le symbole indéfini VerilatedLxt2::quad2Str(unsigned long, int)
collect2: error: ld a retourné le statut de sortie 1
make: *** [Vecp5_jtag_spi_top.mk:69: Vecp5_jtag_spi_top] Error 1

I am using GCC 7.3.0 under Cygwin 64-bit.

I have double-checked the generated code, nothing seems to be wrong.

Verilog code :

reg [47:0] r_er1_dr;

Vecp5_jtag_spi_top.h :

VL_SIG64(ecp5_jtag_spi_top__DOT__r_er1_dr,47,0);

Vecp5_jtag_spi_top__Trace__Slow.cpp :

vcdp->fullQuad (c+41,(vlTOPp->ecp5_jtag_spi_top__DOT__r_er1_dr),48);

Vecp5_jtag_spi_top__Trace.cpp :

vcdp->chgQuad (c+41,(vlTOPp->ecp5_jtag_spi_top__DOT__r_er1_dr),48);

Any clue ?

Regards,

Frederic

#7 Updated by Yu Sheng Lin 4 months ago

Quad-word dumping is tested correctly in t_trace_complex_lxt2 (acf4a3). The function is in verilated_lxt2_c.cpp. Can you run this to check whether that function is defined?

nm -C *.o | grep quad2

It should give something like

0000000000010a40 T VerilatedLxt2::quad2Str(unsigned long, int)

#8 Updated by Frederic Requin 4 months ago

Hello, I have got :
                 U VerilatedLxt2::quad2Str(unsigned long, int)
0000000000003bd0 T VerilatedLxt2::quad2Str(unsigned long long, int)
The first line is from "Vecp5_jtag_spi_top__ALLsup.o", the second one from "verilated_lxt2_c.o".

Those do match the linker error.

Why vluint64_t does not generate the same type in both files, I do not know.

Regards,

Frederic

#9 Updated by Frederic Requin 4 months ago

By putting warning pragmas in verilatedos.h, I can see that Vecp5_jtag_spi_top__ALLsup.cpp got compiled with "typedef unsigned long vluint64_t;" (good behaviour) and other .cpp files with "typedef unsigned long long vluint64_t;" (wrong behaviour).

If I revert back to VCD tracing, I see the same typedef issue but the linker does not complain.

Very strange.

Frederic

#10 Updated by Frederic Requin 4 months ago

Solved by including <sys/types.h> in verilatedos.h.

It looks like an old bug that never got caught.

Frederic

#11 Updated by Wilson Snyder 4 months ago

Thanks for debugging! I added a sys/types include, this seems to set __WORDSIZE. It has to be conditional on the OS, so please see if the current git works for you.

#12 Updated by Frederic Requin 4 months ago

Yep, it works. Thank you.

#13 Updated by Wilson Snyder 3 months ago

  • Status changed from Resolved to Closed

In 4.002, thanks for your work.

#14 Updated by Wilson Snyder 3 months ago

Note based on your LXT2 code/changes we just committed adding FST format to git, please give it a try, if there are issues please file a new bug. Our hope is to eventually have fancier SystemVerilog type support in FST.

Also available in: Atom