From 9b5906b000bb0d418dacd24d3d0661a71dce5416 Mon Sep 17 00:00:00 2001 From: Martin Whitaker Date: Sat, 11 May 2019 21:33:29 +0100 Subject: [PATCH] Fix GitHub issue #244: handle mixed signed/unsigned power operations. The signed version of the power operation in vvp should only be used if the exponent is signed. Both signed and unsigned versions will produce the correct result regardless of the type of the base operand, provided it has been appropriately extended to the result size. (cherry picked from commit ffb34861cfe9d56286bb27983493ac61940094a2) --- expr_synth.cc | 3 ++- ivl_target.h | 6 ++++-- tgt-vvp/eval_vec4.c | 22 +++++++++++----------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/expr_synth.cc b/expr_synth.cc index 6458eeddd..a07900bac 100644 --- a/expr_synth.cc +++ b/expr_synth.cc @@ -388,7 +388,8 @@ NetNet* NetEBPow::synthesize(Design*des, NetScope*scope, NetExpr*root) powr->set_line(*this); des->add_node(powr); - powr->set_signed( has_sign() ); + // The lpm_pwr object only cares about the signedness of the exponent. + powr->set_signed( right_->has_sign() ); connect(powr->pin_DataA(), lsig->pin(0)); connect(powr->pin_DataB(), rsig->pin(0)); diff --git a/ivl_target.h b/ivl_target.h index 1d44f3a5c..2ed94232f 100644 --- a/ivl_target.h +++ b/ivl_target.h @@ -1245,8 +1245,10 @@ extern unsigned ivl_lpm_lineno(ivl_lpm_t net); * width of a general power is the XXXX of the widths of the * inputs. * - * Power may be signed. If so, the output should be sign extended - * to fill in its result. + * Power may be signed. This indicates the type of the exponent. The + * base will always be treated as unsigned. The compiler must ensure + * the width of the base is equal to the width of the output to + * obtain the correct result when the base is really a signed value. * * - Part Select (IVL_LPM_PART_VP and IVL_LPM_PART_PV) * There are two part select devices, one that extracts a part from a diff --git a/tgt-vvp/eval_vec4.c b/tgt-vvp/eval_vec4.c index 00e96bc86..175829754 100644 --- a/tgt-vvp/eval_vec4.c +++ b/tgt-vvp/eval_vec4.c @@ -117,13 +117,18 @@ static void draw_binary_vec4_arith(ivl_expr_t expr) unsigned rwid = ivl_expr_width(re); unsigned ewid = ivl_expr_width(expr); - int signed_flag = ivl_expr_signed(le) && ivl_expr_signed(re) ? 1 : 0; + int is_power_op = ivl_expr_opcode(expr) == 'p' ? 1 : 0; + + /* The power operation differs from the other arithmetic operations + in that we only use the signed version of the operation if the + right hand operand (the exponent) is signed. */ + int signed_flag = (ivl_expr_signed(le) || is_power_op) && ivl_expr_signed(re) ? 1 : 0; const char*signed_string = signed_flag? "/s" : ""; - /* All the arithmetic operations handled here require that the - operands (and the result) be the same width. We further - assume that the core has not given us an operand wider then - the expression width. So padd operands as needed. */ + /* All the arithmetic operations handled here (except for the power + operation) require that the operands (and the result) be the same + width. We further assume that the core has not given us an operand + wider then the expression width. So pad operands as needed. */ draw_eval_vec4(le); if (lwid != ewid) { fprintf(vvp_out, " %%pad/%c %u;\n", ivl_expr_signed(le)? 's' : 'u', ewid); @@ -150,7 +155,7 @@ static void draw_binary_vec4_arith(ivl_expr_t expr) } draw_eval_vec4(re); - if (rwid != ewid) { + if ((rwid != ewid) && !is_power_op) { fprintf(vvp_out, " %%pad/%c %u;\n", ivl_expr_signed(re)? 's' : 'u', ewid); } @@ -171,11 +176,6 @@ static void draw_binary_vec4_arith(ivl_expr_t expr) fprintf(vvp_out, " %%mod%s;\n", signed_string); break; case 'p': - /* Note that the power operator is signed if EITHER of - the operands is signed. This is different from other - arithmetic operators. */ - if (ivl_expr_signed(le) || ivl_expr_signed(re)) - signed_string = "/s"; fprintf(vvp_out, " %%pow%s;\n", signed_string); break;