Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct VPI timed callbacks to be one-shot #5

Closed
wants to merge 5 commits into from

Conversation

mballance
Copy link
Contributor

VPI cbAfterDelay callbacks are specified to be one-shot. Corrected Verilator's implementation to match the spec, and create two tests that confirm this behavior. The tests run against Verilator and Icarus Verilog and confirm proper behavior.

Signed-off-by: Matthew Ballance matt.ballance@gmail.com

mballance and others added 3 commits November 19, 2019 17:25
…m behavior against Verilator and Icarus Verilog

Signed-off-by: Matthew Ballance <matt.ballance@gmail.com>
Signed-off-by: Matthew Ballance <matt.ballance@gmail.com>
@wsnyder
Copy link
Member

wsnyder commented Nov 20, 2019

I pushed the driver.pl and CONTRIBUTORS fix, thanks.

The basic fix and tests seem reasonable, but there are compiler warnings (which are fatal - you should export VERILATOR_AUTHOR_SITE=1 for your testing - see internals.adoc). Can you please fix these?

similar for both tests (and BTW maybe they should use the same .cpp file?)

        make -C obj_vlt/t_vpi_zero_time_cb -f /verilator/test_regress/Makefile_obj --no-print-directory VM_PREFIX=Vt_vpi_zero_time_cb TEST_OBJ_DIR=obj_vlt/t_vpi_zero_tim$
ccache g++  -I. -Wall -Wextra -Wfloat-conversion -Wlogical-op -Werror -MMD -I/verilator/include -I/verilator/include/vltstd -DVL_PRINTF=printf -DVM_COVERAGE=0 -DVM_SC=0 $
/verilator/test_regress/t/t_vpi_zero_time_cb.cpp: In function 'int _zero_time_cb(p_cb_data)':
/verilator/test_regress/t/t_vpi_zero_time_cb.cpp:94:75: error: no return statement in function returning non-void [-Werror=return-type]
 static int _zero_time_cb(p_cb_data cb_data) { callback_count_zero_time++; }
                                                                           ^
/verilator/test_regress/t/t_vpi_zero_time_cb.cpp: In function 'int _start_of_sim_cb(p_cb_data)':
/verilator/test_regress/t/t_vpi_zero_time_cb.cpp:108:1: error: no return statement in function returning non-void [-Werror=return-type]

/verilator/test_regress/t/t_vpi_zero_time_cb.cpp: In function 'int _end_of_sim_cb(p_cb_data)':
/verilator/test_regress/t/t_vpi_zero_time_cb.cpp:114:1: error: control reaches end of non-void function [-Werror=return-type]

