From ca919b3ce0627bbf4d63fdb3bb92c4d59e5bcd83 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 11 Apr 2022 17:40:03 +0200 Subject: [PATCH 1/4] vvp: Add `%disable/flow` instruction The `%disable` instruction will stop the execution of all active threads of a specific scope. This is what is required to implement the semantics of the Verilog `disable` statement. But it is not suited to implement the SystemVerilog flow control statements such as `return`, `continue` and `break`. These only affect the thread hierarchy from which it is called, but not other concurrently running threads from the same scope. Add a new `%disable/flow` instruction that will only disable the thread closest to the current thread in the thread hierarchy. This can either be the thread itself or one of its parents. This will leave other concurrent threads of the same scope untouched and also allows function recursion since only the closest parent thread is disabled. Note that it is not possible to implement this using `%jmp` instructions since a block in a function with variable declarations will be its own sub-thread, but using flow control instructions it is possible to exit from that thread to the parent scope, which is not possible with `%jmp` instructions. Signed-off-by: Lars-Peter Clausen --- vvp/codes.h | 1 + vvp/compile.cc | 1 + vvp/opcodes.txt | 11 +++++++++++ vvp/vthread.cc | 24 ++++++++++++++++++++++++ 4 files changed, 37 insertions(+) diff --git a/vvp/codes.h b/vvp/codes.h index 5c75d3855..2de76495f 100644 --- a/vvp/codes.h +++ b/vvp/codes.h @@ -103,6 +103,7 @@ extern bool of_DELETE_ELEM(vthread_t thr, vvp_code_t code); extern bool of_DELETE_OBJ(vthread_t thr, vvp_code_t code); extern bool of_DELETE_TAIL(vthread_t thr, vvp_code_t code); extern bool of_DISABLE(vthread_t thr, vvp_code_t code); +extern bool of_DISABLE_FLOW(vthread_t thr, vvp_code_t code); extern bool of_DISABLE_FORK(vthread_t thr, vvp_code_t code); extern bool of_DIV(vthread_t thr, vvp_code_t code); extern bool of_DIV_S(vthread_t thr, vvp_code_t code); diff --git a/vvp/compile.cc b/vvp/compile.cc index 5626103b0..9d1e4d26f 100644 --- a/vvp/compile.cc +++ b/vvp/compile.cc @@ -155,6 +155,7 @@ static const struct opcode_table_s opcode_table[] = { { "%delete/obj",of_DELETE_OBJ,1,{OA_FUNC_PTR,OA_NONE, OA_NONE} }, { "%delete/tail",of_DELETE_TAIL,2,{OA_FUNC_PTR,OA_BIT1,OA_NONE} }, { "%disable", of_DISABLE, 1, {OA_VPI_PTR,OA_NONE, OA_NONE} }, + { "%disable/flow", of_DISABLE_FLOW, 1, {OA_VPI_PTR,OA_NONE, OA_NONE} }, { "%disable/fork",of_DISABLE_FORK,0,{OA_NONE,OA_NONE, OA_NONE} }, { "%div", of_DIV, 0, {OA_NONE, OA_NONE, OA_NONE} }, { "%div/s", of_DIV_S, 0, {OA_NONE, OA_NONE, OA_NONE} }, diff --git a/vvp/opcodes.txt b/vvp/opcodes.txt index 97c7a9652..24840c5b8 100644 --- a/vvp/opcodes.txt +++ b/vvp/opcodes.txt @@ -469,6 +469,17 @@ This instruction terminates threads that are part of a specific scope. The label identifies the scope in question, and the threads are the threads that are currently within that scope. +* %disable/flow + +This instruction is similar to `%disable` except that it will only disable a +single thread of the specified scope. The disabled thread will be the thread +closest to the current thread in the thread hierarchy. This can either be thread +itself or one of its parents. + +It is used to implement flow control statements called from within a thread that +only affect the thread or its parents. E.g. SystemVerilog `return`, `continue` +or `break`. + * %disable/fork This instruction terminates all the detached children for the current diff --git a/vvp/vthread.cc b/vvp/vthread.cc index ec4c976c9..29f665125 100644 --- a/vvp/vthread.cc +++ b/vvp/vthread.cc @@ -2718,6 +2718,30 @@ bool of_DISABLE(vthread_t thr, vvp_code_t cp) return ! disabled_myself_flag; } +/* + * Similar to `of_DISABLE`. But will only disable a single thread of the + * specified scope. The disabled thread will be the thread closest to the + * current thread in thread hierarchy. This can either be the current thread, + * either the thread itself or one of its parents. + * This is used for SystemVerilog flow control instructions like `return`, + * `continue` and `break`. + */ + +bool of_DISABLE_FLOW(vthread_t thr, vvp_code_t cp) +{ + __vpiScope*scope = static_cast<__vpiScope*>(cp->handle); + vthread_t cur = thr; + + while (cur && cur->parent_scope != scope) + cur = cur->parent; + + assert(cur); + if (cur) + return !do_disable(cur, thr); + + return false; +} + /* * Implement the %disable/fork (SystemVerilog) instruction by disabling * all the detached children of the given thread. From 84c3c7256313b6f01fce53d33628cd8fb59ff1cb Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 11 Apr 2022 17:40:52 +0200 Subject: [PATCH 2/4] Support recursive functions using `return` statement A `return` statement in a function gets translated into a vvp `%disable` instruction. This works fine as long as no recursion is involved. The `%disable` instruction will stop execution of all active threads of a particular scope. For recursive functions this means as soon as the inner most function returns all containing outer function calls get disabled as well. This results in incorrect behavior. To make recursive functions using the `return` statement work use the new vvp `%disable/parent` instruction. This instruction will only disable the closest thread in the thread hierarchy that matches the target scope. Signed-off-by: Lars-Peter Clausen --- elaborate.cc | 4 ++-- ivl.def | 3 ++- ivl_target.h | 3 +++ net_proc.cc | 4 ++-- netlist.h | 8 +++++++- t-dll-api.cc | 5 +++++ t-dll-proc.cc | 1 + t-dll.h | 1 + tgt-vvp/vvp_process.c | 9 +++++++-- 9 files changed, 30 insertions(+), 8 deletions(-) diff --git a/elaborate.cc b/elaborate.cc index a54a4b822..fa799a22e 100644 --- a/elaborate.cc +++ b/elaborate.cc @@ -5691,7 +5691,7 @@ NetProc* PReturn::elaborate(Design*des, NetScope*scope) const des->errors += 1; return 0; } - NetDisable*disa = new NetDisable(target); + NetDisable*disa = new NetDisable(target, true); disa->set_line( *this ); return disa; } @@ -5720,7 +5720,7 @@ NetProc* PReturn::elaborate(Design*des, NetScope*scope) const assn->set_line( *this ); proc->append(assn); - NetDisable*disa = new NetDisable(target); + NetDisable*disa = new NetDisable(target, true); disa->set_line( *this ); proc->append( disa ); diff --git a/ivl.def b/ivl.def index f02fd0519..7c781eed4 100644 --- a/ivl.def +++ b/ivl.def @@ -292,8 +292,9 @@ ivl_stmt_delay_expr ivl_stmt_delay_val ivl_stmt_events ivl_stmt_file -ivl_stmt_lineno +ivl_stmt_flow_control ivl_stmt_lexp +ivl_stmt_lineno ivl_stmt_lval ivl_stmt_lvals ivl_stmt_lwidth diff --git a/ivl_target.h b/ivl_target.h index 7e41f6ecd..6afde3c27 100644 --- a/ivl_target.h +++ b/ivl_target.h @@ -21,6 +21,7 @@ # include # include +# include /* Re the _CLASS define: clang++ wants this to be class to match the * definition, but clang (the C) compiler needs it to be a struct @@ -2259,6 +2260,8 @@ extern uint64_t ivl_stmt_delay_val(ivl_statement_t net); extern unsigned ivl_stmt_needs_t0_trigger(ivl_statement_t net); extern unsigned ivl_stmt_nevent(ivl_statement_t net); extern ivl_event_t ivl_stmt_events(ivl_statement_t net, unsigned idx); + /* IVL_ST_DISABLE */ +extern bool ivl_stmt_flow_control(ivl_statement_t net); /* IVL_ST_CONTRIB */ extern ivl_expr_t ivl_stmt_lexp(ivl_statement_t net); /* IVL_ST_ASSIGN IVL_ST_ASSIGN_NB IVL_ST_CASSIGN IVL_ST_DEASSIGN diff --git a/net_proc.cc b/net_proc.cc index b5d3d49ca..650be2af5 100644 --- a/net_proc.cc +++ b/net_proc.cc @@ -174,8 +174,8 @@ void NetCase::prune() } } -NetDisable::NetDisable(NetScope*tgt) -: target_(tgt) +NetDisable::NetDisable(NetScope*tgt, bool flow_control) +: target_(tgt), flow_control_(flow_control) { } diff --git a/netlist.h b/netlist.h index 093a05338..fdbe730ca 100644 --- a/netlist.h +++ b/netlist.h @@ -3272,10 +3272,11 @@ class NetDeassign : public NetAssignBase { class NetDisable : public NetProc { public: - explicit NetDisable(NetScope*tgt); + explicit NetDisable(NetScope*tgt, bool flow_control = false); ~NetDisable(); const NetScope*target() const; + bool flow_control() const { return flow_control_; } virtual NexusSet* nex_input(bool rem_out = true, bool always_sens = false, bool nested_func = false) const; @@ -3288,6 +3289,11 @@ class NetDisable : public NetProc { private: NetScope*target_; + // If false all threads in the target_ scope are disabled. If true only + // the closest thread in thread hierachy of the target_ scope is + // disabled. The latter is used to implement flow control statements like + // `return`. + bool flow_control_; private: // not implemented NetDisable(const NetDisable&); diff --git a/t-dll-api.cc b/t-dll-api.cc index e9e7d03ab..c25ab938d 100644 --- a/t-dll-api.cc +++ b/t-dll-api.cc @@ -2713,6 +2713,11 @@ extern "C" ivl_scope_t ivl_stmt_call(ivl_statement_t net) } } +extern "C" bool ivl_stmt_flow_control(ivl_statement_t net) +{ + return net->u_.disable_.flow_control; +} + extern "C" unsigned ivl_stmt_case_count(ivl_statement_t net) { assert(net); diff --git a/t-dll-proc.cc b/t-dll-proc.cc index a5179dcd7..e0469284c 100644 --- a/t-dll-proc.cc +++ b/t-dll-proc.cc @@ -657,6 +657,7 @@ bool dll_target::proc_disable(const NetDisable*net) FILE_NAME(stmt_cur_, net); stmt_cur_->type_ = IVL_ST_DISABLE; + stmt_cur_->u_.disable_.flow_control = net->flow_control(); const NetScope* dis_scope = net->target(); /* A normal disable. */ if (dis_scope) stmt_cur_->u_.disable_.scope = lookup_scope_(dis_scope); diff --git a/t-dll.h b/t-dll.h index 20f156608..3cdaba095 100644 --- a/t-dll.h +++ b/t-dll.h @@ -843,6 +843,7 @@ struct ivl_statement_s { struct { /* IVL_ST_DISABLE */ ivl_scope_t scope; + bool flow_control; } disable_; struct { /* IVL_ST_FOREVER */ diff --git a/tgt-vvp/vvp_process.c b/tgt-vvp/vvp_process.c index 2abb7a025..9c42bcae6 100644 --- a/tgt-vvp/vvp_process.c +++ b/tgt-vvp/vvp_process.c @@ -1340,8 +1340,13 @@ static int show_stmt_disable(ivl_statement_t net, ivl_scope_t sscope) /* A normal disable statement. */ if (target) { - show_stmt_file_line(net, "Disable statement."); - fprintf(vvp_out, " %%disable S_%p;\n", target); + if (ivl_stmt_flow_control(net)) { + show_stmt_file_line(net, "Flow control disable statement."); + fprintf(vvp_out, " %%disable/flow S_%p;\n", target); + } else { + show_stmt_file_line(net, "Disable statement."); + fprintf(vvp_out, " %%disable S_%p;\n", target); + } /* A SystemVerilog disable fork statement. */ } else { show_stmt_file_line(net, "Disable fork statement."); From 5b6d8e968d49da58b6e42226a357c7010304f4f3 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 11 Apr 2022 17:51:14 +0200 Subject: [PATCH 3/4] Add regression test for recursive function using `return Add a regression test that checks that recursive functions using a `return` statement work correctly. Signed-off-by: Lars-Peter Clausen --- .../{recursive_func.v => recursive_func1.v} | 0 ivtest/ivltests/recursive_func2.v | 46 +++++++++++++++++++ ivtest/regress-sv.list | 1 + ivtest/regress-vlg.list | 2 +- ivtest/regress-vlog95.list | 3 +- 5 files changed, 50 insertions(+), 2 deletions(-) rename ivtest/ivltests/{recursive_func.v => recursive_func1.v} (100%) create mode 100644 ivtest/ivltests/recursive_func2.v diff --git a/ivtest/ivltests/recursive_func.v b/ivtest/ivltests/recursive_func1.v similarity index 100% rename from ivtest/ivltests/recursive_func.v rename to ivtest/ivltests/recursive_func1.v diff --git a/ivtest/ivltests/recursive_func2.v b/ivtest/ivltests/recursive_func2.v new file mode 100644 index 000000000..2fd45cbae --- /dev/null +++ b/ivtest/ivltests/recursive_func2.v @@ -0,0 +1,46 @@ +// Check that recursive functions are supported when the `return` statement is +// used. + +module recursive_func(); + +function automatic [15:0] factorial(input [15:0] n); + if (n > 1) begin : recursion + reg [15:0] result; + result = factorial(n - 1) * n; + return result; + end + + return n; +endfunction + +reg [15:0] r1; +reg [15:0] r2; +reg [15:0] r3; + +initial begin + fork + r1 = factorial(3); + r2 = factorial(4); + r3 = factorial(5); + join + $display("factorial 3 = %0d", r1); + $display("factorial 4 = %0d", r2); + $display("factorial 5 = %0d", r3); +end + +wire [15:0] r4; +wire [15:0] r5; +wire [15:0] r6; + +assign r4 = factorial(6); +assign r5 = factorial(7); +assign r6 = factorial(8); + +initial begin + #1; + $display("factorial 6 = %0d", r4); + $display("factorial 7 = %0d", r5); + $display("factorial 8 = %0d", r6); +end + +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index 8a69255de..ef58758eb 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -402,6 +402,7 @@ program5a CE,-g2009 ivltests program5b CE,-g2009 ivltests program_hello normal,-g2009 ivltests program_hello2 CE,-g2009 ivltests +recursive_func2 normal,-g2005-sv ivltests gold=recursive_func.gold sbyte_test normal,-g2005-sv ivltests scalar_vector normal,-g2005-sv ivltests sf_countbits normal,-g2012 ivltests diff --git a/ivtest/regress-vlg.list b/ivtest/regress-vlg.list index 850341b19..cfa4b3b1a 100644 --- a/ivtest/regress-vlg.list +++ b/ivtest/regress-vlg.list @@ -1469,7 +1469,7 @@ real_force_rel normal ivltests real_invalid_ops CE ivltests gold=real_invalid_ops.gold real_logical normal ivltests real_reg_force_rel normal ivltests -recursive_func normal ivltests gold=recursive_func.gold +recursive_func1 normal ivltests gold=recursive_func.gold recursive_task normal ivltests gold=recursive_task.gold redef_net_error CE ivltests redef_reg_error CE ivltests diff --git a/ivtest/regress-vlog95.list b/ivtest/regress-vlog95.list index 6569c8c60..8e5fcf2be 100644 --- a/ivtest/regress-vlog95.list +++ b/ivtest/regress-vlog95.list @@ -90,7 +90,8 @@ pr2172606b CE ivltests pr2276163 CE ivltests pr2929913 CE ivltests real_events CE ivltests -recursive_func CE ivltests +recursive_func1 CE ivltests +recursive_func2 CE ivltests recursive_task CE ivltests task_init_var1 CE,-pallowsigned=1 ivltests task_init_var2 CE,-pallowsigned=1 ivltests From 338516bc555c473d04d9dd7f7b208c5200407f4c Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 11 Apr 2022 18:54:36 +0200 Subject: [PATCH 4/4] Add regression tests for constant recursive functions Check that constant recursive functions are supported. Check both Verilog style using assignments to the implicit function return signal and SystemVerilog style using `return`. Signed-off-by: Lars-Peter Clausen --- ivtest/gold/recursive_func_const.gold | 3 +++ ivtest/ivltests/recursive_func_const1.v | 25 +++++++++++++++++++++++++ ivtest/ivltests/recursive_func_const2.v | 23 +++++++++++++++++++++++ ivtest/regress-sv.list | 1 + ivtest/regress-vlg.list | 1 + ivtest/regress-vlog95.list | 2 ++ 6 files changed, 55 insertions(+) create mode 100644 ivtest/gold/recursive_func_const.gold create mode 100644 ivtest/ivltests/recursive_func_const1.v create mode 100644 ivtest/ivltests/recursive_func_const2.v diff --git a/ivtest/gold/recursive_func_const.gold b/ivtest/gold/recursive_func_const.gold new file mode 100644 index 000000000..08e15931c --- /dev/null +++ b/ivtest/gold/recursive_func_const.gold @@ -0,0 +1,3 @@ +factorial 3 = 6 +factorial 4 = 24 +factorial 5 = 120 diff --git a/ivtest/ivltests/recursive_func_const1.v b/ivtest/ivltests/recursive_func_const1.v new file mode 100644 index 000000000..b45adebf3 --- /dev/null +++ b/ivtest/ivltests/recursive_func_const1.v @@ -0,0 +1,25 @@ +// Check that constant recursive functions are supported. + +module recursive_func(); + +function automatic [15:0] factorial; + +input [15:0] n; + +begin + factorial = (n > 1) ? factorial(n - 1) * n : n; +end + +endfunction + +localparam F3 = factorial(3); +localparam F4 = factorial(4); +localparam F5 = factorial(5); + +initial begin + $display("factorial 3 = %0d", F3); + $display("factorial 4 = %0d", F4); + $display("factorial 5 = %0d", F5); +end + +endmodule diff --git a/ivtest/ivltests/recursive_func_const2.v b/ivtest/ivltests/recursive_func_const2.v new file mode 100644 index 000000000..a7c3b92b9 --- /dev/null +++ b/ivtest/ivltests/recursive_func_const2.v @@ -0,0 +1,23 @@ +// Check that constant recursive functions are supported when the `return` +// statement is used. + +module recursive_func(); + +function automatic [15:0] factorial(input [15:0] n); + if (n > 1) begin + return factorial(n - 1) * n; + end + return n; +endfunction + +localparam F3 = factorial(3); +localparam F4 = factorial(4); +localparam F5 = factorial(5); + +initial begin + $display("factorial 3 = %0d", F3); + $display("factorial 4 = %0d", F4); + $display("factorial 5 = %0d", F5); +end + +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index ef58758eb..15ea67e72 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -403,6 +403,7 @@ program5b CE,-g2009 ivltests program_hello normal,-g2009 ivltests program_hello2 CE,-g2009 ivltests recursive_func2 normal,-g2005-sv ivltests gold=recursive_func.gold +recursive_func_const2 normal,-g2005-sv ivltests gold=recursive_func_const.gold sbyte_test normal,-g2005-sv ivltests scalar_vector normal,-g2005-sv ivltests sf_countbits normal,-g2012 ivltests diff --git a/ivtest/regress-vlg.list b/ivtest/regress-vlg.list index cfa4b3b1a..5f32b39d2 100644 --- a/ivtest/regress-vlg.list +++ b/ivtest/regress-vlg.list @@ -1470,6 +1470,7 @@ real_invalid_ops CE ivltests gold=real_invalid_ops.gold real_logical normal ivltests real_reg_force_rel normal ivltests recursive_func1 normal ivltests gold=recursive_func.gold +recursive_func_const1 normal ivltests gold=recursive_func_const.gold recursive_task normal ivltests gold=recursive_task.gold redef_net_error CE ivltests redef_reg_error CE ivltests diff --git a/ivtest/regress-vlog95.list b/ivtest/regress-vlog95.list index 8e5fcf2be..b70b1bd66 100644 --- a/ivtest/regress-vlog95.list +++ b/ivtest/regress-vlog95.list @@ -92,6 +92,8 @@ pr2929913 CE ivltests real_events CE ivltests recursive_func1 CE ivltests recursive_func2 CE ivltests +recursive_func_const1 CE ivltests +recursive_func_const2 CE ivltests recursive_task CE ivltests task_init_var1 CE,-pallowsigned=1 ivltests task_init_var2 CE,-pallowsigned=1 ivltests