From c4a70a8cc32e3eefca678d2d0ed569078423f994 Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Mon, 4 Sep 2017 21:49:56 -0700 Subject: [PATCH 1/6] Fixed typo in comment. Fixed bug where extract_counter would create up counters when it meant to create down counters. --- passes/techmap/extract_counter.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/passes/techmap/extract_counter.cc b/passes/techmap/extract_counter.cc index 540b1593d..de374ab2b 100644 --- a/passes/techmap/extract_counter.cc +++ b/passes/techmap/extract_counter.cc @@ -424,12 +424,12 @@ void counter_worker( cell->setPort("\\CLK", extract.clk); cell->setPort("\\OUT", extract.outsig); - //Hook up hard-wired ports (for now CE and up/=down are not supported), default to no parallel output + //Hook up hard-wired ports (for now CE and up/down are not supported), default to no parallel output cell->setParam("\\HAS_POUT", RTLIL::Const(0)); cell->setParam("\\HAS_CE", RTLIL::Const(0)); cell->setParam("\\DIRECTION", RTLIL::Const("DOWN")); cell->setPort("\\CE", RTLIL::Const(1)); - cell->setPort("\\UP", RTLIL::Const(1)); + cell->setPort("\\UP", RTLIL::Const(0)); //Hook up any parallel outputs for(auto load : extract.pouts) From a84172b23bce1429dedef1eb08fdb25393ba72b6 Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Wed, 13 Sep 2017 10:58:41 -0700 Subject: [PATCH 2/6] Initial support for extraction of counters with clock enable --- passes/techmap/extract_counter.cc | 75 +++++++++++++++++++++++---- techlibs/greenpak4/cells_map.v | 86 +++++++++++++++++++++++-------- 2 files changed, 131 insertions(+), 30 deletions(-) diff --git a/passes/techmap/extract_counter.cc b/passes/techmap/extract_counter.cc index de374ab2b..ec34d9cd1 100644 --- a/passes/techmap/extract_counter.cc +++ b/passes/techmap/extract_counter.cc @@ -92,9 +92,11 @@ struct CounterExtraction int width; //counter width RTLIL::Wire* rwire; //the register output bool has_reset; //true if we have a reset + bool has_ce; //true if we have a clock enable RTLIL::SigSpec rst; //reset pin int count_value; //value we count from - RTLIL::SigSpec clk; //clock signal + RTLIL::SigSpec ce; //clock signal + RTLIL::SigSpec clk; //clock enable, if any RTLIL::SigSpec outsig; //counter output signal RTLIL::Cell* count_mux; //counter mux RTLIL::Cell* count_reg; //counter register @@ -190,12 +192,43 @@ int counter_tryextract( return 13; extract.underflow_inv = underflow_inv; - //Y connection of the mux must have exactly one load, the counter's internal register + //Y connection of the mux must have exactly one load, the counter's internal register, if there's no clock enable + //If we have a clock enable, Y drives the B input of a mux. A of that mux must come from our register const RTLIL::SigSpec muxy = sigmap(count_mux->getPort("\\Y")); pool muxy_loads = get_other_cells(muxy, index, count_mux); if(muxy_loads.size() != 1) return 14; - Cell* count_reg = *muxy_loads.begin(); + Cell* muxload = *muxy_loads.begin(); + Cell* count_reg = muxload; + Cell* cemux = NULL; + RTLIL::SigSpec cey; + if(muxload->type == "$mux") + { + //This mux is probably a clock enable mux. + //Find our count register (should be our only load) + cemux = muxload; + cey = sigmap(cemux->getPort("\\Y")); + pool cey_loads = get_other_cells(cey, index, cemux); + if(cey_loads.size() != 1) + return 24; + count_reg = *cey_loads.begin(); + + //Mux should have A driven by count Q, and B by muxy + //TODO: if A and B are swapped, CE polarity is inverted + if(sigmap(cemux->getPort("\\B")) != muxy) + return 24; + if(sigmap(cemux->getPort("\\A")) != sigmap(count_reg->getPort("\\Q"))) + return 24; + if(sigmap(cemux->getPort("\\Y")) != sigmap(count_reg->getPort("\\D"))) + return 24; + + //Select of the mux is our clock enable + extract.has_ce = true; + extract.ce = sigmap(cemux->getPort("\\S")); + } + else + extract.has_ce = false; + extract.count_reg = count_reg; if(count_reg->type == "$dff") extract.has_reset = false; @@ -216,17 +249,30 @@ int counter_tryextract( //TODO: support synchronous reset else return 15; - if(!is_full_bus(muxy, index, count_mux, "\\Y", count_reg, "\\D")) + + //Sanity check that we use the ALU output properly + if(extract.has_ce) + { + if(!is_full_bus(muxy, index, count_mux, "\\Y", cemux, "\\B")) + return 16; + if(!is_full_bus(cey, index, cemux, "\\Y", count_reg, "\\D")) + return 16; + } + else if(!is_full_bus(muxy, index, count_mux, "\\Y", count_reg, "\\D")) return 16; //TODO: Verify count_reg CLK_POLARITY is 1 //Register output must have exactly two loads, the inverter and ALU //(unless we have a parallel output!) + //If we have a clock enable, 3 is OK const RTLIL::SigSpec qport = count_reg->getPort("\\Q"); const RTLIL::SigSpec cnout = sigmap(qport); pool cnout_loads = get_other_cells(cnout, index, count_reg); - if(cnout_loads.size() > 2) + unsigned int max_loads = 2; + if(extract.has_ce) + max_loads = 3; + if(cnout_loads.size() > max_loads) { //If we specified a limited set of cells for parallel output, check that we only drive them if(!parallel_cells.empty()) @@ -237,6 +283,8 @@ int counter_tryextract( continue; if(c == cell) continue; + if(c == muxload) + continue; //Make sure we're in the whitelist if( parallel_cells.find(c->type) == parallel_cells.end()) @@ -346,7 +394,7 @@ void counter_worker( //Do nothing, unless extraction was forced in which case give an error if(reason != 0) { - static const char* reasons[24]= + static const char* reasons[25]= { "no problem", //0 "counter is too large/small", //1 @@ -371,7 +419,8 @@ void counter_worker( "No init value found", //20 "Underflow value is not equal to init value", //21 "Reset polarity is not positive", //22 - "Reset is not to zero" //23 + "Reset is not to zero", //23 + "Clock enable configuration is unsupported" //24 }; if(force_extract) @@ -424,9 +473,17 @@ void counter_worker( cell->setPort("\\CLK", extract.clk); cell->setPort("\\OUT", extract.outsig); - //Hook up hard-wired ports (for now CE and up/down are not supported), default to no parallel output + //Hook up clock enable + if(extract.has_ce) + { + cell->setParam("\\HAS_CE", RTLIL::Const(1)); + cell->setPort("\\CE", extract.ce); + } + else + cell->setParam("\\HAS_CE", RTLIL::Const(0)); + + //Hook up hard-wired ports (for now up/down are not supported), default to no parallel output cell->setParam("\\HAS_POUT", RTLIL::Const(0)); - cell->setParam("\\HAS_CE", RTLIL::Const(0)); cell->setParam("\\DIRECTION", RTLIL::Const("DOWN")); cell->setPort("\\CE", RTLIL::Const(1)); cell->setPort("\\UP", RTLIL::Const(0)); diff --git a/techlibs/greenpak4/cells_map.v b/techlibs/greenpak4/cells_map.v index b0ec9fd3e..9897d4e35 100644 --- a/techlibs/greenpak4/cells_map.v +++ b/techlibs/greenpak4/cells_map.v @@ -161,8 +161,8 @@ module \$__COUNT_ (CE, CLK, OUT, POUT, RST, UP); parameter WIDTH = 8; parameter DIRECTION = "DOWN"; - //If we have a CE, or DIRECTION other than DOWN fail... GP_COUNTx_ADV is not supported yet - if(HAS_CE || (DIRECTION != "DOWN") ) begin + //If we have a DIRECTION other than DOWN fail... GP_COUNTx_ADV is not supported yet + if(DIRECTION != "DOWN") begin initial begin $display("ERROR: \$__COUNT_ support for GP_COUNTx_ADV is not yet implemented. This counter should never have been extracted (bug in extract_counter pass?)."); $finish; @@ -187,28 +187,72 @@ module \$__COUNT_ (CE, CLK, OUT, POUT, RST, UP); //Looks like a legal counter! Do something with it else if(WIDTH <= 8) begin - GP_COUNT8 #( - .COUNT_TO(COUNT_TO), - .RESET_MODE(RESET_MODE), - .CLKIN_DIVIDE(1) - ) _TECHMAP_REPLACE_ ( - .CLK(CLK), - .RST(RST), - .OUT(OUT), - .POUT(POUT) - ); + if(HAS_CE) begin + wire ce_not; + GP_INV ceinv( + .IN(CE), + .OUT(ce_not) + ); + GP_COUNT8_ADV #( + .COUNT_TO(COUNT_TO), + .RESET_MODE(RESET_MODE), + .RESET_VALUE("COUNT_TO"), + .CLKIN_DIVIDE(1) + ) _TECHMAP_REPLACE_ ( + .CLK(CLK), + .RST(RST), + .OUT(OUT), + .UP(1'b0), //always count down for now + .KEEP(ce_not), + .POUT(POUT) + ); + end + else begin + GP_COUNT8 #( + .COUNT_TO(COUNT_TO), + .RESET_MODE(RESET_MODE), + .CLKIN_DIVIDE(1) + ) _TECHMAP_REPLACE_ ( + .CLK(CLK), + .RST(RST), + .OUT(OUT), + .POUT(POUT) + ); + end end else begin - GP_COUNT14 #( - .COUNT_TO(COUNT_TO), - .RESET_MODE(RESET_MODE), - .CLKIN_DIVIDE(1) - ) _TECHMAP_REPLACE_ ( - .CLK(CLK), - .RST(RST), - .OUT(OUT) - ); + if(HAS_CE) begin + wire ce_not; + GP_INV ceinv( + .IN(CE), + .OUT(ce_not) + ); + GP_COUNT14_ADV #( + .COUNT_TO(COUNT_TO), + .RESET_MODE(RESET_MODE), + .RESET_VALUE("COUNT_TO"), + .CLKIN_DIVIDE(1) + ) _TECHMAP_REPLACE_ ( + .CLK(CLK), + .RST(RST), + .OUT(OUT), + .UP(1'b0), //always count down for now + .KEEP(ce_not), + .POUT(POUT) + ); + end + else begin + GP_COUNT14 #( + .COUNT_TO(COUNT_TO), + .RESET_MODE(RESET_MODE), + .CLKIN_DIVIDE(1) + ) _TECHMAP_REPLACE_ ( + .CLK(CLK), + .RST(RST), + .OUT(OUT) + ); + end end endmodule From 0484ad666d765c84d7a06a97ee9c7f6280733ed8 Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Wed, 13 Sep 2017 15:32:20 -0700 Subject: [PATCH 3/6] Added support for inferring counters with active-low reset --- passes/techmap/extract_counter.cc | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/passes/techmap/extract_counter.cc b/passes/techmap/extract_counter.cc index ec34d9cd1..3746a3cb6 100644 --- a/passes/techmap/extract_counter.cc +++ b/passes/techmap/extract_counter.cc @@ -94,6 +94,7 @@ struct CounterExtraction bool has_reset; //true if we have a reset bool has_ce; //true if we have a clock enable RTLIL::SigSpec rst; //reset pin + bool rst_inverted; //true if reset is active low int count_value; //value we count from RTLIL::SigSpec ce; //clock signal RTLIL::SigSpec clk; //clock enable, if any @@ -236,10 +237,9 @@ int counter_tryextract( { extract.has_reset = true; - //Verify ARST_VALUE is zero and ARST_POLARITY is 1 - //TODO: infer an inverter to make it 1 if necessary, so we can support negative level resets? - if(count_reg->getParam("\\ARST_POLARITY").as_int() != 1) - return 22; + //Verify ARST_VALUE is zero. + //Detect polarity inversions on reset. + extract.rst_inverted = (count_reg->getParam("\\ARST_POLARITY").as_int() != 1); if(count_reg->getParam("\\ARST_VALUE").as_int() != 0) return 23; @@ -418,7 +418,7 @@ void counter_worker( "Register output is not full bus", //19 "No init value found", //20 "Underflow value is not equal to init value", //21 - "Reset polarity is not positive", //22 + "RESERVED, not implemented", //22, kept for compatibility but not used anymore "Reset is not to zero", //23 "Clock enable configuration is unsupported" //24 }; @@ -458,7 +458,16 @@ void counter_worker( { //TODO: support other kinds of reset cell->setParam("\\RESET_MODE", RTLIL::Const("LEVEL")); - cell->setPort("\\RST", extract.rst); + + //If the reset is active low, infer an inverter ($__COUNT_ cells always have active high reset) + if(extract.rst_inverted) + { + auto realreset = cell->module->addWire(NEW_ID); + cell->module->addNot(NEW_ID, extract.rst, RTLIL::SigSpec(realreset)); + cell->setPort("\\RST", realreset); + } + else + cell->setPort("\\RST", extract.rst); } else { From 122532b7e137a53a91be155668687246a25d27e7 Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Wed, 13 Sep 2017 15:47:06 -0700 Subject: [PATCH 4/6] Added RESET_TO_MAX parameter to $__COUNT_ cell. Cannot yet be extracted. --- passes/techmap/extract_counter.cc | 1 + techlibs/greenpak4/cells_blackbox.v | 1 + techlibs/greenpak4/cells_map.v | 5 +++-- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/passes/techmap/extract_counter.cc b/passes/techmap/extract_counter.cc index 3746a3cb6..1e20b1fd8 100644 --- a/passes/techmap/extract_counter.cc +++ b/passes/techmap/extract_counter.cc @@ -493,6 +493,7 @@ void counter_worker( //Hook up hard-wired ports (for now up/down are not supported), default to no parallel output cell->setParam("\\HAS_POUT", RTLIL::Const(0)); + cell->setParam("\\RESET_TO_MAX", RTLIL::Const(0)); cell->setParam("\\DIRECTION", RTLIL::Const("DOWN")); cell->setPort("\\CE", RTLIL::Const(1)); cell->setPort("\\UP", RTLIL::Const(0)); diff --git a/techlibs/greenpak4/cells_blackbox.v b/techlibs/greenpak4/cells_blackbox.v index 8dd38a027..1895b90de 100644 --- a/techlibs/greenpak4/cells_blackbox.v +++ b/techlibs/greenpak4/cells_blackbox.v @@ -9,6 +9,7 @@ module \$__COUNT_ (CE, CLK, OUT, POUT, RST, UP); parameter COUNT_TO = 1; parameter RESET_MODE = "RISING"; + parameter RESET_TO_MAX = "1"; parameter HAS_POUT = 0; parameter HAS_CE = 0; parameter WIDTH = 8; diff --git a/techlibs/greenpak4/cells_map.v b/techlibs/greenpak4/cells_map.v index 9897d4e35..b971a51fa 100644 --- a/techlibs/greenpak4/cells_map.v +++ b/techlibs/greenpak4/cells_map.v @@ -156,6 +156,7 @@ module \$__COUNT_ (CE, CLK, OUT, POUT, RST, UP); parameter COUNT_TO = 1; parameter RESET_MODE = "RISING"; + parameter RESET_TO_MAX = 0; parameter HAS_POUT = 0; parameter HAS_CE = 0; parameter WIDTH = 8; @@ -196,7 +197,7 @@ module \$__COUNT_ (CE, CLK, OUT, POUT, RST, UP); GP_COUNT8_ADV #( .COUNT_TO(COUNT_TO), .RESET_MODE(RESET_MODE), - .RESET_VALUE("COUNT_TO"), + .RESET_VALUE(RESET_TO_MAX ? "COUNT_TO" : "ZERO"), .CLKIN_DIVIDE(1) ) _TECHMAP_REPLACE_ ( .CLK(CLK), @@ -230,7 +231,7 @@ module \$__COUNT_ (CE, CLK, OUT, POUT, RST, UP); ); GP_COUNT14_ADV #( .COUNT_TO(COUNT_TO), - .RESET_MODE(RESET_MODE), + .RESET_MODE(RESET_TO_MAX ? "COUNT_TO" : "ZERO"), .RESET_VALUE("COUNT_TO"), .CLKIN_DIVIDE(1) ) _TECHMAP_REPLACE_ ( From c8f2f082c6e20a50ca7efe8c5a67d26f2e24a4ab Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Wed, 13 Sep 2017 15:57:17 -0700 Subject: [PATCH 5/6] Added support for inferring counters with reset to full scale instead of zero --- passes/techmap/extract_counter.cc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/passes/techmap/extract_counter.cc b/passes/techmap/extract_counter.cc index 1e20b1fd8..27fc558ad 100644 --- a/passes/techmap/extract_counter.cc +++ b/passes/techmap/extract_counter.cc @@ -95,6 +95,7 @@ struct CounterExtraction bool has_ce; //true if we have a clock enable RTLIL::SigSpec rst; //reset pin bool rst_inverted; //true if reset is active low + bool rst_to_max; //true if we reset to max instead of 0 int count_value; //value we count from RTLIL::SigSpec ce; //clock signal RTLIL::SigSpec clk; //clock enable, if any @@ -237,10 +238,16 @@ int counter_tryextract( { extract.has_reset = true; - //Verify ARST_VALUE is zero. - //Detect polarity inversions on reset. + //Check polarity of reset - we may have to add an inverter later on! extract.rst_inverted = (count_reg->getParam("\\ARST_POLARITY").as_int() != 1); - if(count_reg->getParam("\\ARST_VALUE").as_int() != 0) + + //Verify ARST_VALUE is zero or full scale + int rst_value = count_reg->getParam("\\ARST_VALUE").as_int(); + if(rst_value == 0) + extract.rst_to_max = false; + else if(rst_value == extract.count_value) + extract.rst_to_max = true; + else return 23; //Save the reset @@ -419,7 +426,7 @@ void counter_worker( "No init value found", //20 "Underflow value is not equal to init value", //21 "RESERVED, not implemented", //22, kept for compatibility but not used anymore - "Reset is not to zero", //23 + "Reset is not to zero or COUNT_TO", //23 "Clock enable configuration is unsupported" //24 }; From 367d6b2194c6ba2b09b81b6cd2946702cb5ce895 Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Thu, 14 Sep 2017 10:18:49 -0700 Subject: [PATCH 6/6] Fixed bug where counter extraction on non-GreenPAK devices incorrectly handled parallel counter output --- passes/techmap/extract_counter.cc | 59 ++++++++++++++----------------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/passes/techmap/extract_counter.cc b/passes/techmap/extract_counter.cc index 27fc558ad..af0eb852a 100644 --- a/passes/techmap/extract_counter.cc +++ b/passes/techmap/extract_counter.cc @@ -281,44 +281,34 @@ int counter_tryextract( max_loads = 3; if(cnout_loads.size() > max_loads) { - //If we specified a limited set of cells for parallel output, check that we only drive them - if(!parallel_cells.empty()) + for(auto c : cnout_loads) { - for(auto c : cnout_loads) - { - if(c == underflow_inv) - continue; - if(c == cell) - continue; - if(c == muxload) - continue; + if(c == underflow_inv) + continue; + if(c == cell) + continue; + if(c == muxload) + continue; + //If we specified a limited set of cells for parallel output, check that we only drive them + if(!parallel_cells.empty()) + { //Make sure we're in the whitelist if( parallel_cells.find(c->type) == parallel_cells.end()) return 17; + } - //Figure out what port(s) are driven by it - //TODO: this can probably be done more efficiently w/o multiple iterations over our whole net? - RTLIL::IdString portname; - for(auto b : qport) + //Figure out what port(s) are driven by it + //TODO: this can probably be done more efficiently w/o multiple iterations over our whole net? + for(auto b : qport) + { + pool ports = index.query_ports(b); + for(auto x : ports) { - pool ports = index.query_ports(b); - for(auto x : ports) - { - if(x.cell != c) - continue; - if(portname == "") - portname = x.port; - - //somehow our counter output is going to multiple ports - //this makes no sense, don't allow inference - else if(portname != x.port) - return 17; - } + if(x.cell != c) + continue; + extract.pouts.insert(ModIndex::PortInfo(c, x.port, 0)); } - - //Save the other loads - extract.pouts.insert(ModIndex::PortInfo(c, portname, 0)); } } } @@ -529,10 +519,15 @@ void counter_worker( string reset_type = "non-resettable"; if(extract.has_reset) { + if(extract.rst_inverted) + reset_type = "negative"; + else + reset_type = "positive"; + //TODO: support other kind of reset - reset_type = "async resettable"; + reset_type += " async resettable"; } - log(" Found %d-bit %s down counter %s (counting from %d) for register %s declared at %s\n", + log(" Found %d-bit (%s) down counter %s (counting from %d) for register %s, declared at %s\n", extract.width, reset_type.c_str(), countname.c_str(),