/verilator/test_regress/t/t_vpi_zero_time_cb.cpp: At global scope:
/verilator/test_regress/t/t_vpi_zero_time_cb.cpp:110:12: error: 'int _end_of_sim_cb(p_cb_data)' defined but not used [-Werror=unused-function]
 static int _end_of_sim_cb(p_cb_data cb_data) {
            ^~~~~~~~~~~~~~

/verilator/test_regress/t/t_vpi_zero_time_cb.cpp:96:12: error: 'int _start_of_sim_cb(p_cb_data)' defined but not used [-Werror=unused-function]
 static int _start_of_sim_cb(p_cb_data cb_data) {
            ^~~~~~~~~~~~~~~~

@mballance
Copy link
Contributor Author

Of course, happy to fix the warnings. Didn't realize that those only showed up in 'Author' mode. Just pushed an update that resolves the test warnings for me.

@wsnyder
Copy link
Member

wsnyder commented Nov 20, 2019

Thanks for the contribution, pushed with trivial clang-format space cleanups.

@wsnyder wsnyder closed this Nov 20, 2019
VarunKoyyalagunta added a commit to VarunKoyyalagunta/verilator that referenced this pull request Jul 13, 2022
Causes verilator to segfault when there's no DPI export, or produce bad
code when there is

$ ./test_regress/t/t_trace_split_cfuncs.pl -- debug --gdbbt

	Program received signal SIGSEGV, Segmentation fault.
	V3OutFormatter::puts (this=0x0, strg=0x5555562e2ba0 "\nvoid Vt_trace_split_cfuncs__Syms::_traceDump() {\n") at ../V3File.cpp:699
	699	    if (m_prependIndent && strg[0] != '\n') {
	#0  V3OutFormatter::puts (this=0x0, strg=0x5555562e2ba0 "\nvoid Vt_trace_split_cfuncs__Syms::_traceDump() {\n") at ../V3File.cpp:699
	verilator#1  0x00005555559798f1 in V3OutFormatter::puts (strg=..., this=<optimized out>) at /usr/include/c++/9/bits/basic_string.h:2304
	verilator#2  EmitCBaseVisitor::puts (str=..., this=0x7fffffffd8f0) at ../V3EmitCBase.h:58
	verilator#3  EmitCSyms::emitSymImp (this=this@entry=0x7fffffffd8f0) at ../V3EmitCSyms.cpp:928
	verilator#4  0x00005555559abe1e in EmitCSyms::visit (this=0x7fffffffd8f0, nodep=<optimized out>) at ../V3EmitCSyms.cpp:275
	verilator#5  0x000055555579324a in AstNetlist::accept (this=<optimized out>, v=...) at ../V3AstNodes.h:9348
	verilator#6  0x0000555555974795 in VNVisitor::iterate (nodep=0x555556255f80, this=0x7fffffffd8f0) at ../V3Ast.h:3245
	verilator#7  EmitCSyms::EmitCSyms (dpiHdrOnly=false, nodep=0x555556255f80, this=0x7fffffffd8f0) at ../V3EmitCSyms.cpp:369
	verilator#8  V3EmitC::emitcSyms (dpiHdrOnly=<optimized out>) at ../V3EmitCSyms.cpp:1071
	verilator#9  0x0000555555742333 in process () at ../Verilator.cpp:524
	verilator#10 0x0000555555742c7a in verilate (argString=...) at ../Verilator.cpp:607
	verilator#11 0x0000555555745319 in main (argc=<optimized out>, argv=<optimized out>, env=<optimized out>) at ../Verilator.cpp:733

$ ./test_regress/t/t_trace_split_cfuncs_dpi_export.pl

	vlt/t_trace_split_cfuncs_dpi_export: %Error: Exec of make failed: g++  -I.  -MMD -I/home/ubuntu/src/verilator/test_regress/../include -I/home/ubuntu/src/verilator/test_regress/../include/vltstd -DVM_COVERAGE=0 -DVM_SC=0 -DVM_TRACE=1 -DVM_TRACE_FST=0 -DVM_TRACE_VCD=1 -faligned-new -fcf-protection=none -Wno-bool-operation -Wno-sign-compare -Wno-uninitialized -Wno-unused-but-set-variable -Wno-unused-parameter -Wno-unused-variable -Wno-shadow      -std=gnu++14 -DVERILATOR=1 -DVL_DEBUG=1 -DTEST_OBJ_DIR=obj_vlt/t_trace_split_cfuncs_dpi_export -DVM_PREFIX=Vt_trace_split_cfuncs_dpi_export -DVM_PREFIX_INCLUDE="<Vt_trace_split_cfuncs_dpi_export.h>" -DVM_PREFIX_ROOT_INCLUDE="<Vt_trace_split_cfuncs_dpi_export___024root.h>" -DT_TRACE_SPLIT_CFUNCS_DPI_EXPORT   -O0 -c -o Vt_trace_split_cfuncs_dpi_export__main.o ../../obj_vlt/t_trace_split_cfuncs_dpi_export/Vt_trace_split_cfuncs_dpi_export__main.cpp
VarunKoyyalagunta added a commit to VarunKoyyalagunta/verilator that referenced this pull request Jul 13, 2022
Causes verilator to segfault when there's no DPI export, or produce bad
code when there is

$ ./test_regress/t/t_trace_split_cfuncs.pl -- debug --gdbbt

	Program received signal SIGSEGV, Segmentation fault.
	V3OutFormatter::puts (this=0x0, strg=0x5555562e2ba0 "\nvoid Vt_trace_split_cfuncs__Syms::_traceDump() {\n") at ../V3File.cpp:699
	699	    if (m_prependIndent && strg[0] != '\n') {
	#0  V3OutFormatter::puts (this=0x0, strg=0x5555562e2ba0 "\nvoid Vt_trace_split_cfuncs__Syms::_traceDump() {\n") at ../V3File.cpp:699
	verilator#1  0x00005555559798f1 in V3OutFormatter::puts (strg=..., this=<optimized out>) at /usr/include/c++/9/bits/basic_string.h:2304
	verilator#2  EmitCBaseVisitor::puts (str=..., this=0x7fffffffd8f0) at ../V3EmitCBase.h:58
	verilator#3  EmitCSyms::emitSymImp (this=this@entry=0x7fffffffd8f0) at ../V3EmitCSyms.cpp:928
	verilator#4  0x00005555559abe1e in EmitCSyms::visit (this=0x7fffffffd8f0, nodep=<optimized out>) at ../V3EmitCSyms.cpp:275
	verilator#5  0x000055555579324a in AstNetlist::accept (this=<optimized out>, v=...) at ../V3AstNodes.h:9348
	verilator#6  0x0000555555974795 in VNVisitor::iterate (nodep=0x555556255f80, this=0x7fffffffd8f0) at ../V3Ast.h:3245
	verilator#7  EmitCSyms::EmitCSyms (dpiHdrOnly=false, nodep=0x555556255f80, this=0x7fffffffd8f0) at ../V3EmitCSyms.cpp:369
	verilator#8  V3EmitC::emitcSyms (dpiHdrOnly=<optimized out>) at ../V3EmitCSyms.cpp:1071
	verilator#9  0x0000555555742333 in process () at ../Verilator.cpp:524
	verilator#10 0x0000555555742c7a in verilate (argString=...) at ../Verilator.cpp:607
	verilator#11 0x0000555555745319 in main (argc=<optimized out>, argv=<optimized out>, env=<optimized out>) at ../Verilator.cpp:733

$ ./test_regress/t/t_trace_split_cfuncs_dpi_export.pl

	vlt/t_trace_split_cfuncs_dpi_export: %Error: Exec of make failed: g++  -I.  -MMD -I/home/ubuntu/src/verilator/test_regress/../include -I/home/ubuntu/src/verilator/test_regress/../include/vltstd -DVM_COVERAGE=0 -DVM_SC=0 -DVM_TRACE=1 -DVM_TRACE_FST=0 -DVM_TRACE_VCD=1 -faligned-new -fcf-protection=none -Wno-bool-operation -Wno-sign-compare -Wno-uninitialized -Wno-unused-but-set-variable -Wno-unused-parameter -Wno-unused-variable -Wno-shadow      -std=gnu++14 -DVERILATOR=1 -DVL_DEBUG=1 -DTEST_OBJ_DIR=obj_vlt/t_trace_split_cfuncs_dpi_export -DVM_PREFIX=Vt_trace_split_cfuncs_dpi_export -DVM_PREFIX_INCLUDE="<Vt_trace_split_cfuncs_dpi_export.h>" -DVM_PREFIX_ROOT_INCLUDE="<Vt_trace_split_cfuncs_dpi_export___024root.h>" -DT_TRACE_SPLIT_CFUNCS_DPI_EXPORT   -O0 -c -o Vt_trace_split_cfuncs_dpi_export__main.o ../../obj_vlt/t_trace_split_cfuncs_dpi_export/Vt_trace_split_cfuncs_dpi_export__main.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants