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
3 changes: 2 additions & 1 deletion src/OpenRoad.cc
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,8 @@ void OpenRoad::init(Tcl_Interp* tcl_interp,
ioPlacer_,
opendp_,
global_router_,
detailed_router_);
detailed_router_,
sta_);
icewall_ = new pad::ICeWall(db_, logger_);
dft_ = new dft::Dft(db_, sta_, logger_);
example_ = new exa::Example(db_, logger_);
Expand Down
1 change: 1 addition & 0 deletions src/ram/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ cc_library(
],
deps = [
"//:ord",
"//src/dbSta",
"//src/dbSta:dbNetwork",
"//src/dpl",
"//src/drt",
Expand Down
8 changes: 7 additions & 1 deletion src/ram/include/ram/ram.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class dbMaster;
namespace sta {
class dbNetwork;
class LibertyPort;
class dbSta;
} // namespace sta

namespace utl {
Expand Down Expand Up @@ -92,7 +93,8 @@ class RamGen
ppl::IOPlacer* io_placer,
dpl::Opendp* opendp,
grt::GlobalRouter* global_router,
drt::TritonRoute* detailed_router);
drt::TritonRoute* detailed_router,
sta::dbSta* sta);
~RamGen() = default;

void generate(int mask_size,
Expand Down Expand Up @@ -132,6 +134,8 @@ class RamGen
int r_ports,
int w_ports);

void reportTimingAndPower();

private:
void findMasters();
std::map<PortRole, std::string> buildPortMap(odb::dbMaster*);
Expand Down Expand Up @@ -221,6 +225,7 @@ class RamGen
dpl::Opendp* opendp_{nullptr};
grt::GlobalRouter* global_router_{nullptr};
drt::TritonRoute* detailed_router_{nullptr};
sta::dbSta* sta_{nullptr};

odb::dbMaster* storage_cell_{nullptr};
odb::dbMaster* tristate_cell_{nullptr};
Expand All @@ -245,6 +250,7 @@ class RamGen

std::vector<std::vector<odb::dbBTerm*>> addr_inputs_;
std::vector<odb::dbBTerm*> data_inputs_;
std::vector<odb::dbBTerm*> write_enable_;
std::vector<std::vector<odb::dbBTerm*>> q_outputs_;
std::string behavioral_verilog_filename_;
std::string aoi22_in_a1_, aoi22_in_a2_, aoi22_in_b1_, aoi22_in_b2_,
Expand Down
183 changes: 182 additions & 1 deletion src/ram/src/ram.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <vector>

#include "db_sta/dbNetwork.hh"
#include "db_sta/dbSta.hh"
#include "dpl/Opendp.h"
#include "drt/TritonRoute.h"
#include "grt/GlobalRouter.h"
Expand All @@ -30,8 +31,18 @@
#include "sta/ConcreteLibrary.hh"
#include "sta/FuncExpr.hh"
#include "sta/Liberty.hh"
#include "sta/MinMax.hh"
#include "sta/NetworkClass.hh"
#include "sta/PathEnd.hh"
#include "sta/PortDirection.hh"
#include "sta/PowerClass.hh"
#include "sta/Scene.hh"
#include "sta/SdcClass.hh"
#include "sta/SearchClass.hh"
#include "sta/Sequential.hh"
#include "sta/StringUtil.hh"
#include "sta/Transition.hh"
#include "sta/Units.hh"
#include "utl/Logger.h"

namespace ram {
Expand All @@ -57,7 +68,8 @@ RamGen::RamGen(sta::dbNetwork* network,
ppl::IOPlacer* io_placer,
dpl::Opendp* opendp,
grt::GlobalRouter* global_router,
drt::TritonRoute* detailed_router)
drt::TritonRoute* detailed_router,
sta::dbSta* sta)

: network_(network),
db_(db),
Expand All @@ -67,6 +79,7 @@ RamGen::RamGen(sta::dbNetwork* network,
opendp_(opendp),
global_router_(global_router),
detailed_router_(detailed_router),
sta_(sta),
ram_grid_(odb::horizontal)
{
}
Expand Down Expand Up @@ -1275,6 +1288,7 @@ void RamGen::generate(const int mask_size,
auto in_name = fmt::format("we[{}]", slice);
write_enable[slice] = makeBTerm(in_name, dbIoType::INPUT);
}
write_enable_ = write_enable;

// When column_mux_ratio > 1: word_sel_nets[word_idx] is high/active when
// word_idx is the addressed word within a physical row. Derived from the
Expand Down Expand Up @@ -2056,4 +2070,171 @@ endmodule
logger_->info(RAM, 24, "Behavioral Verilog written for {}", module_name);
}

static bool isClockPin(const sta::Pin* pin, sta::dbNetwork* network)
{
sta::LibertyPort* lp = network->libertyPort(pin);
if (!lp) {
return false;
}
return lp->isClock() || lp->isRegClk() || lp->isClockGateClock();
}

void RamGen::reportTimingAndPower()
{
network_->setBlock(block_);
sta_->updateTiming(false);

sta::SceneSeq scenes = sta_->makeSceneSeq(sta_->cmdScene());
sta::StringSeq group_names;

// Include non clock primary inputs: D, address, write enable pins
auto collect_primary_inputs = [&]() -> sta::PinSet* {
auto* pins = new sta::PinSet(network_);
for (auto* bterm : data_inputs_) {
pins->insert(network_->dbToSta(bterm));
}
for (auto& port_addrs : addr_inputs_) {
for (auto* bterm : port_addrs) {
pins->insert(network_->dbToSta(bterm));
}
}
for (auto* bterm : write_enable_) {
pins->insert(network_->dbToSta(bterm));
}
return pins;
};

auto collect_all_outputs = [&]() -> sta::PinSet* {
auto* pins = new sta::PinSet(network_);
for (auto* inst : block_->getInsts()) {
for (auto* iterm : inst->getITerms()) {
auto* pin = network_->dbToSta(iterm);
if (!pin || isClockPin(pin, network_)) {
continue;
}
auto* lp = network_->libertyPort(pin);
if (lp && lp->direction()->isAnyOutput()) {
pins->insert(pin);
}
}
}
for (auto* bterm : block_->getBTerms()) {
auto* pin = network_->dbToSta(bterm);
if (pin && !isClockPin(pin, network_)
&& network_->direction(pin)->isOutput()) {
pins->insert(pin);
}
}
return pins;
};

// Find worst unconstrained setup path delay/longest path
sta::ExceptionFrom* setup_from
= sta_->makeExceptionFrom(collect_primary_inputs(),
nullptr,
nullptr,
sta::RiseFallBoth::riseFall(),
sta_->cmdSdc());

sta::ExceptionTo* setup_to
= sta_->makeExceptionTo(collect_all_outputs(),
nullptr,
nullptr,
sta::RiseFallBoth::riseFall(),
sta::RiseFallBoth::riseFall(),
sta_->cmdSdc());

sta::PathEndSeq setup_ends
= sta_->findPathEnds(setup_from, /* from */
nullptr, /* thrus */
setup_to, /* to */
true, /* unconstrained */
scenes, /* scenes */
sta::MinMaxAll::max(), /* min_max */
1, /* group_path_count */
1, /* endpoint_path_count */
false, /* unique_pins */
false, /* unique_edges */
-sta::INF, /* slack_min */
sta::INF, /* slack_max */
true, /* sort_by_slack */
group_names, /* group_names */
true, /* setup */
false, /* hold */
false, /* recovery */
false, /* removal */
true, /* clk_gating_setup */
false); /* clk_gating_hold */

float setup_delay
= setup_ends.empty()
? 0.0
: static_cast<float>(setup_ends[0]->dataArrivalTime(sta_));

// Find worst unconstrained hold path delay/shortest path
sta::ExceptionFrom* hold_from
= sta_->makeExceptionFrom(collect_primary_inputs(),
nullptr,
nullptr,
sta::RiseFallBoth::riseFall(),
sta_->cmdSdc());

sta::ExceptionTo* hold_to
= sta_->makeExceptionTo(collect_all_outputs(),
nullptr,
nullptr,
sta::RiseFallBoth::riseFall(),
sta::RiseFallBoth::riseFall(),
sta_->cmdSdc());

sta::PathEndSeq hold_ends
= sta_->findPathEnds(hold_from, /* from */
nullptr, /* thrus */
hold_to, /* to */
true, /* unconstrained */
scenes, /* scenes */
sta::MinMaxAll::min(), /* min_max */
Comment on lines +2194 to +2196

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Filter zero-length paths before logging minimum delay

With the unconstrained min-path query left unfiltered, designs with primary/clock startpoints at zero arrival can be selected as the “shortest” path; the new goldens all report RAM minimum path delay: 0.000 ns, which is not a useful RAM min/cycle delay. Please constrain/filter the returned path to real data paths before logging it, otherwise users get a silently misleading minimum-delay report.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Did some investigation and found that because no create_clock is defined in these automated RAM test the fastest min path appear to always be through the latch/flip-flop clock pins which will report a value of zero second which is misleading.

I added in a check to restrict the pin search to only the functional endpoints (flip-flop/latch D pins and primary Q outputs) since this is more accurate of how the RAMs will perform. This should also works with any RAM sizes and configurations instead of trying to define a specific number of pins that we want to restrict the test to.

Update is made in c121752

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the real solution is just that clock should be excluded and everything else should be included.

You may also need to set clocks to propagated? I'm not sure if that affects the datapath timing.

1, /* group_path_count */
1, /* endpoint_path_count */
false, /* unique_pins */
false, /* unique_edges */
-sta::INF, /* slack_min */
sta::INF, /* slack_max */
true, /* sort_by_slack */
group_names, /* group_names */
false, /* setup */
true, /* hold */
false, /* recovery */
false, /* removal */
false, /* clk_gating_setup */
true); /* clk_gating_hold */

float hold_delay
= hold_ends.empty()
? 0.0
: static_cast<float>(hold_ends[0]->dataArrivalTime(sta_));

sta::Unit* time_unit = sta_->units()->timeUnit();
logger_->info(RAM,
44,
"RAM maximum path delay: {} {}",
time_unit->asString(setup_delay),
time_unit->scaleAbbrevSuffix());
logger_->info(RAM,
45,
"RAM minimum path delay: {} {}",
time_unit->asString(hold_delay),
time_unit->scaleAbbrevSuffix());

sta::PowerResult total, sequential, combinational, clock, macro, pad;
sta_->power(
sta_->cmdScene(), total, sequential, combinational, clock, macro, pad);
sta::Unit* power_unit = sta_->units()->powerUnit();
logger_->info(RAM,
46,
"RAM estimated power: {} {}",
power_unit->asString(total.total()),
power_unit->scaleAbbrevSuffix());
}

} // namespace ram
8 changes: 7 additions & 1 deletion src/ram/src/ram.i
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ void ram_routing()
ram_gen->ramRouting(thread_count);
}

