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 include/ord/OpenRoad.hh
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@

#pragma once

#include <memory>
#include <set>
#include <iostream>
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The <iostream> header is typically used for std::cin, std::cout, std::cerr, etc. If these are only used in the corresponding .cc implementation file, it's generally better to include <iostream> there instead of in the header file. This helps to minimize compilation dependencies for files that include OpenRoad.hh.

Copy link
Member

Choose a reason for hiding this comment

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

@ahmed532 iostream not needed. mistake?

Choose a reason for hiding this comment

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

This is weird! I do not remember adding it myself. I thought it was in the file from the beginning. I'll remove it.

#include <string>
#include <vector>

Expand Down Expand Up @@ -226,6 +225,7 @@ class OpenRoad
void read3Dbx(const std::string& filename);
void write3Dbv(const std::string& filename);
void write3Dbx(const std::string& filename);
void write3DbloxVerilog(const std::string& filename);
void read3DBloxBMap(const std::string& filename);
void check3DBlox();

Expand Down
7 changes: 6 additions & 1 deletion src/OpenRoad.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@
#include "rsz/MakeResizer.hh"
#include "rsz/Resizer.hh"
#include "sta/VerilogReader.hh"
#include "sta/VerilogWriter.hh"
#include "stt/MakeSteinerTreeBuilder.h"
#include "tap/MakeTapcell.h"
#include "tap/tapcell.h"
Expand Down Expand Up @@ -532,6 +531,12 @@ void OpenRoad::write3Dbx(const std::string& filename)
writer.writeDbx(filename, db_->getChip());
}

void OpenRoad::write3DbloxVerilog(const std::string& filename)
{
odb::ThreeDBlox writer(logger_, db_, sta_);
writer.writeVerilog(filename, db_->getChip());
}

// TODO: bool hierarchy should be removed in the future.
// It is retained for a while for backward compatibility.
void OpenRoad::readDb(const char* filename, bool hierarchy)
Expand Down
7 changes: 7 additions & 0 deletions src/OpenRoad.i
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,13 @@ write_3dbx_cmd(const char *filename)
ord->write3Dbx(filename);
}

void
write_3dblox_verilog_cmd(const char *filename)
{
OpenRoad *ord = getOpenRoad();
ord->write3DbloxVerilog(filename);
}

