Skip to content

Conversation

@rafaelmoresco
Copy link
Contributor

Base 3dbvWriter.

When called directly by CLI, does the writeFile function, writing all chiplets defs in the same level. The function writeChiplet is made to be called by the 3dbxWriter, writing all the dependencies of a top 3dbx chiplet, with a call back to the 3dbxWriter if a hier chiplet is found.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@rafaelmoresco rafaelmoresco marked this pull request as ready for review November 3, 2025 20:51
Copy link
Member

@osamahammad21 osamahammad21 left a comment

Choose a reason for hiding this comment

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

I am thinking of merging this PR and addressing the requested changes in a follow up PR. @maliberty Is it ok to do so, given that it doesn’t modify any database objects and shouldn’t cause any performance impact

Comment on lines +29 to +34
void writeLef(YAML::Node& external_node,
odb::dbDatabase* db,
odb::dbChip* chiplet);
void writeDef(YAML::Node& external_node,
odb::dbDatabase* db,
odb::dbChip* chiplet);
Copy link
Member

Choose a reason for hiding this comment

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

Should be moved to dbvWriter as dbxWriter should never handle lef and def.

Comment on lines +47 to +95
void BaseWriter::writeLef(YAML::Node& external_node,
odb::dbDatabase* db,
odb::dbChip* chiplet)
{
auto libs = db->getLibs();
int num_libs = libs.size();
if (num_libs > 0) {
if (num_libs > 1) {
logger_->info(
utl::ODB,
539,
"More than one lib exists, multiple files will be written.");
}
int cnt = 0;
for (auto lib : libs) {
std::string name(std::string(lib->getName()) + ".lef");
if (cnt > 0) {
auto pos = name.rfind('.');
if (pos != std::string::npos) {
name.insert(pos, "_" + std::to_string(cnt));
} else {
name += "_" + std::to_string(cnt);
}
utl::OutStreamHandler stream_handler(name.c_str());
odb::lefout lef_writer(logger_, stream_handler.getStream());
lef_writer.writeLib(lib);
} else {
utl::OutStreamHandler stream_handler(name.c_str());
odb::lefout lef_writer(logger_, stream_handler.getStream());
lef_writer.writeTechAndLib(lib);
}
YAML::Node list_node;
list_node.SetStyle(YAML::EmitterStyle::Flow);
list_node.push_back(name.c_str());
if ((name.find("_tech") != std::string::npos) || (libs.size() == 1)) {
external_node["APR_tech_file"] = list_node;
} else {
external_node["LEF_file"] = list_node;
}
++cnt;
}
} else if (db->getTech()) {
utl::OutStreamHandler stream_handler(
(std::string(chiplet->getName()) + ".lef").c_str());
odb::lefout lef_writer(logger_, stream_handler.getStream());
lef_writer.writeTech(db->getTech());
external_node["APR_tech_file"] = (std::string(chiplet->getName()) + ".lef");
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Couple of comments on this:

  • It's better and cleaner to separate the tech lef from the lib when we write, even though we don't enforce this when we read. So we shouldn't use writeTechAndLib.
  • We should infer the tech used by the current chip instead of using db->getTech(). we should use chip->getTech(). This would not work correctly on a design that has multiple technologies
  • We should resolve which libs are being used in the current chip instead of linking it to all libs in the current database. This could be done by going through the insts of the chip's block and collecting the set of libs used.

Signed-off-by: Rafael Moresco <[email protected]>
Signed-off-by: Rafael Moresco <[email protected]>
Signed-off-by: Rafael Moresco <[email protected]>
Signed-off-by: Rafael Moresco <[email protected]>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

#include "baseWriter.h"

#include <yaml-cpp/emitter.h>
#include <yaml-cpp/emitterstyle.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: included header emitter.h is not used directly [misc-include-cleaner]

Suggested change
#include <yaml-cpp/emitterstyle.h>
#include <yaml-cpp/emitterstyle.h>

#include <yaml-cpp/emitter.h>
#include <yaml-cpp/emitterstyle.h>
#include <yaml-cpp/node/convert.h>
#include <yaml-cpp/node/detail/impl.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: included header convert.h is not used directly [misc-include-cleaner]

Suggested change
#include <yaml-cpp/node/detail/impl.h>
#include <yaml-cpp/node/detail/impl.h>

#include <yaml-cpp/emitterstyle.h>
#include <yaml-cpp/node/convert.h>
#include <yaml-cpp/node/detail/impl.h>
#include <yaml-cpp/node/emit.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: included header impl.h is not used directly [misc-include-cleaner]

Suggested change
#include <yaml-cpp/node/emit.h>
#include <yaml-cpp/node/emit.h>

#include "dbvWriter.h"

#include <yaml-cpp/emitter.h>
#include <yaml-cpp/emitterstyle.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: included header emitter.h is not used directly [misc-include-cleaner]

Suggested change
#include <yaml-cpp/emitterstyle.h>
#include <yaml-cpp/emitterstyle.h>

#include <yaml-cpp/emitter.h>
#include <yaml-cpp/emitterstyle.h>
#include <yaml-cpp/node/convert.h>
#include <yaml-cpp/node/detail/impl.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: included header convert.h is not used directly [misc-include-cleaner]

Suggested change
#include <yaml-cpp/node/detail/impl.h>
#include <yaml-cpp/node/detail/impl.h>

#include <yaml-cpp/emitterstyle.h>
#include <yaml-cpp/node/convert.h>
#include <yaml-cpp/node/detail/impl.h>
#include <yaml-cpp/node/emit.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: included header impl.h is not used directly [misc-include-cleaner]

Suggested change
#include <yaml-cpp/node/emit.h>
#include <yaml-cpp/node/emit.h>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants