From 7335731753ffcf99b68ddcd86ba3a1686569636b Mon Sep 17 00:00:00 2001 From: Ethan Mahintorabi Date: Mon, 22 Apr 2024 21:54:12 +0000 Subject: [PATCH 1/4] Fixes memory leak in verilog attribute code. There appears to be two leaks one with string handling, and one with Dcl not freeing the attribute stmt list when it returns nullptr. Signed-off-by: Ethan Mahintorabi --- verilog/VerilogParse.yy | 9 +++++++-- verilog/VerilogReader.cc | 5 ++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/verilog/VerilogParse.yy b/verilog/VerilogParse.yy index a7752bd6..17588bc3 100644 --- a/verilog/VerilogParse.yy +++ b/verilog/VerilogParse.yy @@ -505,9 +505,14 @@ attr_specs: attr_spec: ID - { $$ = new sta::VerilogAttributeEntry($1, "1"); } + { $$ = new sta::VerilogAttributeEntry($1, "1"); + delete[] $1; + } | ID '=' attr_spec_value - { $$ = new sta::VerilogAttributeEntry($1, $3); } + { $$ = new sta::VerilogAttributeEntry($1, $3); + delete[] $1; + delete[] $3; + } ; attr_spec_value: diff --git a/verilog/VerilogReader.cc b/verilog/VerilogReader.cc index c3649785..8980c7a0 100644 --- a/verilog/VerilogReader.cc +++ b/verilog/VerilogReader.cc @@ -424,8 +424,11 @@ VerilogReader::makeDcl(PortDirection *dir, dcl_count_++; return new VerilogDcl(dir, assign_args, attribute_stmts, line); } - else + else { + attribute_stmts->deleteContents(); + delete attribute_stmts; return nullptr; + } } else { dcl_count_++; From 8ae487c602f5e80ddb9a869339fd747c566e2d66 Mon Sep 17 00:00:00 2001 From: James Cherry Date: Tue, 23 Apr 2024 12:57:06 -0700 Subject: [PATCH 2/4] write_timing_model leak Signed-off-by: James Cherry --- include/sta/Sdc.hh | 16 +++++----- sdc/Sdc.cc | 62 ++++++++++++++++-------------------- search/MakeTimingModel.cc | 19 ++++++----- search/MakeTimingModelPvt.hh | 1 + 4 files changed, 48 insertions(+), 50 deletions(-) diff --git a/include/sta/Sdc.hh b/include/sta/Sdc.hh index 2c730191..ba5fc14e 100644 --- a/include/sta/Sdc.hh +++ b/include/sta/Sdc.hh @@ -239,8 +239,8 @@ public: const RiseFall *rf, const EarlyLate *early_late) const; void unsetTimingDerate(); - static void moveDeratingFactors(Sdc *from, - Sdc *to); + static void swapDeratingFactors(Sdc *sdc1, + Sdc *sdc2); void setInputSlew(const Port *port, const RiseFallBoth *rf, @@ -445,8 +445,8 @@ public: float delay); void removeClockInsertion(const Clock *clk, const Pin *pin); - static void moveClockInsertions(Sdc *from, - Sdc *to); + static void swapClockInsertions(Sdc *sdc1, + Sdc *sdc2); bool hasClockInsertion(const Pin *pin) const; float clockInsertion(const Clock *clk, const RiseFall *rf, @@ -569,8 +569,8 @@ public: const Clock *clk, const RiseFall *clk_rf, const MinMaxAll *min_max); - static void movePortDelays(Sdc *from, - Sdc *to); + static void swapPortDelays(Sdc *sdc1, + Sdc *sdc2); // Set port external pin load (set_load -pin_load port). void setPortExtPinCap(const Port *port, @@ -585,8 +585,8 @@ public: const Corner *corner, const MinMax *min_max, float cap); - static void movePortExtCaps(Sdc *from, - Sdc *to); + static void swapPortExtCaps(Sdc *sdc1, + Sdc *sdc2); // Remove all "set_load net" annotations. void removeNetLoadCaps(); void setNetWireCap(const Net *net, diff --git a/sdc/Sdc.cc b/sdc/Sdc.cc index c78edd4d..bbf0106e 100644 --- a/sdc/Sdc.cc +++ b/sdc/Sdc.cc @@ -51,6 +51,8 @@ namespace sta { +using std::swap; + bool ClockPairLess::operator()(const ClockPair &pair1, const ClockPair &pair2) const @@ -660,17 +662,13 @@ Sdc::unsetTimingDerate() } void -Sdc::moveDeratingFactors(Sdc *from, - Sdc *to) +Sdc::swapDeratingFactors(Sdc *sdc1, + Sdc *sdc2) { - if (from->derating_factors_) { - to->derating_factors_ = from->derating_factors_; - from->derating_factors_ = nullptr; - } - - to->net_derating_factors_ = std::move(from->net_derating_factors_); - to->inst_derating_factors_ = std::move(from->inst_derating_factors_); - to->cell_derating_factors_ = std::move(from->cell_derating_factors_); + swap(sdc1->derating_factors_, sdc2->derating_factors_); + swap(sdc1->net_derating_factors_, sdc2->net_derating_factors_); + swap(sdc1->inst_derating_factors_, sdc2->inst_derating_factors_); + swap(sdc1->cell_derating_factors_, sdc2->cell_derating_factors_); } void @@ -1733,10 +1731,10 @@ Sdc::removeClockInsertion(const Clock *clk, } void -Sdc::moveClockInsertions(Sdc *from, - Sdc *to) +Sdc::swapClockInsertions(Sdc *sdc1, + Sdc *sdc2) { - to->clk_insertions_ = std::move(from->clk_insertions_); + swap(sdc1->clk_insertions_, sdc2->clk_insertions_); } void @@ -2742,21 +2740,20 @@ Sdc::deleteInputDelay(InputDelay *input_delay) } void -Sdc::movePortDelays(Sdc *from, - Sdc *to) +Sdc::swapPortDelays(Sdc *sdc1, + Sdc *sdc2) { - to->input_delays_ = std::move(from->input_delays_); - to->input_delay_pin_map_ = std::move(from->input_delay_pin_map_); - to->input_delay_ref_pin_map_ = std::move(from->input_delay_ref_pin_map_); - to->input_delay_leaf_pin_map_ = std::move(from->input_delay_leaf_pin_map_); - to->input_delay_internal_pin_map_ = std::move(from->input_delay_internal_pin_map_); - to->input_delay_index_ = from->input_delay_index_; - from->input_delay_index_ = 0; + swap(sdc1->input_delays_, sdc2->input_delays_); + swap(sdc1->input_delay_pin_map_, sdc2->input_delay_pin_map_); + swap(sdc1->input_delay_ref_pin_map_, sdc2->input_delay_ref_pin_map_); + swap(sdc1->input_delay_leaf_pin_map_, sdc2->input_delay_leaf_pin_map_); + swap(sdc1->input_delay_internal_pin_map_, sdc2->input_delay_internal_pin_map_); + swap(sdc1->input_delay_index_, sdc2->input_delay_index_); - to->output_delays_ = std::move(from->output_delays_); - to->output_delay_pin_map_ = std::move(from->output_delay_pin_map_); - to->output_delay_ref_pin_map_ = std::move(from->output_delay_ref_pin_map_); - to->output_delay_leaf_pin_map_ = std::move(from->output_delay_leaf_pin_map_); + swap(sdc1->output_delays_, sdc2->output_delays_); + swap(sdc1->output_delay_pin_map_, sdc2->output_delay_pin_map_); + swap(sdc1->output_delay_ref_pin_map_, sdc2->output_delay_ref_pin_map_); + swap(sdc1->output_delay_leaf_pin_map_, sdc2->output_delay_leaf_pin_map_); } //////////////////////////////////////////////////////////////// @@ -3322,15 +3319,12 @@ Sdc::ensurePortExtPinCap(const Port *port, } void -Sdc::movePortExtCaps(Sdc *from, - Sdc *to) +Sdc::swapPortExtCaps(Sdc *sdc1, + Sdc *sdc2) { - for (int corner_index = 0; corner_index < from->corners()->count(); corner_index++) { - to->port_ext_cap_maps_[corner_index] = - std::move(from->port_ext_cap_maps_[corner_index]); - - to->net_wire_cap_maps_[corner_index] = - std::move(from->net_wire_cap_maps_[corner_index]); + for (int corner_index = 0; corner_index < sdc1->corners()->count(); corner_index++) { + swap(sdc1->port_ext_cap_maps_[corner_index], sdc2->port_ext_cap_maps_[corner_index]); + swap(sdc1->net_wire_cap_maps_[corner_index], sdc2->net_wire_cap_maps_[corner_index]); } } diff --git a/search/MakeTimingModel.cc b/search/MakeTimingModel.cc index 10867949..eb6941b5 100644 --- a/search/MakeTimingModel.cc +++ b/search/MakeTimingModel.cc @@ -111,24 +111,27 @@ void MakeTimingModel::saveSdc() { sdc_backup_ = new Sdc(this); - Sdc::movePortDelays(sdc_, sdc_backup_); - Sdc::movePortExtCaps(sdc_, sdc_backup_); - Sdc::moveDeratingFactors(sdc_, sdc_backup_); - Sdc::moveClockInsertions(sdc_, sdc_backup_); + swapSdcWithBackup(); sta_->delaysInvalid(); } void MakeTimingModel::restoreSdc() { - Sdc::movePortDelays(sdc_backup_, sdc_); - Sdc::movePortExtCaps(sdc_backup_, sdc_); - Sdc::moveDeratingFactors(sdc_backup_, sdc_); - Sdc::moveClockInsertions(sdc_backup_, sdc_); + swapSdcWithBackup(); delete sdc_backup_; sta_->delaysInvalid(); } +void +MakeTimingModel::swapSdcWithBackup() +{ + Sdc::swapPortDelays(sdc_, sdc_backup_); + Sdc::swapPortExtCaps(sdc_, sdc_backup_); + Sdc::swapDeratingFactors(sdc_, sdc_backup_); + Sdc::swapClockInsertions(sdc_, sdc_backup_); +} + void MakeTimingModel::makeLibrary() { diff --git a/search/MakeTimingModelPvt.hh b/search/MakeTimingModelPvt.hh index e7587f6f..7e3b83e4 100644 --- a/search/MakeTimingModelPvt.hh +++ b/search/MakeTimingModelPvt.hh @@ -92,6 +92,7 @@ private: void saveSdc(); void restoreSdc(); + void swapSdcWithBackup(); const char *lib_name_; const char *cell_name_; From 22453cc8bf88339c203733e3d380f6183e4c69a8 Mon Sep 17 00:00:00 2001 From: James Cherry Date: Wed, 24 Apr 2024 08:38:25 -0700 Subject: [PATCH 3/4] Sta::netSlack use connectedPinIterator Signed-off-by: James Cherry --- search/Sta.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/search/Sta.cc b/search/Sta.cc index 01a6766d..fd001418 100644 --- a/search/Sta.cc +++ b/search/Sta.cc @@ -2968,7 +2968,7 @@ Sta::netSlack(const Net *net, { ensureGraph(); Slack slack = MinMax::min()->initValue(); - NetPinIterator *pin_iter = network_->pinIterator(net); + NetConnectedPinIterator *pin_iter = network_->connectedPinIterator(net); while (pin_iter->hasNext()) { const Pin *pin = pin_iter->next(); if (network_->isLoad(pin)) { From fdca0dff7ae32edcecf2008977a084d92dc11190 Mon Sep 17 00:00:00 2001 From: James Cherry Date: Wed, 24 Apr 2024 09:34:21 -0700 Subject: [PATCH 4/4] write_verilog use inout for power/ground Signed-off-by: James Cherry --- verilog/VerilogWriter.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/verilog/VerilogWriter.cc b/verilog/VerilogWriter.cc index e3362da3..799495e5 100644 --- a/verilog/VerilogWriter.cc +++ b/verilog/VerilogWriter.cc @@ -214,9 +214,9 @@ VerilogWriter::verilogPortDir(PortDirection *dir) else if (dir == PortDirection::bidirect()) return "inout"; else if (dir == PortDirection::power()) - return "input"; + return "inout"; else if (dir == PortDirection::ground()) - return "input"; + return "inout"; else if (dir == PortDirection::internal()) return nullptr; else { @@ -412,7 +412,8 @@ VerilogWriter::writeAssigns(Instance *inst) Net *net = network_->net(term); Port *port = network_->port(pin); if (port - && network_->direction(port)->isAnyOutput() + && (network_->direction(port)->isAnyOutput() + || network_->direction(port)->isPowerGround()) && !stringEqual(network_->name(port), network_->name(net))) { // Port name is different from net name. string port_vname = netVerilogName(network_->name(port),