From 78f37f7156e3a3c32bfb161e06925922a824a35e Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Sun, 11 Dec 2022 14:54:50 -0800 Subject: [PATCH 1/2] Handle for loops with empty initialization statement For loops may have empty initialization statements. In that case some things can't be done, such as loop unrolling or synthesis, but otherwise it is a valid thing to do. So generate the correct code in this case. --- design_dump.cc | 7 ++- elaborate.cc | 126 ++++++++++++++++++++++++++++++------------------- net_proc.cc | 17 +++++-- parse.y | 10 +++- pform_dump.cc | 18 ++++++- synth2.cc | 6 +++ 6 files changed, 127 insertions(+), 57 deletions(-) diff --git a/design_dump.cc b/design_dump.cc index f6540a6da..0abb2ed3f 100644 --- a/design_dump.cc +++ b/design_dump.cc @@ -1381,7 +1381,12 @@ void NetForever::dump(ostream&o, unsigned ind) const void NetForLoop::dump(ostream&fd, unsigned ind) const { - fd << setw(ind) << "" << "FOR LOOP index=" << index_->name() << endl; + fd << setw(ind) << "" << "FOR LOOP index="; + if (index_) + fd << index_->name(); + else + fd << ""; + fd << endl; statement_->dump(fd, ind+4); step_statement_->dump(fd, ind+4); } diff --git a/elaborate.cc b/elaborate.cc index 6e9cc3cdc..b18463f6d 100644 --- a/elaborate.cc +++ b/elaborate.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998-2021 Stephen Williams (steve@icarus.com) + * Copyright (c) 1998-2022 Stephen Williams (steve@icarus.com) * Copyright CERN 2013 / Stephen Williams (steve@icarus.com) * * This source code is free software; you can redistribute it @@ -5434,81 +5434,111 @@ NetProc* PForeach::elaborate_static_array_(Design*des, NetScope*scope, } /* - * elaborate the for loop as the equivalent while loop. This eases the - * task for the target code generator. The structure is: + * Elaborate the PForStatement as discovered by the parser into a + * NetForLoop object. The parser detects the: * - * begin : top - * name1_ = expr1_; - * while (cond_) begin : body - * statement_; - * name2_ = expr2_; - * end - * end + * - index variable (name1_) (optional) + * - initial value (expr1_) (only if name1_ is present) + * - condition expression (cond_) + * - step statement (step_) + * - sub-statement (statement_) + * + * The rules that lead to the PForStatment look like: + * + * for ( = ; ; ) + * for ( ; ) */ NetProc* PForStatement::elaborate(Design*des, NetScope*scope) const { NetExpr*initial_expr; + NetNet*sig; + bool error_flag = false; assert(scope); - const PEIdent*id1 = dynamic_cast(name1_); - assert(id1); + if (!name1_) { + // If there is no initial assignment expression, then mark that + // fact with null pointers. + ivl_assert(*this, !expr1_); + sig = nullptr; + initial_expr = nullptr; - /* make the expression, and later the initial assignment to - the condition variable. The statement in the for loop is - very specifically an assignment. */ - NetNet*sig = des->find_signal(scope, id1->path()); - if (sig == 0) { - cerr << id1->get_fileline() << ": register ``" << id1->path() - << "'' unknown in " << scope_path(scope) << "." << endl; + } else if (const PEIdent*id1 = dynamic_cast(name1_)) { + // If there is an initialization assignment, make the expression, + // and later the initial assignment to the condition variable. The + // statement in the for loop is very specifically an assignment. + sig = des->find_signal(scope, id1->path()); + if (sig == 0) { + cerr << id1->get_fileline() << ": register ``" << id1->path() + << "'' unknown in " << scope_path(scope) << "." << endl; + des->errors += 1; + return 0; + } + + // Make the r-value of the initial assignment, and size it + // properly. Then use it to build the assignment statement. + initial_expr = elaborate_rval_expr(des, scope, sig->net_type(), + expr1_); + if (!initial_expr) + error_flag = true; + + if (debug_elaborate && initial_expr) { + cerr << get_fileline() << ": debug: FOR initial assign: " + << sig->name() << " = " << *initial_expr << endl; + } + + } else { + cerr << get_fileline() << ": internal error: " + << "Index name " << *name1_ << " is not a PEIdent." << endl; des->errors += 1; return 0; } - assert(sig); - /* Make the r-value of the initial assignment, and size it - properly. Then use it to build the assignment statement. */ - initial_expr = elaborate_rval_expr(des, scope, sig->net_type(), - expr1_); - - if (debug_elaborate && initial_expr) { - cerr << get_fileline() << ": debug: FOR initial assign: " - << sig->name() << " = " << *initial_expr << endl; + // Elaborate the statement that is contained in the for + // loop. If there is an error, this will return 0 and I should + // skip the append. No need to worry, the error has been + // reported so it's OK that the netlist is bogus. + NetProc*sub; + if (statement_) { + sub = statement_->elaborate(des, scope); + if (sub == 0) + error_flag = true; + } else { + sub = new NetBlock(NetBlock::SEQU, 0); } - /* Elaborate the statement that is contained in the for - loop. If there is an error, this will return 0 and I should - skip the append. No need to worry, the error has been - reported so it's OK that the netlist is bogus. */ - NetProc*sub; - if (statement_) - sub = statement_->elaborate(des, scope); - else - sub = new NetBlock(NetBlock::SEQU, 0); - - /* Now elaborate the for_step statement. I really should do - some error checking here to make sure the step statement - really does step the variable. */ + // Now elaborate the for_step statement. I really should do + // some error checking here to make sure the step statement + // really does step the variable. NetProc*step = step_->elaborate(des, scope); + if (!step) + error_flag = true; - /* Elaborate the condition expression. Try to evaluate it too, - in case it is a constant. This is an interesting case - worthy of a warning. */ + // Elaborate the condition expression. Try to evaluate it too, + // in case it is a constant. This is an interesting case + // worthy of a warning. NetExpr*ce = elab_and_eval(des, scope, cond_, -1); + if (!ce) + error_flag = true; if (dynamic_cast(ce)) { cerr << get_fileline() << ": warning: condition expression " "of for-loop is constant." << endl; } - /* Error recovery - if we failed to elaborate any of the loop - expressions, give up now. */ - if (initial_expr == 0 || ce == 0 || step == 0 || sub == 0) { + // Error recovery - if we failed to elaborate any of the loop + // expressions, give up now. Error counts where handled elsewhere. + if (error_flag) { delete initial_expr; delete ce; delete step; delete sub; return 0; } - /* All done, build up the loop. */ + + // All done, build up the loop. Note that sig and initial_expr may be + // nil. But if one is nil, then so is the other. The follow-on code + // can handle that case, but let's make sure with an assert that we + // have a consistent input. + ivl_assert(*this, sig || !initial_expr); NetForLoop*loop = new NetForLoop(sig, initial_expr, ce, sub, step); loop->set_line(*this); diff --git a/net_proc.cc b/net_proc.cc index 650be2af5..ffe8d82a7 100644 --- a/net_proc.cc +++ b/net_proc.cc @@ -207,12 +207,19 @@ NetForLoop::NetForLoop(NetNet*ind, NetExpr*iexpr, NetExpr*cond, NetProc*sub, Net void NetForLoop::wrap_up() { NetBlock*top = new NetBlock(NetBlock::SEQU, 0); - top->set_line(*this); - NetAssign_*lv = new NetAssign_(index_); - NetAssign*set_stmt = new NetAssign(lv, init_expr_); - set_stmt->set_line(*init_expr_); - top->append(set_stmt); + // Handle the case that we are missing the initialization + // statement. This can happen for example with statments like this: + // for ( ; ; ) ; + // If the index_ and init_expr_ are present, then generate the + // inital assignment and push it into the sequential block + if (index_ || init_expr_) { + top->set_line(*this); + NetAssign_*lv = new NetAssign_(index_); + NetAssign*set_stmt = new NetAssign(lv, init_expr_); + set_stmt->set_line(*init_expr_); + top->append(set_stmt); + } NetBlock*internal_block = new NetBlock(NetBlock::SEQU, 0); internal_block->set_line(*this); diff --git a/parse.y b/parse.y index a4d427704..303f9cdb6 100644 --- a/parse.y +++ b/parse.y @@ -1,7 +1,7 @@ %{ /* - * Copyright (c) 1998-2021 Stephen Williams (steve@icarus.com) + * Copyright (c) 1998-2022 Stephen Williams (steve@icarus.com) * Copyright CERN 2012-2013 / Stephen Williams (steve@icarus.com) * * This source code is free software; you can redistribute it @@ -1623,6 +1623,14 @@ loop_statement /* IEEE1800-2005: A.6.8 */ $$ = tmp; } + // The initialization statement is optional. + | K_for '(' ';' expression ';' for_step ')' + statement_or_null + { PForStatement*tmp = new PForStatement(nullptr, nullptr, $4, $6, $8); + FILE_NAME(tmp, @1); + $$ = tmp; + } + // Handle for_variable_declaration syntax by wrapping the for(...) // statement in a synthetic named block. We can name the block // after the variable that we are creating, that identifier is diff --git a/pform_dump.cc b/pform_dump.cc index 15c7504e9..627aa2bb3 100644 --- a/pform_dump.cc +++ b/pform_dump.cc @@ -1142,8 +1142,22 @@ void PForever::dump(ostream&out, unsigned ind) const void PForStatement::dump(ostream&out, unsigned ind) const { - out << setw(ind) << "" << "for (" << *name1_ << " = " << *expr1_ - << "; " << *cond_ << "; )" << endl; + out << setw(ind) << "" << "for ("; + if (name1_) + out << *name1_; + else + out << ""; + out << " = "; + if (expr1_) + out << *expr1_; + else + out << ""; + out << "; "; + if (cond_) + out << *cond_; + else + out << ""; + out << "; )" << endl; step_->dump(out, ind+6); if (statement_) statement_->dump(out, ind+3); diff --git a/synth2.cc b/synth2.cc index 3d5e2ccda..251ab4a79 100644 --- a/synth2.cc +++ b/synth2.cc @@ -1453,6 +1453,12 @@ bool NetForLoop::synth_async(Design*des, NetScope*scope, NexusSet&nex_map, NetBus&nex_out, NetBus&enables, vector&bitmasks) { + if (!index_) { + cerr << get_fileline() << ": sorry: Unable to synthesize for-loop without explicit index variable." << endl; + return false; + } + + ivl_assert(*this, index_ && init_expr_); if (debug_synth2) { cerr << get_fileline() << ": NetForLoop::synth_async: " << "Index variable is " << index_->name() << endl; From afe2cd63ef940433809e88e48f4066e56fe20c1a Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Sun, 11 Dec 2022 14:55:18 -0800 Subject: [PATCH 2/2] Regression test for github issue 801 --- ivtest/ivltests/br_gh801.v | 16 ++++++++++++++++ ivtest/regress-sv.list | 1 + 2 files changed, 17 insertions(+) create mode 100644 ivtest/ivltests/br_gh801.v diff --git a/ivtest/ivltests/br_gh801.v b/ivtest/ivltests/br_gh801.v new file mode 100644 index 000000000..fbc4e6891 --- /dev/null +++ b/ivtest/ivltests/br_gh801.v @@ -0,0 +1,16 @@ + +module main; + initial begin + int idx; + idx = 1; + for ( ; idx < 5 ; idx += 1) begin + $display("... %02d", idx); + end + if (idx !== 5) begin + $display("FAILED -- idx=%0d", idx); + $finish; + end + $display("PASSED"); + $finish; + end +endmodule // main diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index f9456c777..f0a8003e2 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -221,6 +221,7 @@ br_gh672 normal,-g2009 ivltests br_gh699 CE,-g2009 ivltests br_gh756 normal,-g2009 ivltests br_gh782a normal,-g2009 ivltests gold=br_gh782a.gold +br_gh801 normal,-g2009 ivltests br_ml20171017 normal,-g2009 ivltests br_ml20180227 CE,-g2009 ivltests br_ml20180309a normal,-g2009 ivltests