From 00a6c1d9a57da0e0b0fef07b2d618847ed93a9fd Mon Sep 17 00:00:00 2001 From: Clifford Wolf Date: Tue, 9 Jul 2013 14:31:57 +0200 Subject: [PATCH] Major redesign of expr width/sign detecion (verilog/ast frontend) --- frontends/ast/ast.h | 6 +- frontends/ast/genrtlil.cc | 196 +++++++++++++++++++++++++++------ frontends/verilog/const2ast.cc | 2 +- tests/simple/signedexpr.v | 18 +++ tests/tools/autotest.sh | 4 +- 5 files changed, 189 insertions(+), 37 deletions(-) create mode 100644 tests/simple/signedexpr.v diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h index 12e9a71bc..99760e09c 100644 --- a/frontends/ast/ast.h +++ b/frontends/ast/ast.h @@ -174,10 +174,14 @@ namespace AST void dumpAst(FILE *f, std::string indent, AstNode *other = NULL); void dumpVlog(FILE *f, std::string indent); + // used by genRTLIL() for detecting expression width and sign + void detectSignWidthWorker(int &width_hint, bool &sign_hint); + void detectSignWidth(int &width_hint, bool &sign_hint); + // create RTLIL code for this AST node // for expressions the resulting signal vector is returned // all generated cell instances, etc. are written to the RTLIL::Module pointed to by AST_INTERNAL::current_module - RTLIL::SigSpec genRTLIL(int width_hint = -1); + RTLIL::SigSpec genRTLIL(int width_hint = -1, bool sign_hint = false); RTLIL::SigSpec genWidthRTLIL(int width, RTLIL::SigSpec *subst_from = NULL, RTLIL::SigSpec *subst_to = NULL); // compare AST nodes diff --git a/frontends/ast/genrtlil.cc b/frontends/ast/genrtlil.cc index f9f5c6f88..39f6e90ef 100644 --- a/frontends/ast/genrtlil.cc +++ b/frontends/ast/genrtlil.cc @@ -33,6 +33,7 @@ #include #include #include +#include using namespace AST; using namespace AST_INTERNAL; @@ -503,6 +504,126 @@ struct AST_INTERNAL::ProcessGenerator } }; +// detect sign and width of an expression +void AstNode::detectSignWidthWorker(int &width_hint, bool &sign_hint) +{ + std::string type_name; + bool dummy_sign_hint = true; + // int dummy_width_hint = -1; + + switch (type) + { + case AST_CONSTANT: + width_hint = std::max(width_hint, int(bits.size())); + if (!is_signed) + sign_hint = false; + break; + + case AST_IDENTIFIER: + if ((id2ast && !id2ast->is_signed) || children.size() > 0) + sign_hint = false; + width_hint = std::max(width_hint, genRTLIL().width); + break; + + case AST_TO_SIGNED: + children.at(0)->detectSignWidthWorker(width_hint, dummy_sign_hint); + break; + + case AST_TO_UNSIGNED: + children.at(0)->detectSignWidthWorker(width_hint, sign_hint); + sign_hint = false; + break; + + case AST_CONCAT: + case AST_REPLICATE: + width_hint = std::max(width_hint, genRTLIL().width); + sign_hint = false; + break; + + case AST_NEG: + case AST_BIT_NOT: + case AST_POS: + children[0]->detectSignWidthWorker(width_hint, sign_hint); + break; + + case AST_BIT_AND: + case AST_BIT_OR: + case AST_BIT_XOR: + case AST_BIT_XNOR: + for (auto child : children) + child->detectSignWidthWorker(width_hint, sign_hint); + break; + + case AST_REDUCE_AND: + case AST_REDUCE_OR: + case AST_REDUCE_XOR: + case AST_REDUCE_XNOR: + case AST_REDUCE_BOOL: + width_hint = std::max(width_hint, 1); + sign_hint = false; + break; + + case AST_SHIFT_LEFT: + case AST_SHIFT_RIGHT: + case AST_SHIFT_SLEFT: + case AST_SHIFT_SRIGHT: + children[0]->detectSignWidthWorker(width_hint, sign_hint); + break; + + case AST_LT: + case AST_LE: + case AST_EQ: + case AST_NE: + case AST_GE: + case AST_GT: + width_hint = std::max(width_hint, 1); + sign_hint = false; + break; + + case AST_ADD: + case AST_SUB: + case AST_MUL: + case AST_DIV: + case AST_MOD: + case AST_POW: + for (auto child : children) + child->detectSignWidthWorker(width_hint, sign_hint); + break; + + case AST_LOGIC_AND: + case AST_LOGIC_OR: + case AST_LOGIC_NOT: + for (auto child : children) + child->detectSignWidthWorker(width_hint, sign_hint); + break; + + case AST_TERNARY: + children.at(1)->detectSignWidthWorker(width_hint, sign_hint); + children.at(2)->detectSignWidthWorker(width_hint, sign_hint); + break; + + case AST_MEMRD: + if (!is_signed) + sign_hint = false; + width_hint = std::max(width_hint, current_module->memories.at(str)->width); + break; + + // everything should have been handled above -> print error if not. + default: + for (auto f : log_files) + current_ast->dumpAst(f, "verilog-ast> "); + log_error("Don't know how to detect sign and width for %s node at %s:%d!\n", + type2str(type).c_str(), filename.c_str(), linenum); + } +} + +// detect sign and width of an expression +void AstNode::detectSignWidth(int &width_hint, bool &sign_hint) +{ + width_hint = -1, sign_hint = true; + detectSignWidthWorker(width_hint, sign_hint); +} + // create RTLIL from an AST node // all generated cells, wires and processes are added to the module pointed to by 'current_module' // when the AST node is an expression (AST_ADD, AST_BIT_XOR, etc.), the result signal is returned. @@ -510,7 +631,7 @@ struct AST_INTERNAL::ProcessGenerator // note that this function is influenced by a number of global variables that might be set when // called from genWidthRTLIL(). also note that this function recursively calls itself to transform // larger expressions into a netlist of cells. -RTLIL::SigSpec AstNode::genRTLIL(int width_hint) +RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint) { // in the following big switch() statement there are some uses of // Clifford's Device (http://www.clifford.at/cfun/cliffdev/). In this @@ -612,6 +733,9 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint) // simply return the corresponding RTLIL::SigSpec for an AST_CONSTANT node case AST_CONSTANT: { + if (width_hint < 0) + detectSignWidth(width_hint, sign_hint); + RTLIL::SigChunk chunk; chunk.wire = NULL; chunk.data.bits = bits; @@ -621,6 +745,8 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint) RTLIL::SigSpec sig; sig.chunks.push_back(chunk); sig.width = chunk.width; + + is_signed = sign_hint; return sig; } @@ -702,17 +828,14 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint) if (genRTLIL_subst_from && genRTLIL_subst_to) sig.replace(*genRTLIL_subst_from, *genRTLIL_subst_to); - is_signed = id2ast->is_signed; - if (children.size() != 0) - is_signed = false; - + is_signed = children.size() > 0 ? false : id2ast->is_signed && sign_hint; return sig; } // just pass thru the signal. the parent will evaluated the is_signed property and inperpret the SigSpec accordingly case AST_TO_SIGNED: case AST_TO_UNSIGNED: { - RTLIL::SigSpec sig = children[0]->genRTLIL(width_hint); + RTLIL::SigSpec sig = children[0]->genRTLIL(width_hint, sign_hint); is_signed = type == AST_TO_SIGNED; return sig; } @@ -750,11 +873,13 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint) if (0) { case AST_POS: type_name = "$pos"; } if (0) { case AST_NEG: type_name = "$neg"; } { - RTLIL::SigSpec arg = children[0]->genRTLIL(width_hint); - is_signed = type == AST_NEG || (type == AST_POS && children[0]->is_signed); - int width = type == AST_NEG && arg.width < width_hint ? arg.width+1 : arg.width; - if (width_hint > 0) + RTLIL::SigSpec arg = children[0]->genRTLIL(width_hint, sign_hint); + is_signed = children[0]->is_signed; + int width = arg.width; + if (width_hint > 0) { width = width_hint; + arg.extend(width, is_signed); + } return uniop2rtlil(this, type_name, width, arg); } @@ -764,8 +889,8 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint) if (0) { case AST_BIT_XOR: type_name = "$xor"; } if (0) { case AST_BIT_XNOR: type_name = "$xnor"; } { - RTLIL::SigSpec left = children[0]->genRTLIL(width_hint); - RTLIL::SigSpec right = children[1]->genRTLIL(width_hint); + RTLIL::SigSpec left = children[0]->genRTLIL(width_hint, sign_hint); + RTLIL::SigSpec right = children[1]->genRTLIL(width_hint, sign_hint); int width = std::max(left.width, right.width); if (width_hint > 0) width = width_hint; @@ -795,12 +920,15 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint) // generate cells for binary operations: $shl, $shr, $sshl, $sshr if (0) { case AST_SHIFT_LEFT: type_name = "$shl"; } if (0) { case AST_SHIFT_RIGHT: type_name = "$shr"; } - if (0) { case AST_SHIFT_SLEFT: type_name = "$sshl"; is_signed = true; } - if (0) { case AST_SHIFT_SRIGHT: type_name = "$sshr"; is_signed = true; } + if (0) { case AST_SHIFT_SLEFT: type_name = "$sshl"; } + if (0) { case AST_SHIFT_SRIGHT: type_name = "$sshr"; } { - RTLIL::SigSpec left = children[0]->genRTLIL(); - RTLIL::SigSpec right = children[1]->genRTLIL(width_hint); + if (width_hint < 0) + detectSignWidth(width_hint, sign_hint); + RTLIL::SigSpec left = children[0]->genRTLIL(width_hint, sign_hint); + RTLIL::SigSpec right = children[1]->genRTLIL(); int width = width_hint > 0 ? width_hint : left.width; + is_signed = children[0]->is_signed; return binop2rtlil(this, type_name, width, left, right); } @@ -812,8 +940,11 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint) if (0) { case AST_GE: type_name = "$ge"; } if (0) { case AST_GT: type_name = "$gt"; } { - RTLIL::SigSpec left = children[0]->genRTLIL(); - RTLIL::SigSpec right = children[1]->genRTLIL(); + width_hint = -1, sign_hint = true; + children[0]->detectSignWidthWorker(width_hint, sign_hint); + children[1]->detectSignWidthWorker(width_hint, sign_hint); + RTLIL::SigSpec left = children[0]->genRTLIL(width_hint, sign_hint); + RTLIL::SigSpec right = children[1]->genRTLIL(width_hint, sign_hint); RTLIL::SigSpec sig = binop2rtlil(this, type_name, 1, left, right); return sig; } @@ -826,8 +957,10 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint) if (0) { case AST_MOD: type_name = "$mod"; } if (0) { case AST_POW: type_name = "$pow"; } { - RTLIL::SigSpec left = children[0]->genRTLIL(width_hint); - RTLIL::SigSpec right = children[1]->genRTLIL(width_hint); + if (width_hint < 0) + detectSignWidth(width_hint, sign_hint); + RTLIL::SigSpec left = children[0]->genRTLIL(width_hint, sign_hint); + RTLIL::SigSpec right = children[1]->genRTLIL(width_hint, sign_hint); int width = std::max(left.width, right.width); if (width > width_hint && width_hint > 0) width = width_hint; @@ -842,7 +975,7 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint) if (type == AST_MUL) width = std::min(left.width + right.width, width_hint); } - is_signed = children[0]->is_signed || children[1]->is_signed; + is_signed = children[0]->is_signed && children[1]->is_signed; return binop2rtlil(this, type_name, width, left, right); } @@ -866,22 +999,16 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint) case AST_TERNARY: { RTLIL::SigSpec cond = children[0]->genRTLIL(); - RTLIL::SigSpec val1 = children[1]->genRTLIL(); - RTLIL::SigSpec val2 = children[2]->genRTLIL(); + RTLIL::SigSpec val1 = children[1]->genRTLIL(width_hint, sign_hint); + RTLIL::SigSpec val2 = children[2]->genRTLIL(width_hint, sign_hint); if (cond.width > 1) cond = uniop2rtlil(this, "$reduce_bool", 1, cond, false); int width = std::max(val1.width, val2.width); - if (children[1]->is_signed && children[2]->is_signed) { - is_signed = true; - val1.extend(width, children[1]->is_signed); - val2.extend(width, children[2]->is_signed); - } else { - is_signed = false; - val1.extend(width); - val2.extend(width); - } + is_signed = children[1]->is_signed && children[2]->is_signed; + val1.extend(width); + val2.extend(width); return mux2rtlil(this, cond, val1, val2); } @@ -1063,7 +1190,10 @@ RTLIL::SigSpec AstNode::genWidthRTLIL(int width, RTLIL::SigSpec *subst_from, RT if (subst_to) genRTLIL_subst_to = subst_to; - RTLIL::SigSpec sig = genRTLIL(width); + bool sign_hint = true; + int width_hint = width; + detectSignWidthWorker(width_hint, sign_hint); + RTLIL::SigSpec sig = genRTLIL(width_hint, sign_hint); genRTLIL_subst_from = backup_subst_from; genRTLIL_subst_to = backup_subst_to; diff --git a/frontends/verilog/const2ast.cc b/frontends/verilog/const2ast.cc index 3a88fc046..e38ff2047 100644 --- a/frontends/verilog/const2ast.cc +++ b/frontends/verilog/const2ast.cc @@ -161,7 +161,7 @@ AstNode *VERILOG_FRONTEND::const2ast(std::string code, char case_type) if (str == endptr) intval = -1; - // The "'[bodh]" syntax + // The "'s?[bodh]" syntax if (*endptr == '\'') { int len_in_bits = intval; diff --git a/tests/simple/signedexpr.v b/tests/simple/signedexpr.v new file mode 100644 index 000000000..3eb5e93df --- /dev/null +++ b/tests/simple/signedexpr.v @@ -0,0 +1,18 @@ +module test01(a, b, xu, xs, yu, ys, zu, zs); + +input signed [1:0] a; +input signed [2:0] b; +output [3:0] xu, xs; +output [3:0] yu, ys; +output zu, zs; + +assign xu = (a + b) + 3'd0; +assign xs = (a + b) + 3'sd0; + +assign yu = {a + b} + 3'd0; +assign ys = {a + b} + 3'sd0; + +assign zu = a + b != 3'd0; +assign zs = a + b != 3'sd0; + +endmodule diff --git a/tests/tools/autotest.sh b/tests/tools/autotest.sh index 6b22f9023..e599db3a1 100755 --- a/tests/tools/autotest.sh +++ b/tests/tools/autotest.sh @@ -51,7 +51,7 @@ create_ref() { ( set +x prefix="$2" - xilver=$( ls -v /opt/Xilinx/ | tail -n1; ) + xilver=$( ls -v /opt/Xilinx/ | grep '^[0-9]' | tail -n1; ) case "$( uname -m )" in x86_64) set --; . /opt/Xilinx/$xilver/ISE_DS/settings64.sh ;; @@ -73,7 +73,7 @@ compile_and_run() { ( set +x files=( "$@" ) - xilver=$( ls -v /opt/Xilinx/ | tail -n1; ) + xilver=$( ls -v /opt/Xilinx/ | grep '^[0-9]' | tail -n1; ) case "$( uname -m )" in x86_64) set --; . /opt/Xilinx/$xilver/ISE_DS/settings64.sh ;; -- 2.30.2