void
read_db_cmd(const char *filename, bool hierarchy)
{
Expand Down
9 changes: 9 additions & 0 deletions src/OpenRoad.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,15 @@ proc write_3dbx { args } {
ord::write_3dbx_cmd $filename
}

sta::define_cmd_args "write_3dblox_verilog" {filename}

proc write_3dblox_verilog { args } {
Copy link
Member

Choose a reason for hiding this comment

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

@maliberty @precisionmoon Do you think we need to expose this command?

sta::parse_key_args "write_3dblox_verilog" args keys {} flags {}
sta::check_argc_eq1 "write_3dblox_verilog" $args
set filename [file nativename [lindex $args 0]]
ord::write_3dblox_verilog_cmd $filename
}

sta::define_cmd_args "read_3dbx" {filename}

proc read_3dbx { args } {
Expand Down
1 change: 1 addition & 0 deletions src/odb/include/odb/3dblox.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class ThreeDBlox
void writeDbv(const std::string& dbv_file, odb::dbChip* chip);
void writeDbx(const std::string& dbx_file, odb::dbChip* chip);
void writeBMap(const std::string& bmap_file, odb::dbChipRegion* region);
void writeVerilog(const std::string& verilog_file, odb::dbChip* chip);

private:
void createChiplet(const ChipletDef& chiplet);
Expand Down
14 changes: 14 additions & 0 deletions src/odb/src/3dblox/3dblox.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "sta/VerilogReader.hh"
#include "utl/Logger.h"
#include "utl/ScopedTemporaryFile.h"
#include "verilogWriter.h"
namespace odb {

static std::map<std::string, std::string> dup_orient_map
Expand Down Expand Up @@ -301,10 +302,23 @@ void ThreeDBlox::writeDbx(const std::string& dbx_file, odb::dbChip* chip)

writeDbv(current_dir_path + chip->getName() + ".3dbv", chip);

// Write the Verilog connectivity file for this HIER chiplet.
writeVerilog(current_dir_path + chip->getName() + ".v", chip);
Comment on lines +305 to +306
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The writeDbx method now has a side effect of writing a Verilog connectivity file. While this might be intended for hierarchical chiplets, the method name writeDbx (presumably for 'write Dbx') does not clearly indicate this additional output. Consider renaming the method to better reflect its full functionality or making the Verilog writing an explicit, optional step if it's not always required when writing a Dbx file.


DbxWriter writer(logger_, db_);
writer.writeChiplet(dbx_file, chip);
}

void ThreeDBlox::writeVerilog(const std::string& verilog_file,
odb::dbChip* chip)
{
if (chip == nullptr) {
return;
}
VerilogWriter writer(logger_);
writer.writeChiplet(verilog_file, chip);
}

void ThreeDBlox::writeBMap(const std::string& bmap_file,
odb::dbChipRegion* region)
{
Expand Down
1 change: 1 addition & 0 deletions src/odb/src/3dblox/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ add_library(3dblox
bmapWriter.cpp
dbvWriter.cpp
dbxWriter.cpp
verilogWriter.cpp
3dblox.cpp
checker.cpp
)
Expand Down
2 changes: 2 additions & 0 deletions src/odb/src/3dblox/dbxWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ void DbxWriter::writeYamlContent(YAML::Node& root, odb::dbChip* chiplet)
void DbxWriter::writeDesign(YAML::Node& design_node, odb::dbChip* chiplet)
{
design_node["name"] = chiplet->getName();
YAML::Node external_node = design_node["external"];
external_node["verilog_file"] = std::string(chiplet->getName()) + ".v";
}

void DbxWriter::writeChipletInsts(YAML::Node& instances_node,
Expand Down
121 changes: 121 additions & 0 deletions src/odb/src/3dblox/verilogWriter.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// SPDX-License-Identifier: BSD-3-Clause
// Copyright (c) 2019-2025, The OpenROAD Authors

#include "verilogWriter.h"

#include <algorithm>
#include <cstddef>
#include <cstdint>
#include <fstream>
#include <map>
#include <string>
#include <utility>
#include <vector>

#include "odb/db.h"
#include "utl/Logger.h"

namespace odb {

VerilogWriter::VerilogWriter(utl::Logger* logger) : logger_(logger)
{
}

void VerilogWriter::writeChiplet(const std::string& filename, odb::dbChip* chip)
{
std::ofstream verilog_file(filename);
if (!verilog_file.is_open()) {
logger_->error(
utl::ODB, 563, "Unable to open Verilog file for writing: {}", filename);
return;
}

// chip_inst -> list of (port_name, net_name)
std::map<dbChipInst*, std::vector<std::pair<std::string, std::string>>>
inst_connections;

for (dbChipNet* net : chip->getChipNets()) {
const std::string net_name = net->getName();
const uint32_t num_bumps = net->getNumBumpInsts();
for (uint32_t i = 0; i < num_bumps; i++) {
std::vector<dbChipInst*> path;
dbChipBumpInst* bump_inst = net->getBumpInst(i, path);
if (bump_inst == nullptr || path.size() != 1) {
continue;
}
// Only handle direct children (path length 1) — "single bump
// connections".
dbChipInst* chip_inst = path[0];
dbChipBump* bump = bump_inst->getChipBump();
if (bump == nullptr) {
continue;
}
dbBTerm* bterm = bump->getBTerm();
if (bterm == nullptr) {
continue;
}
inst_connections[chip_inst].emplace_back(bterm->getName(), net_name);
}
}

// Sort each instance's port connections alphabetically by port name.
for (std::pair<dbChipInst* const,
std::vector<std::pair<std::string, std::string>>>& entry :
inst_connections) {
std::ranges::sort(entry.second);
}

// Write module header.
fmt::print(verilog_file, "module {} ();\n", chip->getName());

// Collect and sort net names alphabetically for deterministic wire order.
std::vector<std::string> net_names;
for (dbChipNet* net : chip->getChipNets()) {
net_names.push_back(net->getName());
}
std::ranges::sort(net_names);

// Write wire declarations in sorted order.
for (const std::string& name : net_names) {
fmt::print(verilog_file, " wire {};\n", name);
}

// Collect and sort instances alphabetically by instance name.
std::vector<dbChipInst*> chip_insts;
for (dbChipInst* chip_inst : chip->getChipInsts()) {
chip_insts.push_back(chip_inst);
}
std::ranges::sort(chip_insts, [](dbChipInst* a, dbChipInst* b) {
return a->getName() < b->getName();
});

// Write instance declarations in sorted order.
for (dbChipInst* chip_inst : chip_insts) {
fmt::print(verilog_file,
" {} {} (\n",
chip_inst->getMasterChip()->getName(),
chip_inst->getName());
const std::map<
dbChipInst*,
std::vector<std::pair<std::string, std::string>>>::const_iterator it
= inst_connections.find(chip_inst);
Comment on lines +98 to +101
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The type declaration for it is quite verbose. Using auto would improve readability and conciseness without losing type safety.

    auto it = inst_connections.find(chip_inst);

if (it != inst_connections.end()) {
const std::vector<std::pair<std::string, std::string>>& conns
= it->second;
for (size_t j = 0; j < conns.size(); j++) {
const bool is_last = (j + 1 == conns.size());
fmt::print(verilog_file,
" .{}({}){}\n",
conns[j].first,
conns[j].second,
is_last ? "" : ",");
Comment on lines +105 to +111
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The loop structure for printing with commas is common but can be slightly refactored for better readability and conciseness. While functional, a pattern that handles the first element separately or prefixes subsequent elements with a comma can sometimes be cleaner.

}
}
fmt::print(verilog_file, " );\n");
}

fmt::print(verilog_file, "endmodule\n");
verilog_file.close();
}

} // namespace odb
24 changes: 24 additions & 0 deletions src/odb/src/3dblox/verilogWriter.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// SPDX-License-Identifier: BSD-3-Clause
// Copyright (c) 2019-2025, The OpenROAD Authors

#pragma once

#include <string>

namespace utl {
class Logger;
}
namespace odb {
class dbChip;

class VerilogWriter
{
public:
VerilogWriter(utl::Logger* logger);
void writeChiplet(const std::string& filename, odb::dbChip* chip);

private:
utl::Logger* logger_ = nullptr;
};

} // namespace odb
6 changes: 6 additions & 0 deletions src/odb/test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ COMPULSORY_TESTS = [
"write_cdl",
"write_3dbv",
"write_3dbx",
"write_3dblox_verilog",
"write_3dblox_verilog_nets",
"write_def58",
"write_def58_gzip",
"write_lef_and_def",
Expand Down Expand Up @@ -293,6 +295,9 @@ filegroup(
"data/example2.bmap",
"data/example3.bmap",
"data/example4.bmap",
"data/example_nets.3dbv",
"data/example_nets.3dbx",
"data/example_nets.bmap",
"data/floorplan_initialize.def",
"data/floorplan_initialize.v",
"data/floorplan_initialize2.def",
Expand All @@ -313,6 +318,7 @@ filegroup(
"data/rounding.def",
"data/sky130_fd_sc_hd__inv_1.gds",
"data/sky130hd_multi_patterned.def",
"data/top_nets.v",
"dump_netlists.tcl",
"dump_netlists_withfill.tcl",
"fake_macros.def",
Expand Down
2 changes: 2 additions & 0 deletions src/odb/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ or_integration_tests(
write_cdl
write_3dbv
write_3dbx
write_3dblox_verilog
write_3dblox_verilog_nets
write_def58
write_def58_gzip
write_lef_and_def
Expand Down
1 change: 1 addition & 0 deletions src/odb/test/cpp/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ cc_test(
name = "Test3DBloxParser",
srcs = [
"Test3DBloxParser.cpp",
"Test3DBloxVerilogWriter.cpp",
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Adding Test3DBloxVerilogWriter.cpp to the srcs of Test3DBloxParser might lead to less modular test organization. It would be clearer to create a separate cc_test target for Test3DBloxVerilogWriter.cpp if its tests are distinct from Test3DBloxParser. This improves test discoverability and allows for more granular test execution.

],
data = [
"//src/odb/test:regression_resources",
Expand Down
2 changes: 1 addition & 1 deletion src/odb/test/cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ add_executable(TestNetTrack TestNetTrack.cpp)
add_executable(TestMaster TestMaster.cpp)
add_executable(TestGDSIn TestGDSIn.cpp)
add_executable(TestChips TestChips.cpp)
add_executable(Test3DBloxParser Test3DBloxParser.cpp)
add_executable(Test3DBloxParser Test3DBloxParser.cpp Test3DBloxVerilogWriter.cpp)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Similar to the BUILD file, including Test3DBloxVerilogWriter.cpp in the Test3DBloxParser executable in CMakeLists.txt could be made more explicit. Consider creating a dedicated executable target for Test3DBloxVerilogWriter.cpp to improve test modularity and clarity.

Copy link
Member

Choose a reason for hiding this comment

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

Please address. It needs a separate executable IMO.

Choose a reason for hiding this comment

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

Sure.

add_executable(Test3DBloxChecker Test3DBloxChecker.cpp Test3DBloxCheckerLogicalConn.cpp Test3DBloxCheckerBumps.cpp)
add_executable(TestSwapMaster TestSwapMaster.cpp)
add_executable(TestSwapMasterUnusedPort TestSwapMasterUnusedPort.cpp)
Expand Down
Loading
Loading