From 2fa7260a4c870b81294c742d627c42dfa8c89281 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 28 Dec 2021 17:13:08 +0100 Subject: [PATCH 1/3] tgt-vvp: Consolidate vec4 logical `and` and `or` generation The code for generating the logical `and` and `or` operators is identical except for the final opcode to combine the two results. Consolidate this into a single function to reduce the code a bit. Signed-off-by: Lars-Peter Clausen --- tgt-vvp/eval_vec4.c | 48 +++++++++++++++++---------------------------- 1 file changed, 18 insertions(+), 30 deletions(-) diff --git a/tgt-vvp/eval_vec4.c b/tgt-vvp/eval_vec4.c index ae9dba47d..41d1733a0 100644 --- a/tgt-vvp/eval_vec4.c +++ b/tgt-vvp/eval_vec4.c @@ -432,8 +432,22 @@ static void draw_binary_vec4_lequiv(ivl_expr_t expr) assert(ivl_expr_width(expr) == 1); } -static void draw_binary_vec4_land(ivl_expr_t expr) +static void draw_binary_vec4_logical(ivl_expr_t expr, char op) { + const char *opcode; + + switch (op) { + case 'a': + opcode = "and"; + break; + case 'o': + opcode = "or"; + break; + default: + assert(0); + break; + } + ivl_expr_t le = ivl_expr_oper1(expr); ivl_expr_t re = ivl_expr_oper2(expr); @@ -449,7 +463,7 @@ static void draw_binary_vec4_land(ivl_expr_t expr) if (ivl_expr_width(re) > 1) fprintf(vvp_out, " %%or/r;\n"); - fprintf(vvp_out, " %%and;\n"); + fprintf(vvp_out, " %%%s;\n", opcode); if (ivl_expr_width(expr) > 1) fprintf(vvp_out, " %%pad/u %u;\n", ivl_expr_width(expr)); @@ -632,29 +646,6 @@ static void draw_binary_vec4_le(ivl_expr_t expr) } } -static void draw_binary_vec4_lor(ivl_expr_t expr) -{ - ivl_expr_t le = ivl_expr_oper1(expr); - ivl_expr_t re = ivl_expr_oper2(expr); - - /* Push the left expression. Reduce it to a single bit if - necessary. */ - draw_eval_vec4(le); - if (ivl_expr_width(le) > 1) - fprintf(vvp_out, " %%or/r;\n"); - - /* Now push the right expression. Again, reduce to a single - bit if necessary. */ - draw_eval_vec4(re); - if (ivl_expr_width(re) > 1) - fprintf(vvp_out, " %%or/r;\n"); - - fprintf(vvp_out, " %%or;\n"); - - if (ivl_expr_width(expr) > 1) - fprintf(vvp_out, " %%pad/u %u;\n", ivl_expr_width(expr)); -} - static void draw_binary_vec4_lrs(ivl_expr_t expr) { ivl_expr_t le = ivl_expr_oper1(expr); @@ -695,7 +686,8 @@ static void draw_binary_vec4(ivl_expr_t expr) { switch (ivl_expr_opcode(expr)) { case 'a': /* Logical && */ - draw_binary_vec4_land(expr); + case 'o': /* || (logical or) */ + draw_binary_vec4_logical(expr, ivl_expr_opcode(expr)); break; case '+': @@ -738,10 +730,6 @@ static void draw_binary_vec4(ivl_expr_t expr) draw_binary_vec4_lrs(expr); break; - case 'o': /* || (logical or) */ - draw_binary_vec4_lor(expr); - break; - case 'q': /* -> (logical implication) */ draw_binary_vec4_limpl(expr); break; From d4334139d3b77bd26a7ac8385e5e73736d746825 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 28 Dec 2021 16:44:41 +0100 Subject: [PATCH 2/3] tgt-vvp: Short circuit logical operators Section 11.4.7 of the SystemVerilog LRM states ``` The && and || operators shall use short circuit evaluation as follows: - The first operand expression shall always be evaluated. - For &&, if the first operand value is logically false then the second operand shall not be evaluated. - For ||, if the first operand value is logically true then the second operand shall not be evaluated. ``` vvp currently evaluates both operands of a logical operator. This works fine as long as the right-hand side does not have a side effect. But if it has the result might be incorrect. E.g. for `a && b++` `b` must not be incremented if `a` evaluates to false. The Verilog LRM mentions that it is allowed to short circuit any expression "if the final result of an expression can be determined early". But there is no requirement to do so. So the new and the old behavior are both correct implementations in Verilog. Use the new behavior in both Verilog and SystemVerilog mode to make sure the behavior is consistent when an expression has side effects. Signed-off-by: Lars-Peter Clausen --- tgt-vvp/eval_condit.c | 29 +++++++++++++++++++++++------ tgt-vvp/eval_vec4.c | 22 ++++++++++++++-------- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/tgt-vvp/eval_condit.c b/tgt-vvp/eval_condit.c index 2e46894ef..1fc44d32e 100644 --- a/tgt-vvp/eval_condit.c +++ b/tgt-vvp/eval_condit.c @@ -278,22 +278,39 @@ static int draw_condition_binary_le(ivl_expr_t expr) static int draw_condition_binary_lor(ivl_expr_t expr) { + unsigned label_out = local_count++; + ivl_expr_t le = ivl_expr_oper1(expr); ivl_expr_t re = ivl_expr_oper2(expr); int lx = draw_eval_condition(le); + int tmp_flag = lx; + + /* Short circuit right hand side if necessary */ + fprintf(vvp_out, " %%jmp/1 T_%u.%u, %d;\n", thread_count, label_out, lx); if (lx < 8) { - int tmp = allocate_flag(); - fprintf(vvp_out, " %%flag_mov %d, %d;\n", tmp, lx); - lx = tmp; + tmp_flag = allocate_flag(); + fprintf(vvp_out, " %%flag_mov %d, %d;\n", tmp_flag, lx); } int rx = draw_eval_condition(re); - fprintf(vvp_out, " %%flag_or %d, %d;\n", rx, lx); - clr_flag(lx); - return rx; + /* + * The flag needs to be in the same position regardless of whether the + * right side is short-cicuited or not. + */ + if (lx == tmp_flag) { + fprintf(vvp_out, " %%flag_or %d, %d;\n", lx, rx); + } else { + fprintf(vvp_out, " %%flag_or %d, %d;\n", rx, tmp_flag); + if (lx != rx) + fprintf(vvp_out, " %%flag_mov %d, %d;\n", lx, rx); + clr_flag(tmp_flag); + } + fprintf(vvp_out, "T_%u.%u;\n", thread_count, label_out); + clr_flag(rx); + return lx; } static int draw_condition_binary(ivl_expr_t expr) diff --git a/tgt-vvp/eval_vec4.c b/tgt-vvp/eval_vec4.c index 41d1733a0..d57ee39bc 100644 --- a/tgt-vvp/eval_vec4.c +++ b/tgt-vvp/eval_vec4.c @@ -435,36 +435,42 @@ static void draw_binary_vec4_lequiv(ivl_expr_t expr) static void draw_binary_vec4_logical(ivl_expr_t expr, char op) { const char *opcode; + const char *jmp_type; switch (op) { case 'a': opcode = "and"; + jmp_type = "0"; break; case 'o': opcode = "or"; + jmp_type = "1"; break; default: assert(0); break; } + unsigned label_out = local_count++; + ivl_expr_t le = ivl_expr_oper1(expr); ivl_expr_t re = ivl_expr_oper2(expr); - /* Push the left expression. Reduce it to a single bit if - necessary. */ - draw_eval_vec4(le); - if (ivl_expr_width(le) > 1) - fprintf(vvp_out, " %%or/r;\n"); - - /* Now push the right expression. Again, reduce to a single - bit if necessary. */ + /* Evaluate the left expression as a conditon and skip the right expression + * if the left is false. */ + int flag = draw_eval_condition(le); + fprintf(vvp_out, " %%flag_get/vec4 %d;\n", flag); + fprintf(vvp_out, " %%jmp/%s T_%u.%u, %d;\n", jmp_type, thread_count, + label_out, flag); + clr_flag(flag); + /* Now push the right expression. Reduce to a single bit if necessary. */ draw_eval_vec4(re); if (ivl_expr_width(re) > 1) fprintf(vvp_out, " %%or/r;\n"); fprintf(vvp_out, " %%%s;\n", opcode); + fprintf(vvp_out, "T_%u.%u;\n", thread_count, label_out); if (ivl_expr_width(expr) > 1) fprintf(vvp_out, " %%pad/u %u;\n", ivl_expr_width(expr)); } From 957e3d482f745c5e47af4519a464c93587191564 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 28 Dec 2021 14:15:22 +0100 Subject: [PATCH 3/3] Short circuit logical operator to constant if possible If the left-hand side of a logical operator is a constant that causes the right-hand side to be short-circuited the right-hand side can be discarded even if it is not constant. In this case replace the expression by a constant. E.g. * `0 && expr` will be replaced by a constant 0. * `1 || expr` will be replaced by a constant 1. * `0 -> expr` will be replaced by a constant 1. Note that it is not possible to replace the expression by a constant if only the right-hand side is a constant, even when the value of the expression is constant. The left side still has to be evaluated for side effects. E.g. it is known at elaboration that `a++ && 0` will yield 0, but the increment on `a` has to be executed regardless. Signed-off-by: Lars-Peter Clausen --- eval_tree.cc | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/eval_tree.cc b/eval_tree.cc index 8811f30e7..c4110cd6f 100644 --- a/eval_tree.cc +++ b/eval_tree.cc @@ -811,6 +811,36 @@ NetEConst* NetEBLogic::eval_arguments_(const NetExpr*l, const NetExpr*r) const const NetEConst*lc = dynamic_cast(l); const NetEConst*rc = dynamic_cast(r); + + // If the left side is constant and the right side is short circuited + // replace the expression with a constant + if (rc == 0 && lc != 0) { + verinum v = lc->value(); + verinum::V res = verinum::Vx; + switch (op_) { + case 'a': // Logical AND (&&) + if (v.is_zero()) + res = verinum::V0; + break; + case 'o': // Logical OR (||) + if (! v.is_zero() && v.is_defined()) + res = verinum::V1; + break; + case 'q': // Logical implication (->) + if (v.is_zero()) + res = verinum::V1; + break; + default: + break; + } + if (res != verinum::Vx) { + NetEConst*tmp = new NetEConst(verinum(res, 1)); + ivl_assert(*this, tmp); + eval_debug(this, tmp, false); + return tmp; + } + } + if (lc == 0 || rc == 0) return 0; verinum::V lv = verinum::V0;