From: Zachary Snow Date: Thu, 25 Mar 2021 18:06:05 +0000 (-0400) Subject: verilog: fix case expression sign and width handling X-Git-Tag: yosys-0.10~164 X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=0795b3ec076d8d2c0aa0d954b707271bd2f064bf;p=yosys.git verilog: fix case expression sign and width handling - The case expression and case item expressions are extended to the maximum width among them, and are only interpreted as signed if all of them are signed - Add overall width and sign detection for AST_CASE - Add sign argument to genWidthRTLIL helper - Coverage for both const and non-const case statements --- diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h index 069479353..9887d24ea 100644 --- a/frontends/ast/ast.h +++ b/frontends/ast/ast.h @@ -291,7 +291,7 @@ namespace AST // 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, bool sign_hint = false); - RTLIL::SigSpec genWidthRTLIL(int width, const dict *new_subst_ptr = NULL); + RTLIL::SigSpec genWidthRTLIL(int width, bool sgn, const dict *new_subst_ptr = NULL); // compare AST nodes bool operator==(const AstNode &other) const; diff --git a/frontends/ast/genrtlil.cc b/frontends/ast/genrtlil.cc index ad5814f1b..b8b9f715e 100644 --- a/frontends/ast/genrtlil.cc +++ b/frontends/ast/genrtlil.cc @@ -566,7 +566,7 @@ struct AST_INTERNAL::ProcessGenerator case AST_ASSIGN_LE: { RTLIL::SigSpec unmapped_lvalue = ast->children[0]->genRTLIL(), lvalue = unmapped_lvalue; - RTLIL::SigSpec rvalue = ast->children[1]->genWidthRTLIL(lvalue.size(), &subst_rvalue_map.stdmap()); + RTLIL::SigSpec rvalue = ast->children[1]->genWidthRTLIL(lvalue.size(), true, &subst_rvalue_map.stdmap()); pool lvalue_sigbits; for (int i = 0; i < GetSize(lvalue); i++) { @@ -593,9 +593,13 @@ struct AST_INTERNAL::ProcessGenerator case AST_CASE: { + int width_hint; + bool sign_hint; + ast->detectSignWidth(width_hint, sign_hint); + RTLIL::SwitchRule *sw = new RTLIL::SwitchRule; set_src_attr(sw, ast); - sw->signal = ast->children[0]->genWidthRTLIL(-1, &subst_rvalue_map.stdmap()); + sw->signal = ast->children[0]->genWidthRTLIL(width_hint, sign_hint, &subst_rvalue_map.stdmap()); current_case->switches.push_back(sw); for (auto &attr : ast->attributes) { @@ -637,7 +641,7 @@ struct AST_INTERNAL::ProcessGenerator else if (node->type == AST_BLOCK) processAst(node); else - current_case->compare.push_back(node->genWidthRTLIL(sw->signal.size(), &subst_rvalue_map.stdmap())); + current_case->compare.push_back(node->genWidthRTLIL(width_hint, sign_hint, &subst_rvalue_map.stdmap())); } if (default_case != current_case) sw->cases.push_back(current_case); @@ -715,9 +719,9 @@ struct AST_INTERNAL::ProcessGenerator RTLIL::MemWriteAction action; set_src_attr(&action, child); action.memid = memid; - action.address = child->children[0]->genWidthRTLIL(-1, &subst_rvalue_map.stdmap()); - action.data = child->children[1]->genWidthRTLIL(current_module->memories[memid]->width, &subst_rvalue_map.stdmap()); - action.enable = child->children[2]->genWidthRTLIL(-1, &subst_rvalue_map.stdmap()); + action.address = child->children[0]->genWidthRTLIL(-1, true, &subst_rvalue_map.stdmap()); + action.data = child->children[1]->genWidthRTLIL(current_module->memories[memid]->width, true, &subst_rvalue_map.stdmap()); + action.enable = child->children[2]->genWidthRTLIL(-1, true, &subst_rvalue_map.stdmap()); RTLIL::Const orig_priority_mask = child->children[4]->bitsAsConst(); RTLIL::Const priority_mask = RTLIL::Const(0, cur_idx); for (int i = 0; i < portid; i++) { @@ -954,6 +958,32 @@ void AstNode::detectSignWidthWorker(int &width_hint, bool &sign_hint, bool *foun width_hint = max(width_hint, this_width); break; + case AST_CASE: + { + // This detects the _overall_ sign and width to be used for comparing + // the case expression with the case item expressions. The case + // expression and case item expressions are extended to the maximum + // width among them, and are only interpreted as signed if all of them + // are signed. + width_hint = -1; + sign_hint = true; + auto visit_case_expr = [&width_hint, &sign_hint] (AstNode *node) { + int sub_width_hint = -1; + bool sub_sign_hint = true; + node->detectSignWidth(sub_width_hint, sub_sign_hint); + width_hint = max(width_hint, sub_width_hint); + sign_hint &= sub_sign_hint; + }; + visit_case_expr(children[0]); + for (size_t i = 1; i < children.size(); i++) { + AstNode *child = children[i]; + for (AstNode *v : child->children) + if (v->type != AST_DEFAULT && v->type != AST_BLOCK) + visit_case_expr(v); + } + break; + } + case AST_FCALL: if (str == "\\$anyconst" || str == "\\$anyseq" || str == "\\$allconst" || str == "\\$allseq") { if (GetSize(children) == 1) { @@ -1695,7 +1725,7 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint) SigSpec addr_sig = children[0]->genRTLIL(); cell->setPort(ID::ADDR, addr_sig); - cell->setPort(ID::DATA, children[1]->genWidthRTLIL(current_module->memories[str]->width * num_words)); + cell->setPort(ID::DATA, children[1]->genWidthRTLIL(current_module->memories[str]->width * num_words, true)); cell->parameters[ID::MEMID] = RTLIL::Const(str); cell->parameters[ID::ABITS] = RTLIL::Const(GetSize(addr_sig)); @@ -1754,7 +1784,7 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint) case AST_ASSIGN: { RTLIL::SigSpec left = children[0]->genRTLIL(); - RTLIL::SigSpec right = children[1]->genWidthRTLIL(left.size()); + RTLIL::SigSpec right = children[1]->genWidthRTLIL(left.size(), true); if (left.has_const()) { RTLIL::SigSpec new_left, new_right; for (int i = 0; i < GetSize(left); i++) @@ -1979,14 +2009,14 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint) // this is a wrapper for AstNode::genRTLIL() when a specific signal width is requested and/or // signals must be substituted before being used as input values (used by ProcessGenerator) // note that this is using some global variables to communicate this special settings to AstNode::genRTLIL(). -RTLIL::SigSpec AstNode::genWidthRTLIL(int width, const dict *new_subst_ptr) +RTLIL::SigSpec AstNode::genWidthRTLIL(int width, bool sgn, const dict *new_subst_ptr) { const dict *backup_subst_ptr = genRTLIL_subst_ptr; if (new_subst_ptr) genRTLIL_subst_ptr = new_subst_ptr; - bool sign_hint = true; + bool sign_hint = sgn; int width_hint = width; detectSignWidthWorker(width_hint, sign_hint); RTLIL::SigSpec sig = genRTLIL(width_hint, sign_hint); diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc index b9965ba99..305f67da8 100644 --- a/frontends/ast/simplify.cc +++ b/frontends/ast/simplify.cc @@ -1185,8 +1185,12 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, if (const_fold && type == AST_CASE) { + int width_hint; + bool sign_hint; + detectSignWidth(width_hint, sign_hint); while (children[0]->simplify(const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param)) { } if (children[0]->type == AST_CONSTANT && children[0]->bits_only_01()) { + RTLIL::Const case_expr = children[0]->bitsAsConst(width_hint, sign_hint); std::vector new_children; new_children.push_back(children[0]); for (int i = 1; i < GetSize(children); i++) { @@ -1199,7 +1203,10 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, continue; while (v->simplify(const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param)) { } if (v->type == AST_CONSTANT && v->bits_only_01()) { - if (v->bits == children[0]->bits) { + RTLIL::Const case_item_expr = v->bitsAsConst(width_hint, sign_hint); + RTLIL::Const match = const_eq(case_expr, case_item_expr, sign_hint, sign_hint, 1); + log_assert(match.bits.size() == 1); + if (match.bits.front() == RTLIL::State::S1) { while (i+1 < GetSize(children)) delete children[++i]; goto keep_const_cond; diff --git a/tests/simple/case_expr_const.v b/tests/simple/case_expr_const.v new file mode 100644 index 000000000..58267b965 --- /dev/null +++ b/tests/simple/case_expr_const.v @@ -0,0 +1,49 @@ +// Note: case_expr_{,non_}const.v should be modified in tandem to ensure both +// the constant and non-constant case evaluation logic is covered +module top( + // expected to output all 1s + output reg a, b, c, d, e, f, g, h +); + initial begin + case (2'b0) + 1'b0: a = 1; + default: a = 0; + endcase + case (2'sb11) + 2'sb01: b = 0; + 1'sb1: b = 1; + endcase + case (2'sb11) + 1'sb0: c = 0; + 1'sb1: c = 1; + endcase + case (2'sb11) + 1'b0: d = 0; + 1'sb1: d = 0; + default: d = 1; + endcase + case (2'b11) + 1'sb0: e = 0; + 1'sb1: e = 0; + default: e = 1; + endcase + case (1'sb1) + 1'sb0: f = 0; + 2'sb11: f = 1; + default: f = 0; + endcase + case (1'sb1) + 1'sb0: g = 0; + 3'b0: g = 0; + 2'sb11: g = 0; + default: g = 1; + endcase + case (1'sb1) + 1'sb0: h = 0; + 1'b1: h = 1; + 3'b0: h = 0; + 2'sb11: h = 0; + default: h = 0; + endcase + end +endmodule diff --git a/tests/simple/case_expr_non_const.v b/tests/simple/case_expr_non_const.v new file mode 100644 index 000000000..7856e781c --- /dev/null +++ b/tests/simple/case_expr_non_const.v @@ -0,0 +1,59 @@ +// Note: case_expr_{,non_}const.v should be modified in tandem to ensure both +// the constant and non-constant case evaluation logic is covered +module top( + // expected to output all 1s + output reg a, b, c, d, e, f, g, h +); + reg x_1b0 = 1'b0; + reg x_1b1 = 1'b1; + reg signed x_1sb0 = 1'sb0; + reg signed x_1sb1 = 1'sb1; + reg [1:0] x_2b0 = 2'b0; + reg [1:0] x_2b11 = 2'b11; + reg signed [1:0] x_2sb01 = 2'sb01; + reg signed [1:0] x_2sb11 = 2'sb11; + reg [2:0] x_3b0 = 3'b0; + + initial begin + case (x_2b0) + x_1b0: a = 1; + default: a = 0; + endcase + case (x_2sb11) + x_2sb01: b = 0; + x_1sb1: b = 1; + endcase + case (x_2sb11) + x_1sb0: c = 0; + x_1sb1: c = 1; + endcase + case (x_2sb11) + x_1b0: d = 0; + x_1sb1: d = 0; + default: d = 1; + endcase + case (x_2b11) + x_1sb0: e = 0; + x_1sb1: e = 0; + default: e = 1; + endcase + case (x_1sb1) + x_1sb0: f = 0; + x_2sb11: f = 1; + default: f = 0; + endcase + case (x_1sb1) + x_1sb0: g = 0; + x_3b0: g = 0; + x_2sb11: g = 0; + default: g = 1; + endcase + case (x_1sb1) + x_1sb0: h = 0; + x_1b1: h = 1; + x_3b0: h = 0; + x_2sb11: h = 0; + default: h = 0; + endcase + end +endmodule