Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/gui/src/chiplet3DWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ void Chiplet3DWidget::buildGeometries()
return;
}
const odb::Cuboid global_cuboid = chip_->getCuboid();
const odb::UnfoldedModel model(logger_, chip_);
const odb::UnfoldedModel* model = chip_->getDb()->getUnfoldedModel();
const odb::dbTransform center_transform
= odb::dbTransform(odb::Point3D(-global_cuboid.xCenter(),
-global_cuboid.yCenter(),
Expand All @@ -98,7 +98,7 @@ void Chiplet3DWidget::buildGeometries()
}

int index = 0;
for (const auto& chip : model.getChips()) {
for (const auto& chip : model->getChips()) {
odb::Cuboid draw_cuboid = chip.cuboid;
center_transform.apply(draw_cuboid);
// Color by Depth (proportional to Z)
Expand Down
5 changes: 5 additions & 0 deletions src/odb/include/odb/db.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ class dbExtControl;
// Custom iterators
class dbModuleBusPortModBTermItr;

class UnfoldedModel;

///////////////////////////////////////////////////////////////////////////////
///
/// A box is the element used to represent layout shapes.
Expand Down Expand Up @@ -7550,6 +7552,9 @@ class dbDatabase : public dbObject
///
dbChip* getChip();

void constructUnfoldedModel();

const UnfoldedModel* getUnfoldedModel() const;
////////////////////////
/// DEPRECATED
////////////////////////
Expand Down
4 changes: 2 additions & 2 deletions src/odb/src/3dblox/3dblox.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ void ThreeDBlox::readDbx(const std::string& dbx_file)

void ThreeDBlox::check()
{
Checker checker(logger_);
checker.check(db_->getChip());
Checker checker(logger_, db_);
checker.check();
}

namespace {
Expand Down
1 change: 0 additions & 1 deletion src/odb/src/3dblox/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ add_library(3dblox
dbvWriter.cpp
dbxWriter.cpp
3dblox.cpp
unfoldedModel.cpp
checker.cpp
)

Expand Down
40 changes: 22 additions & 18 deletions src/odb/src/3dblox/checker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,18 @@ bool isValid(const UnfoldedConnection& conn)

} // namespace

Checker::Checker(utl::Logger* logger) : logger_(logger)
Checker::Checker(utl::Logger* logger, dbDatabase* db) : logger_(logger), db_(db)
{
}

void Checker::check(dbChip* chip)
void Checker::check()
{
dbChip* chip = db_->getChip();
const UnfoldedModel* model = db_->getUnfoldedModel();
if (model == nullptr || chip == nullptr) {
return;
}
auto* top_cat = dbMarkerCategory::createOrReplace(chip, "3DBlox");
UnfoldedModel model(logger_, chip);
checkLogicalConnectivity(top_cat, model);
checkFloatingChips(top_cat, model);
checkOverlappingChips(top_cat, model);
Expand All @@ -108,9 +112,9 @@ void Checker::check(dbChip* chip)
}

void Checker::checkFloatingChips(dbMarkerCategory* top_cat,
const UnfoldedModel& model)
const UnfoldedModel* model)
{
const auto& chips = model.getChips();
const auto& chips = model->getChips();
// Add one more node for "ground" (external world: package, PCB, ...)
utl::UnionFind uf(chips.size() + 1);
const size_t ground_node = chips.size();
Expand All @@ -120,7 +124,7 @@ void Checker::checkFloatingChips(dbMarkerCategory* top_cat,
chip_map[&chips[i]] = i;
}

for (const auto& conn : model.getConnections()) {
for (const auto& conn : model->getConnections()) {
if (isValid(conn)) {
// Case 1: Both regions exist - connect the two chips together
if (conn.top_region && conn.bottom_region) {
Expand Down Expand Up @@ -181,9 +185,9 @@ void Checker::checkFloatingChips(dbMarkerCategory* top_cat,
}

void Checker::checkOverlappingChips(dbMarkerCategory* top_cat,
const UnfoldedModel& model)
const UnfoldedModel* model)
{
const auto& chips = model.getChips();
const auto& chips = model->getChips();
std::vector<int> sorted(chips.size());
std::iota(sorted.begin(), sorted.end(), 0);
std::ranges::sort(sorted, [&](int a, int b) {
Expand Down Expand Up @@ -220,10 +224,10 @@ void Checker::checkOverlappingChips(dbMarkerCategory* top_cat,
}

void Checker::checkInternalExtUsage(dbMarkerCategory* top_cat,
const UnfoldedModel& model)
const UnfoldedModel* model)
{
dbMarkerCategory* cat = nullptr;
for (const auto& chip : model.getChips()) {
for (const auto& chip : model->getChips()) {
for (const auto& region : chip.regions) {
if (region.isInternalExt() && !region.isUsed) {
if (!cat) {
Expand All @@ -246,7 +250,7 @@ void Checker::checkInternalExtUsage(dbMarkerCategory* top_cat,
}

void Checker::checkConnectionRegions(dbMarkerCategory* top_cat,
const UnfoldedModel& model)
const UnfoldedModel* model)
{
auto describe = [](const UnfoldedRegion* r, dbMarker* marker) {
marker->addSource(r->region_inst);
Expand All @@ -261,7 +265,7 @@ void Checker::checkConnectionRegions(dbMarkerCategory* top_cat,
};
int count = 0;
dbMarkerCategory* cat = nullptr;
for (const auto& conn : model.getConnections()) {
for (const auto& conn : model->getConnections()) {
if (!isValid(conn)) {
if (!cat) {
cat = dbMarkerCategory::createOrReplace(top_cat, "Connection regions");
Expand All @@ -283,11 +287,11 @@ void Checker::checkConnectionRegions(dbMarkerCategory* top_cat,
}

void Checker::checkBumpPhysicalAlignment(dbMarkerCategory* top_cat,
const UnfoldedModel& model)
const UnfoldedModel* model)
{
dbMarkerCategory* cat = nullptr;
int violation_count = 0;
for (const auto& chip : model.getChips()) {
for (const auto& chip : model->getChips()) {
for (const auto& region : chip.regions) {
for (const auto& bump : region.bumps) {
const auto& p = bump.global_position;
Expand Down Expand Up @@ -318,15 +322,15 @@ void Checker::checkBumpPhysicalAlignment(dbMarkerCategory* top_cat,
}

void Checker::checkNetConnectivity(dbMarkerCategory* top_cat,
const UnfoldedModel& model)
const UnfoldedModel* model)
{
}

void Checker::checkLogicalConnectivity(dbMarkerCategory* top_cat,
const UnfoldedModel& model)
const UnfoldedModel* model)
{
std::unordered_map<const UnfoldedBump*, const UnfoldedNet*> bump_net_map;
for (const auto& net : model.getNets()) {
for (const auto& net : model->getNets()) {
for (const auto* bump : net.connected_bumps) {
bump_net_map[bump] = &net;
}
Expand All @@ -341,7 +345,7 @@ void Checker::checkLogicalConnectivity(dbMarkerCategory* top_cat,
};

dbMarkerCategory* cat = nullptr;
for (const auto& conn : model.getConnections()) {
for (const auto& conn : model->getConnections()) {
if (!isValid(conn)) {
continue;
}
Expand Down
28 changes: 15 additions & 13 deletions src/odb/src/3dblox/checker.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,17 @@

#pragma once

#include "odb/db.h"
#include "odb/unfoldedModel.h"
#include "utl/Logger.h"
namespace utl {
class Logger;
}

namespace sta {
class Sta;
}

namespace odb {
class dbChip;
class dbDatabase;
class UnfoldedModel;
class dbMarkerCategory;

struct MatingSurfaces
Expand All @@ -25,26 +26,27 @@ struct MatingSurfaces
class Checker
{
public:
Checker(utl::Logger* logger);
Checker(utl::Logger* logger, dbDatabase* db);
~Checker() = default;
void check(dbChip* chip);
void check();

private:
void checkLogicalConnectivity(dbMarkerCategory* top_cat,
const UnfoldedModel& model);
const UnfoldedModel* model);
void checkFloatingChips(dbMarkerCategory* top_cat,
const UnfoldedModel& model);
const UnfoldedModel* model);
void checkOverlappingChips(dbMarkerCategory* top_cat,
const UnfoldedModel& model);
const UnfoldedModel* model);
void checkInternalExtUsage(dbMarkerCategory* top_cat,
const UnfoldedModel& model);
const UnfoldedModel* model);
void checkConnectionRegions(dbMarkerCategory* top_cat,
const UnfoldedModel& model);
const UnfoldedModel* model);
void checkBumpPhysicalAlignment(dbMarkerCategory* top_cat,
const UnfoldedModel& model);
const UnfoldedModel* model);
void checkNetConnectivity(dbMarkerCategory* top_cat,
const UnfoldedModel& model);
const UnfoldedModel* model);
utl::Logger* logger_;
dbDatabase* db_;
};

} // namespace odb
1 change: 1 addition & 0 deletions src/odb/src/db/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ add_library(db
dbGroupGroundNetItr.cpp
dbGDSLib.cpp
dbSwapMasterSanityChecker.cpp
unfoldedModel.cpp
# Generator Code Begin cpp
dbAccessPoint.cpp
dbBusPort.cpp
Expand Down
20 changes: 20 additions & 0 deletions src/odb/src/db/dbDatabase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
#include "odb/dbDatabaseObserver.h"
#include "odb/dbObject.h"
#include "odb/dbStream.h"
#include "odb/unfoldedModel.h"
#include "utl/Logger.h"
// User Code End Includes
namespace odb {
Expand Down Expand Up @@ -207,6 +208,8 @@ _dbDatabase::_dbDatabase(_dbDatabase* db)
chip_bump_inst_itr_ = new dbChipBumpInstItr(chip_bump_inst_tbl_);

chip_net_itr_ = new dbChipNetItr(chip_net_tbl_);

unfolded_model_ = nullptr;
// User Code End Constructor
}

Expand Down Expand Up @@ -446,6 +449,7 @@ _dbDatabase::~_dbDatabase()
delete chip_conn_itr_;
delete chip_bump_inst_itr_;
delete chip_net_itr_;
delete unfolded_model_;
// User Code End Destructor
}

Expand Down Expand Up @@ -495,6 +499,8 @@ _dbDatabase::_dbDatabase(_dbDatabase* /* unused: db */, int id)
chip_bump_inst_itr_ = new dbChipBumpInstItr(chip_bump_inst_tbl_);

chip_net_itr_ = new dbChipNetItr(chip_net_tbl_);

unfolded_model_ = nullptr;
}

utl::Logger* _dbDatabase::getLogger() const
Expand Down Expand Up @@ -685,6 +691,19 @@ dbChip* dbDatabase::getChip()
return (dbChip*) db->chip_tbl_->getPtr(db->chip_);
}

void dbDatabase::constructUnfoldedModel()
Copy link
Member

Choose a reason for hiding this comment

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

Should we have auto creation in get rather than explicit construction? Do we want get to return a nullptr?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could, which I did earlier, but I think this introduces many issues for the current use cases.

  • If we create in the getter, do we create once or each time? If we create once, then any changes to the database after retrieving the unfoldedModel will never be reflected.
  • The cpp test cases change the database objects directly and test that change on the checker, which needs a way of updating the unfoldedModel every time since it's static.

I would prefer to keep it that way temporarily until we make it dynamic.

Copy link
Member

Choose a reason for hiding this comment

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

As it stand calling it twice leaks the previous unfolding. At a minimum it needs to manage memory if you want to stick with this temporarily

I think get should create if needed or return otherwise. Updating the unfolding dynamically should be a separate, automatic process inside the database.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the memory leak stands and will be fixed by destroying the unfoldedModel before reconstructing it.
However, the dynamic update is planned but not included in this PR because it requires some work to listen for database objects callbacks. So is it ok to merge this PR and handle the automatic updates in future PRs or is it preferred to handle that in this PR?

{
_dbDatabase* db = (_dbDatabase*) this;
delete db->unfolded_model_;
db->unfolded_model_ = new UnfoldedModel(db->logger_, getChip());
}

const UnfoldedModel* dbDatabase::getUnfoldedModel() const
{
_dbDatabase* db = (_dbDatabase*) this;
return db->unfolded_model_;
}

dbTech* dbDatabase::getTech()
{
auto techs = getTechs();
Expand Down Expand Up @@ -993,6 +1012,7 @@ void dbDatabase::triggerPostRead3Dbx(dbChip* chip)
for (dbDatabaseObserver* observer : db->observers_) {
observer->postRead3Dbx(chip);
}
constructUnfoldedModel();
}

void dbDatabase::triggerPostReadDb()
Expand Down
3 changes: 3 additions & 0 deletions src/odb/src/db/dbDatabase.h
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ class _dbNameCache;
class _dbTech;
class _dbLib;
class _dbGDSLib;
class UnfoldedModel;
// User Code End Classes

class _dbDatabase : public _dbObject
Expand Down Expand Up @@ -335,6 +336,8 @@ class _dbDatabase : public _dbObject

utl::Logger* logger_;
std::set<dbDatabaseObserver*> observers_;
UnfoldedModel* unfolded_model_; // non-persistent object

// User Code End Fields
};
dbIStream& operator>>(dbIStream& stream, _dbDatabase& obj);
Expand Down
File renamed without changes.
4 changes: 4 additions & 0 deletions src/odb/test/check_3dblox.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ set p [odb::Point3D]
$p set $x1 $y1 [expr $z1 + $t1]
$inst2 setLoc $p

$db constructUnfoldedModel
check_3dblox
check "Touching chips not floating" { get_3dblox_marker_count "Floating chips" } 0

Expand All @@ -33,6 +34,7 @@ check "Touching chips not floating" { get_3dblox_marker_count "Floating chips" }
$p set $x1 $y1 [expr $z1 + $t1 + 10000]
$inst2 setLoc $p

$db constructUnfoldedModel
check_3dblox
check "Vertical gap detected as floating" { get_3dblox_marker_count "Floating chips" } 1

Expand All @@ -42,6 +44,7 @@ set inst3 [odb::dbChipInst_create $top_chip $master1 "inst3"]
$p set [expr $x1 + 10 * $w1] [expr $y1 + 10 * $h1] $z1
$inst3 setLoc $p

$db constructUnfoldedModel
check_3dblox
# Should find 2 sets of floating chips (inst1, inst2 and inst3 are all separate from each other)
check "Multiple floating sets detected" { get_3dblox_marker_count "Floating chips" } 2
Expand All @@ -52,6 +55,7 @@ set p [odb::Point3D]
$p set [expr $x1 + $w1 / 4] [expr $y1 + $h1 / 4] [expr $z1 + $t1 / 2]
$inst2 setLoc $p

$db constructUnfoldedModel
check_3dblox
check "Partial overlap detected" { get_3dblox_marker_count "Overlapping chips" } 1

Expand Down
1 change: 1 addition & 0 deletions src/odb/test/cpp/Test3DBloxCheckerFixture.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class CheckerFixture : public tst::Fixture
void check()
{
db_->setTopChip(top_chip_);
db_->constructUnfoldedModel();
ThreeDBlox three_dblox(&logger_, db_.get());
three_dblox.check();
}
Expand Down
1 change: 1 addition & 0 deletions src/odb/test/cpp/Test3DBloxCheckerLogicalConn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class CheckerLogicalConnFixture : public CheckerFixture
void check()
{
utl::Logger logger;
db_->constructUnfoldedModel();
ThreeDBlox three_dblox(&logger, db_.get());
three_dblox.check();
}
Expand Down
Loading