dcalc: CCS ReceiverModel accessors, converged-Ceff exposure, reportGateDelay crash fix, receiver-cap Ceff solve - #384
Conversation
…sors Add hasCapacitanceModel()/capacitance() read-only accessors on ReceiverModel to evaluate the CCS receiver-capacitance tables that OpenSTA already parses into capacitance_models_ but never consumes. Used only by the additive OpenROAD report_ccs_receiver_delta diagnostic; the NLDM delay path does not call them, so flag-off behavior is byte-identical to baseline. Signed-off-by: Saurav Singh <saurav.singh@fermions.co>
Additive read-only accessor effectiveCapacitance() on ArcDelayCalc (base returns a negative sentinel) overridden by CcsCeffDelayCalc to return the effective capacitance (region 0) that the most recent gateDelay() converged to, or a negative sentinel when gateDelay() fell back to the NLDM table calc. Used by OpenROAD's report_ccs_driver_delta (dbSta) diagnostic to surface the driver-side Ceff the CCS calculator already computes internally but did not expose. Purely additive: no existing delay value or code path changes, so timing is byte-identical when the accessor is not called. OpenROAD-fork: ccs-delay2 Signed-off-by: Saurav Singh <saurav.singh@fermions.co>
reportGateDelay() can be called directly (e.g. by report_dcalc) without a preceding gateDelay() for the same arc. It used the stale drvr_pin_ member and an unset parasitics_ member, dereferencing bad pointers and crashing. Resolve parasitics_ from the scene and use the drvr_pin parameter, matching gateDelay(). Marked OpenROAD-fork: ccs-delay-path. Signed-off-by: Saurav Singh <saurav.singh@fermions.co>
…ag-gated) Flag-gated (default OFF) addition to CcsCeffDelayCalc: when the receiver model is enabled, the per-region far-cap (c1) contribution of any load pin that carries a CCS receiver_capacitance model is swapped from its constant NLDM pin cap to the library's region-dependent receiver cap (Cr1 in the Miller/transition region below the receiver threshold, Cr2 in the settled region). Pins without a receiver model keep the constant NLDM contribution (graceful fallback). Default OFF -> byte-identical to prior ccs_ceff. - CcsCeffDelayCalc::setReceiverModelEnabled/receiverModelEnabled static toggle - initReceiverModel() gathers per-pin Cr1/Cr2/NLDM caps before the solve - regionFarCap() returns c1_ unchanged when off; region cap when on - DelayCalc.i: set_ccs_receiver_model_enabled / ccs_receiver_model_enabled All src/sta edits marked // OpenROAD-fork: ccs-receiver. Signed-off-by: Saurav Singh <saurav.singh@fermions.co>
|
|
There was a problem hiding this comment.
Code Review
This pull request implements support for the CCS receiver-capacitance model in the Ceff delay calculations, introducing a process-global toggle, region-dependent receiver capacitance evaluations, and exposing the converged effective capacitance. The review feedback identifies three critical safety issues: an uninitialized member variable drvr_pin_ in gateDelay(), a potential null pointer dereference when accessing parasitics_ in reportGateDelay(), and a missing null check for the rf parameter in capacitanceModel().
| parasitics_ = scene->parasitics(min_max); | ||
| parasitic_ = parasitic; | ||
| output_waveforms_ = nullptr; | ||
| last_ceff_ = -1.0; // OpenROAD-fork: ccs-delay2 -- reset; set on CCS path. |
There was a problem hiding this comment.
The member variable drvr_pin_ is never initialized or assigned in gateDelay(). However, it is subsequently used on line 161 (pinPvt(drvr_pin_, scene, min_max)) and in watchWaveform() (line 617). This results in undefined behavior or crashes due to dereferencing an uninitialized or stale pointer. Assign drvr_pin_ = drvr_pin; at the beginning of gateDelay() to resolve this.
drvr_pin_ = drvr_pin;
parasitics_ = scene->parasitics(min_max);
parasitic_ = parasitic;
output_waveforms_ = nullptr;
last_ceff_ = -1.0; // OpenROAD-fork: ccs-delay2 -- reset; set on CCS path.| parasitics_ = scene->parasitics(min_max); | ||
| if (parasitic && !parasitics_->isPiElmore(parasitic)) { | ||
| // Use the drvr_pin PARAMETER, not the stale drvr_pin_ member. | ||
| pi_elmore = | ||
| parasitics_->reduceToPiElmore(parasitic, drvr_pin_, rf, scene, min_max); | ||
| parasitics_->reduceToPiElmore(parasitic, drvr_pin, rf, scene, min_max); | ||
| } | ||
| std::string report = | ||
| table_dcalc_->reportGateDelay(drvr_pin, arc, in_slew, load_cap, pi_elmore, | ||
| load_pin_index_map, scene, min_max, digits); | ||
| parasitics_->deleteDrvrReducedParasitics(drvr_pin); | ||
| parasitics_->deleteDrvrReducedParasitics(drvr_pin); // OpenROAD-fork: ccs-delay-path |
There was a problem hiding this comment.
If scene->parasitics(min_max) returns nullptr (e.g., if parasitics are not defined or SPEF is not loaded for the current corner), dereferencing parasitics_ on lines 796, 799, and 804 will cause a crash (Signal 11). Add a null check for parasitics_ before accessing its members.
parasitics_ = scene->parasitics(min_max);
if (parasitics_ && parasitic && !parasitics_->isPiElmore(parasitic)) {
// Use the drvr_pin PARAMETER, not the stale drvr_pin_ member.
pi_elmore =
parasitics_->reduceToPiElmore(parasitic, drvr_pin, rf, scene, min_max);
}
std::string report =
table_dcalc_->reportGateDelay(drvr_pin, arc, in_slew, load_cap, pi_elmore,
load_pin_index_map, scene, min_max, digits);
if (parasitics_) {
parasitics_->deleteDrvrReducedParasitics(drvr_pin); // OpenROAD-fork: ccs-delay-path
}| { | ||
| const size_t idx = segment * RiseFall::index_count + rf->index(); |
There was a problem hiding this comment.
If the rf parameter is passed as nullptr, calling rf->index() will dereference a null pointer and crash. Adding a defensive null check at the beginning of capacitanceModel() will safely protect this method and its callers (hasCapacitanceModel() and capacitance()).
{
if (rf == nullptr)
return nullptr;
const size_t idx = segment * RiseFall::index_count + rf->index();- reportGateDelay: guard against scene->parasitics(min_max) returning null (parasitics not loaded for the corner) before dereferencing. - ReceiverModel::capacitanceModel: return nullptr on a null RiseFall instead of dereferencing it. Signed-off-by: Saurav Singh <saurav.singh@fermions.co>
ae4fb67 to
e6e286c
Compare
|
Thanks:
On the |
Signed-off-by: James Cherry <cherry@parallaxsw.com>
|
Please push any core dcalc changes in upstream OpenSTA (parallaxsw/OpenSTA repo). We want to ensure the core algorithms are the same in the timing engine for ease of development of the upstream OpenSTA too. Only if this is very specific to OpenROAD alone, would it be okay to have in this fork and not upstream. Could you please provide a justification for this solely to be in OpenROAD/OpenSTA? And if and when you push this upstream, please add a testcase in the sta/test directory. That is the public test area of OpenSTA we have access to. |
Summary
Four CCS (Composite Current Source) delay-calc changes: three purely-additive read-only accessors, one Signal-11 crash fix, and one flag-gated (default-OFF) enhancement to the
ccs_ceffeffective-capacitance solve. With flags off / accessors not called, timing is byte-identical to upstream.1. Read-only
ReceiverModelcapacitance accessors (TableModel.{hh,cc})Adds
hasCapacitanceModel()/capacitance()to evaluate the CCS receiver-capacitance tables OpenSTA already parses intocapacitance_models_but never consumes. Used only by an additive external diagnostic; the NLDM delay path does not call them → flag-off byte-identical.2. Expose converged CCS effective capacitance (
ArcDelayCalc.hh,CcsCeffDelayCalc.*)Additive read-only
effectiveCapacitance()onArcDelayCalc(base returns a negative sentinel), overridden byCcsCeffDelayCalcto return the region-0 effective capacitance the most recentgateDelay()converged to (negative sentinel on NLDM fallback). Purely additive — no existing delay value changes.3.
reportGateDelaycrash fix (CcsCeffDelayCalc.cc)reportGateDelay()can be called directly (e.g. byreport_dcalc) without a precedinggateDelay()for the same arc. It used the staledrvr_pin_member and an unsetparasitics_member, dereferencing bad pointers and crashing (Signal 11). The fix resolvesparasitics_from the scene and uses thedrvr_pinparameter, matchinggateDelay().4. Receiver capacitance in
ccs_ceffCeff solve (CcsCeffDelayCalc.*, flag-gated, default OFF)When the receiver model is enabled, the per-region far-cap (c1) contribution of any load pin carrying a CCS
receiver_capacitancemodel is swapped from its constant NLDM pin cap to the library's region-dependent receiver cap (Cr1 in the Miller/transition region below the receiver threshold, Cr2 in the settled region). Pins without a receiver model keep the constant NLDM contribution (graceful fallback). Default OFF → byte-identical to priorccs_ceff.Testing
tcl.power.vcd_detailed) is a pre-existing last-digit floating-point rounding diff in the power module that fails identically on unmodifiedmaster(unrelated to this change).set_ccs_delay_calcregressions pass; default-OFF behavior verified byte-identical on both the global-NLDM-default and ccs_ceff-receiver-off axes.Notes
dcalc/DelayCalc.iis a SWIG interface;clang-formatintentionally not run onsrc/sta/*per project convention.