From d4334139d3b77bd26a7ac8385e5e73736d746825 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 28 Dec 2021 16:44:41 +0100 Subject: [PATCH] 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)); }