From 4e86274ff2c5428d34f3d0c12be59c9841ca5c82 Mon Sep 17 00:00:00 2001 From: Martin Whitaker Date: Thu, 9 Jun 2011 08:39:03 +0100 Subject: [PATCH 1/2] Compiler fix for pr3296466. The compiler was handling bi-directional pass switches using the default case for primitive gates, where the first port is treated as an output and the remaining ports are treated as inputs. This patch adds a special case for pass switches, so that the first two ports are treated as bi-directional. --- PGate.h | 3 +- elaborate.cc | 89 ++++++++++++++++++++++++++++++++-------------------- 2 files changed, 57 insertions(+), 35 deletions(-) diff --git a/PGate.h b/PGate.h index 3a4b9ced4..84fb26b9d 100644 --- a/PGate.h +++ b/PGate.h @@ -166,7 +166,8 @@ class PGBuiltin : public PGate { unsigned calculate_array_count_(Design*, NetScope*, long&high, long&low) const; - unsigned calculate_output_count_(void) const; + void calculate_gate_and_lval_count_(unsigned&gate_count, + unsigned&lval_count) const; NetNode* create_gate_for_output_(Design*, NetScope*, perm_string gate_name, diff --git a/elaborate.cc b/elaborate.cc index 5f549de9a..b5ba65c0f 100644 --- a/elaborate.cc +++ b/elaborate.cc @@ -271,26 +271,35 @@ unsigned PGBuiltin::calculate_array_count_(Design*des, NetScope*scope, return count; } -unsigned PGBuiltin::calculate_output_count_(void) const +void PGBuiltin::calculate_gate_and_lval_count_(unsigned&gate_count, + unsigned&lval_count) const { - unsigned output_count; - switch (type()) { case BUF: case NOT: - if (pin_count() > 2) output_count = pin_count() - 1; - else output_count = 1; + if (pin_count() > 2) gate_count = pin_count() - 1; + else gate_count = 1; + lval_count = gate_count; break; case PULLDOWN: case PULLUP: - output_count = pin_count(); + gate_count = pin_count(); + lval_count = gate_count; + break; + case TRAN: + case RTRAN: + case TRANIF0: + case TRANIF1: + case RTRANIF0: + case RTRANIF1: + gate_count = 1; + lval_count = 2; break; default: - output_count = 1; + gate_count = 1; + lval_count = 1; break; } - - return output_count; } NetNode* PGBuiltin::create_gate_for_output_(Design*des, NetScope*scope, @@ -694,22 +703,28 @@ void PGBuiltin::elaborate(Design*des, NetScope*scope) const unsigned array_count = calculate_array_count_(des, scope, high, low); if (array_count == 0) return; - unsigned output_count = calculate_output_count_(); + unsigned gate_count = 0, lval_count = 0; + calculate_gate_and_lval_count_(gate_count, lval_count); - /* Now we have a gate count. Elaborate the output expressions - only. We do it early so that we can see if we can make - wide gates instead of an array of gates. */ + /* Now we have a gate count. Elaborate the lval (output or + bi-directional) expressions only. We do it early so that + we can see if we can make wide gates instead of an array + of gates. */ - vectorlval_sigs (output_count); + vectorlval_sigs (lval_count); - for (unsigned idx = 0 ; idx < output_count ; idx += 1) { + for (unsigned idx = 0 ; idx < lval_count ; idx += 1) { if (pin(idx) == 0) { cerr << get_fileline() << ": error: Logic gate port " "expressions are not optional." << endl; des->errors += 1; return; } - lval_sigs[idx] = pin(idx)->elaborate_lnet(des, scope); + if (lval_count > gate_count) + lval_sigs[idx] = pin(idx)->elaborate_bi_net(des, scope); + else + lval_sigs[idx] = pin(idx)->elaborate_lnet(des, scope); + // The only way this should return zero is if an error // happened, so for that case just return. if (lval_sigs[idx] == 0) return; @@ -757,19 +772,19 @@ void PGBuiltin::elaborate(Design*des, NetScope*scope) const des, scope); /* Allocate all the netlist nodes for the gates. */ - vectorcur (array_count*output_count); + vectorcur (array_count*gate_count); /* Now make as many gates as the bit count dictates. Give each a unique name, and set the delay times. */ - for (unsigned idx = 0 ; idx < array_count*output_count ; idx += 1) { - unsigned array_idx = idx/output_count; - unsigned output_idx = idx%output_count; + for (unsigned idx = 0 ; idx < array_count*gate_count ; idx += 1) { + unsigned array_idx = idx/gate_count; + unsigned gate_idx = idx%gate_count; ostringstream tmp; unsigned index = (low < high)? (low+array_idx) : (low-array_idx); - tmp << name << "<" << index << "." << output_idx << ">"; + tmp << name << "<" << index << "." << gate_idx << ">"; perm_string inm = lex_strings.make(tmp.str()); cur[idx] = create_gate_for_output_(des, scope, inm, instance_width); @@ -808,7 +823,7 @@ void PGBuiltin::elaborate(Design*des, NetScope*scope) const return; } NetNet*sig = 0; - if (idx < output_count) { + if (idx < lval_count) { sig = lval_sigs[idx]; } else { @@ -872,11 +887,11 @@ void PGBuiltin::elaborate(Design*des, NetScope*scope) const // Although in Verilog proper a multiple // output gate has only 1 input, this conditional // handles gates with N outputs and M inputs. - if (idx < output_count) { + if (idx < gate_count) { connect(cur[idx]->pin(0), sig->pin(0)); } else { - for (unsigned dev = 0 ; dev < output_count; dev += 1) - connect(cur[dev]->pin(idx-output_count+1), sig->pin(0)); + for (unsigned dev = 0 ; dev < gate_count; dev += 1) + connect(cur[dev]->pin(idx-gate_count+1), sig->pin(0)); } } else if (sig->vector_width() == 1) { @@ -886,26 +901,32 @@ void PGBuiltin::elaborate(Design*des, NetScope*scope) const output port, connect it to all array_count devices that have outputs at this position. Otherwise, idx is an input to all - array_count*output_count devices. */ + array_count*gate_count devices. */ - if (idx < output_count) { + if (idx < gate_count) { for (unsigned gdx = 0 ; gdx < array_count ; gdx += 1) { - unsigned dev = gdx*output_count; + unsigned dev = gdx*gate_count; connect(cur[dev+idx]->pin(0), sig->pin(0)); } } else { - unsigned use_idx = idx - output_count + 1; + unsigned use_idx = idx - gate_count + 1; for (unsigned gdx = 0 ; gdx < cur.size() ; gdx += 1) connect(cur[gdx]->pin(use_idx), sig->pin(0)); } } else if (sig->vector_width() == array_count) { + /* Bi-directional switches should get collapsed into + a single wide instance, so should never reach this + point. Check this is so, as the following code + doesn't handle bi-directional connections. */ + ivl_assert(*this, lval_count == gate_count); + /* Handle the general case that each bit of the value is connected to a different instance. In this case, the output is handled slightly different from the inputs. */ - if (idx < output_count) { + if (idx < gate_count) { NetConcat*cc = new NetConcat(scope, scope->local_symbol(), sig->vector_width(), @@ -918,7 +939,7 @@ void PGBuiltin::elaborate(Design*des, NetScope*scope) const /* Connect the outputs of the gates to the concat. */ for (unsigned gdx = 0 ; gdx < array_count; gdx += 1) { - unsigned dev = gdx*output_count; + unsigned dev = gdx*gate_count; connect(cur[dev+idx]->pin(0), cc->pin(gdx+1)); NetNet*tmp2 = new NetNet(scope, @@ -944,9 +965,9 @@ void PGBuiltin::elaborate(Design*des, NetScope*scope) const tmp2->local_flag(true); tmp2->data_type(sig->data_type()); connect(tmp1->pin(0), tmp2->pin(0)); - unsigned use_idx = idx - output_count + 1; - unsigned dev = gdx*output_count; - for (unsigned gdx2 = 0 ; gdx2 < output_count ; gdx2 += 1) + unsigned use_idx = idx - gate_count + 1; + unsigned dev = gdx*gate_count; + for (unsigned gdx2 = 0 ; gdx2 < gate_count ; gdx2 += 1) connect(cur[dev+gdx2]->pin(use_idx), tmp1->pin(0)); } From 6ffd19cd7eecddc2946255d71c0085655f27aa3a Mon Sep 17 00:00:00 2001 From: Martin Whitaker Date: Sun, 12 Jun 2011 13:10:16 +0100 Subject: [PATCH 2/2] vvp fix for pr3296466. This patch reworks the tran island code to allow it to handle cases where tran primitives cross-connect different bits of the same vector. --- vvp/island_tran.cc | 352 ++++++++++++++------------------------------- vvp/vvp_island.h | 3 +- 2 files changed, 106 insertions(+), 249 deletions(-) diff --git a/vvp/island_tran.cc b/vvp/island_tran.cc index 072f2cac2..97c0c5c20 100644 --- a/vvp/island_tran.cc +++ b/vvp/island_tran.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008-2010 Stephen Williams (steve@icarus.com) + * Copyright (c) 2008-2011 Stephen Williams (steve@icarus.com) * * This source code is free software; you can redistribute it * and/or modify it in source code form under the terms of the GNU @@ -33,34 +33,17 @@ class vvp_island_tran : public vvp_island { struct vvp_island_branch_tran : public vvp_island_branch { - // Behavior. (This stuff should be moved to a derived - // class. The members here are specific to the tran island - // class.) vvp_island_branch_tran(vvp_net_t*en__, bool active_high__, unsigned width__, unsigned part__, unsigned offset__); bool run_test_enabled(); void run_resolution(); - - void clear_resolution_flags() { flags_ &= ~0x0f; } - - void mark_done(unsigned ab) { flags_ |= 1 << ab; } - bool test_done(unsigned ab) const { return flags_ & (1<next_branch) { vvp_island_branch_tran*tmp = dynamic_cast(cur); @@ -108,13 +87,17 @@ void vvp_island_tran::run_island() assert(tmp); tmp->run_resolution(); } + + // Now output the resolved values. + for (vvp_island_branch*cur = branches_ ; cur ; cur = cur->next_branch) { + vvp_island_branch_tran*tmp = dynamic_cast(cur); + assert(tmp); + tmp->run_output(); + } } bool vvp_island_branch_tran::run_test_enabled() { - // Clear all the flags. - clear_resolution_flags(); - vvp_island_port*ep = en? dynamic_cast (en->fun) : 0; // If there is no ep port (no "enabled" input) then this is a @@ -158,260 +141,133 @@ bool vvp_island_branch_tran::run_test_enabled() return true; } -static void island_send_value(list&connections, const vvp_vector8_t&val) +static void push_value_through_branches(const vvp_vector8_t&val, + list&connections); + +static void push_value_through_branch(const vvp_vector8_t&val, + vvp_branch_ptr_t cur) { - for (list::iterator idx = connections.begin() - ; idx != connections.end() ; ++ idx ) { + vvp_island_branch_tran*branch = BRANCH_TRAN(cur.ptr()); - vvp_island_branch*tmp_ptr = idx->ptr(); + // If the branch is not enabled, skip. + if (! branch->enabled_flag) + return; - unsigned tmp_ab = idx->port(); - island_send_value(tmp_ab? tmp_ptr->b : tmp_ptr->a, val); + unsigned src_ab = cur.port(); + unsigned dst_ab = src_ab^1; + + vvp_net_t*dst_net = dst_ab? branch->b : branch->a; + vvp_island_port*dst_port = dynamic_cast(dst_net->fun); + + vvp_vector8_t old_val = dst_port->value; + + // If the port on the other side has not yet been visited, + // get its input value. + if (dst_port->value.size() == 0) + dst_port->value = island_get_value(dst_net); + + // If we don't yet have an initial value for the port, skip. + if (dst_port->value.size() == 0) + return; + + // Now resolve the pushed value with whatever values we have + // previously collected (and resolved) for the port. + if (branch->width == 0) { + // There are no part selects. + dst_port->value = resolve(dst_port->value, val); + + } else if (dst_ab == 1) { + // The other side is a strict subset (part select) + // of this side. + vvp_vector8_t tmp = val.subvalue(branch->offset, branch->part); + dst_port->value = resolve(dst_port->value, tmp); + + } else { + // The other side is a superset of this side. + vvp_vector8_t tmp = part_expand(val, branch->width, branch->offset); + dst_port->value = resolve(dst_port->value, tmp); + } + + // If the resolved value for the port has changed, push the new + // value back into the network. + if (! dst_port->value.eeq(old_val)) { + list connections; + + vvp_branch_ptr_t dst_side(branch, dst_ab); + island_collect_node(connections, dst_side); + + push_value_through_branches(dst_port->value, connections); } } -static void mark_done_flags(list&connections) -{ - for (list::iterator idx = connections.begin() - ; idx != connections.end() ; ++ idx ) { - - vvp_island_branch*tmp_ptr = idx->ptr(); - vvp_island_branch_tran*cur = dynamic_cast(tmp_ptr); - - unsigned tmp_ab = idx->port(); - cur->mark_done(tmp_ab); - } -} - -static void mark_visited_flags(list&connections) -{ - for (list::iterator idx = connections.begin() - ; idx != connections.end() ; ++ idx ) { - - vvp_island_branch*tmp_ptr = idx->ptr(); - vvp_island_branch_tran*cur = dynamic_cast(tmp_ptr); - assert(cur); - - unsigned tmp_ab = idx->port(); - cur->mark_visited(tmp_ab); - } -} - -static void clear_visited_flags(list&connections) -{ - for (list::iterator idx = connections.begin() - ; idx != connections.end() ; ++ idx ) { - - vvp_island_branch_tran*tmp_ptr = BRANCH_TRAN(idx->ptr()); - - unsigned tmp_ab = idx->port(); - tmp_ptr->clear_visited(tmp_ab); - } -} - -static vvp_vector8_t get_value_from_branch(vvp_branch_ptr_t cur); - -static void resolve_values_from_connections(vvp_vector8_t&val, - list&connections) -{ - for (list::iterator idx = connections.begin() - ; idx != connections.end() ; ++ idx ) { - vvp_vector8_t tmp = get_value_from_branch(*idx); - if (val.size() == 0) - val = tmp; - else if (tmp.size() != 0) - val = resolve(val, tmp); - } -} - -static vvp_vector8_t get_value_from_branch(vvp_branch_ptr_t cur) -{ - vvp_island_branch_tran*ptr = BRANCH_TRAN(cur.ptr()); - assert(ptr); - unsigned ab = cur.port(); - unsigned ab_other = ab^1; - - // If the branch link is disabled, return nil. - if (ptr->enabled_flag == false) - return vvp_vector8_t(); - - vvp_branch_ptr_t other (ptr, ab_other); - - // If the branch other side is already visited, return - // nil. This prevents recursion loops. - if (ptr->test_visited(ab_other)) - return vvp_vector8_t(); - - // Other side net, and port value. - vvp_net_t*net_other = ab? ptr->a : ptr->b; - vvp_vector8_t val_other = island_get_value(net_other); - - // recurse - list connections; - island_collect_node(connections, other); - mark_visited_flags(connections); - - resolve_values_from_connections(val_other, connections); - - // Remove/unwind visited flags - clear_visited_flags(connections); - - if (val_other.size() == 0) - return val_other; - - if (ptr->width) { - if (ab == 0) { - val_other = part_expand(val_other, ptr->width, ptr->offset); - - } else { - val_other = val_other.subvalue(ptr->offset, ptr->part); - - } - } - - return val_other; -} - -/* - * Try to recursively push a fully resolved value back through the - * graph. This can save many span iterations through the graph by - * marking as done that are obviously and easily done. But it is - * better to be conservative here. - * - * The connections list is filled with connections that are already - * marked done, and the val is the resolved value. We are going to try - * to follow branches to see if we can push the value further and mark - * the other side done as well. - */ static void push_value_through_branches(const vvp_vector8_t&val, list&connections) { for (list::iterator idx = connections.begin() ; idx != connections.end() ; ++ idx ) { - vvp_island_branch_tran*tmp_ptr = BRANCH_TRAN(idx->ptr()); - unsigned tmp_ab = idx->port(); - unsigned other_ab = tmp_ab^1; - - // If other side already done, skip - if (tmp_ptr->test_done(other_ab)) - continue; - - // If link is not enabled, skip. - if (! tmp_ptr->enabled_flag) - continue; - - vvp_net_t*other_net = other_ab? tmp_ptr->b : tmp_ptr->a; - - if (tmp_ptr->width == 0) { - // There are no part selects, so we can safely - // Mark this end as done. - tmp_ptr->mark_done(other_ab); - island_send_value(other_net, val); - - } else if (other_ab == 1) { - // The other side is a strict subset (part select) - // of this side, so we can mark this end as done. - tmp_ptr->mark_done(other_ab); - vvp_vector8_t tmp = val.subvalue(tmp_ptr->offset, tmp_ptr->part); - island_send_value(other_net, tmp); - - } else { - // Otherwise, the other side is not fully - // specified (is a subset of the done side) so we - // can't take this shortcut. - } + push_value_through_branch(val, *idx); } } /* * This method resolves the value for a branch recursively. It uses - * recursive descent to span the graph of branches, collecting values - * that need to be resolved together. + * recursive descent to span the graph of branches, pushing values + * through the network until a stable state is reached. */ void vvp_island_branch_tran::run_resolution() { - // Collect all the branch endpoints that are joined to my A - // side. list connections; - bool processed_a_side = false; - vvp_vector8_t val; + vvp_island_port*port; - // The "flags" member is a bitmask that marks whether an - // endpoint of a branch has been visited. If flags&1, then the - // A side has been visited. If flags&2, then the B side has - // been visited. The flags help us avoid recursion when doing - // spanning trees. - - // If the A side has already been completed, then skip it. - if (! test_done(0)) { - processed_a_side = true; + // If the A side port hasn't already been visited, then push + // its input value through all the branches connected to it. + port = dynamic_cast(a->fun); + if (port->value.size() == 0) { vvp_branch_ptr_t a_side(this, 0); island_collect_node(connections, a_side); - // Mark my A side as done. Do this early to prevent recursing - // back. All the connections that share this port are also - // done. Make sure their flags are set appropriately. - mark_done_flags(connections); + port->value = island_get_value(a); + if (port->value.size() != 0) + push_value_through_branches(port->value, connections); - // Start with my branch-point value. - val = island_get_value(a); - mark_visited_flags(connections); // Mark as visited. - - - // Now scan the other sides of all the branches connected to - // my A side. The get_value_from_branch() will recurse as - // necessary to depth-first walk the graph. - resolve_values_from_connections(val, connections); - - // A side is done. - island_send_value(connections, val); - - // Clear the visited flags. This must be done so that other - // branches can read this input value. - clear_visited_flags(connections); - - // Try to push the calculated value out through the - // branches. This is useful for A-side results because - // there is a high probability that the other side of - // all the connected branches is fully specified by this - // result. - push_value_through_branches(val, connections); + connections.clear(); } - // If the B side got taken care of by above, then this branch - // is done. Stop now. - if (test_done(1)) - return; + // Do the same for the B side port. Note that if the branch + // is enabled, the B side port will have already been visited + // when we resolved the A side port. + port = dynamic_cast(b->fun); + if (port->value.size() == 0) { + vvp_branch_ptr_t b_side(this, 1); + island_collect_node(connections, b_side); - // Repeat the above for the B side. + port->value = island_get_value(b); + if (port->value.size() != 0) + push_value_through_branches(port->value, connections); - connections.clear(); - island_collect_node(connections, vvp_branch_ptr_t(this, 1)); - mark_done_flags(connections); + connections.clear(); + } +} - if (enabled_flag && processed_a_side) { - // If this is a connected branch, then we know from the - // start that we have all the bits needed to complete - // the B side. Even if the B side is a part select, the - // simple part select must be correct because the - // recursive resolve_values_from_connections above must - // of cycled back to the B side of myself when resolving - // the connections. - if (width != 0) - val = val.subvalue(offset, part); +void vvp_island_branch_tran::run_output() +{ + vvp_island_port*port; - } else { - - // If this branch is not enabled, then the B-side must - // be processed on its own. - val = island_get_value(b); - mark_visited_flags(connections); - resolve_values_from_connections(val, connections); - clear_visited_flags(connections); + // If the A side port hasn't already been updated, send the + // resolved value to the output. + port = dynamic_cast(a->fun); + if (port->value.size() != 0) { + island_send_value(a, port->value); + port->value = vvp_vector8_t::nil; } - island_send_value(connections, val); + // Do the same for the B side port. + port = dynamic_cast(b->fun); + if (port->value.size() != 0) { + island_send_value(b, port->value); + port->value = vvp_vector8_t::nil; + } } void compile_island_tran(char*label) @@ -454,7 +310,7 @@ void compile_island_tranvp(char*island, char*pa, char*pb, free(island); vvp_island_branch_tran*br = new vvp_island_branch_tran(NULL, false, - wid, par, off) ; + wid, par, off); use_island->add_branch(br, pa, pb); diff --git a/vvp/vvp_island.h b/vvp/vvp_island.h index f7203bfbe..afe27e1d5 100644 --- a/vvp/vvp_island.h +++ b/vvp/vvp_island.h @@ -1,7 +1,7 @@ #ifndef __vvp_island_H #define __vvp_island_H /* - * Copyright (c) 2008 Stephen Williams (steve@icarus.com) + * Copyright (c) 2008,2011 Stephen Williams (steve@icarus.com) * * This source code is free software; you can redistribute it * and/or modify it in source code form under the terms of the GNU @@ -139,6 +139,7 @@ class vvp_island_port : public vvp_net_fun_t { public: vvp_vector8_t invalue; vvp_vector8_t outvalue; + vvp_vector8_t value; private: vvp_island*island_;