Skip to content

gpl: change size_t to int #9596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
openroad-ci wants to merge 1 commit into
base: master
Choose a base branch
from
+7 −9
Open

gpl: change size_t to int #9596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions src/gpl/src/nesterovBase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3519,8 +3519,7 @@ void NesterovBaseCommon::createCbkITerm(odb::dbITerm* iTerm)

// assuming fixpointers will be called later
// maintaining consistency in NBC::gcellStor_ and NB::gCells_
std::pair<odb::dbInst*, size_t> NesterovBase::destroyCbkGCell(
odb::dbInst* db_inst)
std::pair<odb::dbInst*, int> NesterovBase::destroyCbkGCell(odb::dbInst* db_inst)
{
debugPrint(log_,
GPL,
Expand All @@ -3530,7 +3529,7 @@ std::pair<odb::dbInst*, size_t> NesterovBase::destroyCbkGCell(
pb_->getGroup() ? pb_->getGroup()->getName() : "Top-level",
db_inst->getName());
auto db_it = db_inst_to_nb_index_.find(db_inst);
std::pair<odb::dbInst*, size_t> replacement = {nullptr, -1};
std::pair<odb::dbInst*, int> replacement = {nullptr, -1};
if (db_it != db_inst_to_nb_index_.end()) {
size_t last_index = nb_gcells_.size() - 1;
size_t gcell_index = db_it->second;
Expand Down Expand Up @@ -3600,10 +3599,10 @@ bool NesterovBase::updateHandle(odb::dbInst* db_inst, size_t handle)
return true;
}

std::pair<odb::dbInst*, size_t> NesterovBaseCommon::destroyCbkGCell(
std::pair<odb::dbInst*, int> NesterovBaseCommon::destroyCbkGCell(
odb::dbInst* db_inst)
Comment on lines +3602 to 3603
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The rule for NesterovBase::destroyCbkGCell states that its return value, which is an index of type size_t, should be checked against static_cast<size_t>(-1) for failure. Therefore, changing the return type's second element to int to handle a -1 sentinel is not aligned with this established pattern and introduces a potential narrowing conversion (lines 3630 and 3632) where a size_t index is converted to an int. For very large designs where the number of g-cells could exceed INT_MAX, this might lead to data loss or undefined behavior.

If the intention is to use -1 as a sentinel, it should be static_cast<size_t>(-1) when returning a size_t.

A more robust long-term solution would be to refactor the return type to avoid using a sentinel value like static_cast<size_t>(-1). For example, using std::optional<std::pair<odb::dbInst*, size_t>> could provide a safer and clearer interface.

References
  1. The NesterovBase::destroyCbkGCell method returns a size_t index, and its failure value is static_cast<size_t>(-1).

{
std::pair<odb::dbInst*, size_t> replacement = {nullptr, -1};
std::pair<odb::dbInst*, int> replacement = {nullptr, -1};
auto it = db_inst_to_nbc_index_map_.find(db_inst);
if (it == db_inst_to_nbc_index_map_.end()) {
debugPrint(log_,
Expand Down
4 changes: 2 additions & 2 deletions src/gpl/src/nesterovBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ class NesterovBaseCommon
size_t createCbkGCell(odb::dbInst* db_inst);
void createCbkGNet(odb::dbNet* net, bool skip_io_mode);
void createCbkITerm(odb::dbITerm* iTerm);
std::pair<odb::dbInst*, size_t> destroyCbkGCell(odb::dbInst* db_inst);
std::pair<odb::dbInst*, int> destroyCbkGCell(odb::dbInst* db_inst);
void destroyCbkGNet(odb::dbNet*);
void destroyCbkITerm(odb::dbITerm*);
void resizeGCell(odb::dbInst* db_inst);
Expand Down Expand Up @@ -1113,7 +1113,7 @@ class NesterovBase
bool isDiverged() const { return isDiverged_; }

void createCbkGCell(odb::dbInst* db_inst, size_t stor_index);
std::pair<odb::dbInst*, size_t> destroyCbkGCell(odb::dbInst* db_inst);
std::pair<odb::dbInst*, int> destroyCbkGCell(odb::dbInst* db_inst);
bool updateHandle(odb::dbInst* db_inst, size_t handle);

// Must be called after fixPointers() to initialize internal values of gcells,
Expand Down
3 changes: 1 addition & 2 deletions src/gpl/src/nesterovPlace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1286,8 +1286,7 @@ void NesterovPlace::destroyCbkGCell(odb::dbInst* db_inst)

bool destroyed = false;
for (auto& nesterov : nbVec_) {
std::pair<odb::dbInst*, size_t> replaced
= nesterov->destroyCbkGCell(db_inst);
std::pair<odb::dbInst*, int> replaced = nesterov->destroyCbkGCell(db_inst);
if (replaced.first) {
bool updated = false;
for (auto& nesterov : nbVec_) {
Expand Down