void ram_report_timing_power()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As a side note, I think a lot of this function naming is redundant. It is already inside the ram namespace you could just do ram::report_timing_power, etc. Something to perhaps address in a separate PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I ran some tests in setting clocks to propagated by adding set_propagated_clock to the tcl file. It appears to have no effect on the delay and power numbers (since I exclude clock pins in the analysis with the current implementation).

However If you do define a clock I notice that the power reporting will be impacted dramatically. I added a clock using create_clock in the tcl file and the general trend is that shorter period will report higher power (i.e. 1.49 million nW at 0.5ns vs. 374k nW at 5ns and so on). Currently if we don't set a clock the power is likely being underreported (currently 265k nW) but at the same time I'm not sure if there's a "correct" period to set for these tests.

Do you want to leave as is or add an arbitrary clock with period?

{
RamGen* ram_gen = ord::getRamGen();
ram_gen->reportTimingAndPower();
}

void set_behavioral_verilog_filename(const char* filename)
{
RamGen* ram_gen = ord::getRamGen();
Expand All @@ -135,4 +141,4 @@ void set_behavioral_verilog_filename(const char* filename)

} //namespace_ram

%} // inline
%} // inline
2 changes: 2 additions & 0 deletions src/ram/src/ram.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -297,4 +297,6 @@ proc generate_ram { args } {
ram::ram_filler $filler_cells

ram::ram_routing
estimate_parasitics -global_routing
ram::ram_report_timing_power
}
6 changes: 6 additions & 0 deletions src/ram/test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ COMPULSORY_TESTS = [
"make_7x7_2r1w_nangate45",
"make_7x7_nangate45",
"make_8x8_2r1w_sky130",
"make_8x8_latch_sky130",
"make_8x8_mux2_sky130",
"make_8x8_mux4_sky130",
"make_8x8_sky130",
]

Expand Down Expand Up @@ -64,6 +67,9 @@ filegroup(
"Nangate45/Nangate45_typ.lib",
],
"make_8x8_2r1w_sky130": ["make_8x8_2r1w_sky130_behavioral.vok"],
"make_8x8_latch_sky130": ["make_8x8_behavioral.vok"],
"make_8x8_mux2_sky130": ["make_8x8_behavioral.vok"],
"make_8x8_mux4_sky130": ["make_8x8_behavioral.vok"],
"make_8x8_sky130": ["make_8x8_sky130_behavioral.vok"],
}.get(test_name, []),
)
Expand Down
3 changes: 3 additions & 0 deletions src/ram/test/make_7x7_2r1w_nangate45.ok
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@
[INFO DRT-0036] via9 guide region query size = 0.
[INFO DRT-0036] metal10 guide region query size = 0.
[INFO DRT-0179] Init gr pin query.
[INFO RAM-0044] RAM maximum path delay: 0.284 ns
[INFO RAM-0045] RAM minimum path delay: 0.116 ns
[INFO RAM-0046] RAM estimated power: 84905.438 nW
No differences found.
No differences found.
No differences found.
3 changes: 3 additions & 0 deletions src/ram/test/make_7x7_nangate45.ok
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@
[INFO DRT-0036] via9 guide region query size = 0.
[INFO DRT-0036] metal10 guide region query size = 0.
[INFO DRT-0179] Init gr pin query.
[INFO RAM-0044] RAM maximum path delay: 0.282 ns
[INFO RAM-0045] RAM minimum path delay: 0.118 ns
[INFO RAM-0046] RAM estimated power: 55594.766 nW
No differences found.
No differences found.
No differences found.
3 changes: 3 additions & 0 deletions src/ram/test/make_8x8_2r1w_sky130.ok
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@
[INFO DRT-0036] via4 guide region query size = 0.
[INFO DRT-0036] met5 guide region query size = 0.
[INFO DRT-0179] Init gr pin query.
[INFO RAM-0044] RAM maximum path delay: 1.131 ns
[INFO RAM-0045] RAM minimum path delay: 0.395 ns
[INFO RAM-0046] RAM estimated power: 427698.656 nW
No differences found.
No differences found.
No differences found.
Loading
Loading