[consider merging] Fixed a MT problem that can be fixed by avoiding recursive calls of Layout::update

This commit is contained in:
Matthias Koefferlein 2026-01-31 21:53:28 +01:00
parent 955c917783
commit f7870338cf
5 changed files with 221 additions and 49 deletions

View File

@ -981,9 +981,10 @@ public:
* Only after sorting the query iterators are available.
* Sorting complexity is approx O(N*log(N)).
*/
void sort (const BoxConv &conv)
template <class BC>
void sort (const BC &conv)
{
typename BoxConv::complexity complexity_tag;
typename BC::complexity complexity_tag;
sort (conv, complexity_tag);
}
@ -1192,7 +1193,8 @@ private:
box_tree_node *mp_root;
/// Sort implementation for simple bboxes - no caching
void sort (const BoxConv &conv, const db::simple_bbox_tag &/*complexity*/)
template <class BC>
void sort (const BC &conv, const db::simple_bbox_tag &/*complexity*/)
{
m_elements.clear ();
m_elements.reserve (m_objects.size ());
@ -1204,7 +1206,7 @@ private:
if (! m_objects.empty ()) {
box_tree_picker_type picker (conv);
box_tree_picker<Box, Obj, BC, obj_vector_type> picker (conv);
box_type bbox;
for (typename obj_vector_type::const_iterator o = m_objects.begin (); o != m_objects.end (); ++o) {
@ -1221,7 +1223,8 @@ private:
}
/// Sort implementation for complex bboxes - with caching
void sort (const box_conv_type &conv, const db::complex_bbox_tag &/*complexity*/)
template <class BC>
void sort (const BC &conv, const db::complex_bbox_tag &/*complexity*/)
{
m_elements.clear ();
m_elements.reserve (m_objects.size ());
@ -1233,7 +1236,7 @@ private:
if (! m_objects.empty ()) {
box_tree_cached_picker<object_type, box_type, box_conv_type, obj_vector_type> picker (conv, m_objects.begin (), m_objects.end ());
box_tree_cached_picker<object_type, box_type, BC, obj_vector_type> picker (conv, m_objects.begin (), m_objects.end ());
for (typename obj_vector_type::const_iterator o = m_objects.begin (); o != m_objects.end (); ++o) {
m_elements.push_back (o.index ());
@ -1997,9 +2000,10 @@ public:
* Only after sorting the query iterators are available.
* Sorting complexity is approx O(N*log(N)).
*/
void sort (const BoxConv &conv)
template <class BC>
void sort (const BC &conv)
{
typename BoxConv::complexity complexity_tag;
typename BC::complexity complexity_tag;
sort (conv, complexity_tag);
}
@ -2159,13 +2163,14 @@ private:
box_tree_node *mp_root;
/// Sort implementation for simple bboxes - no caching
void sort (const BoxConv &conv, const db::simple_bbox_tag &/*complexity*/)
template <class BC>
void sort (const BC &conv, const db::simple_bbox_tag &/*complexity*/)
{
if (m_objects.empty ()) {
return;
}
box_tree_picker_type picker (conv);
box_tree_picker<box_type, object_type, BC, obj_vector_type> picker (conv);
if (mp_root) {
delete mp_root;
@ -2184,13 +2189,14 @@ private:
}
/// Sort implementation for complex bboxes - with caching
void sort (const box_conv_type &conv, const db::complex_bbox_tag &/*complexity*/)
template <class BC>
void sort (const BC &conv, const db::complex_bbox_tag &/*complexity*/)
{
if (m_objects.empty ()) {
return;
}
box_tree_cached_picker<object_type, box_type, box_conv_type, obj_vector_type> picker (conv, m_objects.begin (), m_objects.end ());
box_tree_cached_picker<object_type, box_type, BC, obj_vector_type> picker (conv, m_objects.begin (), m_objects.end ());
if (mp_root) {
delete mp_root;

View File

@ -274,11 +274,67 @@ Cell::is_shape_bbox_dirty () const
return false;
}
namespace
{
/**
* @brief An alternative box converter for CellInst which does not call layout.update
*
* Using this converter in cell.update_bbox() prevents recursive calls to layout.update.
*/
class InternalCellInstBoxConverter
{
public:
typedef db::Cell::box_type box_type;
typedef db::simple_bbox_tag complexity;
InternalCellInstBoxConverter (const db::Layout *layout)
: mp_layout (layout)
{
// .. nothing yet ..
}
box_type operator() (const db::CellInst &cell_inst) const
{
return mp_layout->cell (cell_inst.cell_index ()).bbox_with_empty_no_update ();
}
private:
const db::Layout *mp_layout;
};
/**
* @brief An alternative box converter for CellInst and layer which does not call layout.update
*
* Using this converter in cell.update_bbox() prevents recursive calls to layout.update.
*/
class InternalPerLayerCellInstBoxConverter
{
public:
typedef db::Cell::box_type box_type;
typedef db::simple_bbox_tag complexity;
InternalPerLayerCellInstBoxConverter (const db::Layout *layout, unsigned int layer)
: mp_layout (layout), m_layer (layer)
{
// .. nothing yet ..
}
box_type operator() (const db::CellInst &cell_inst) const
{
return mp_layout->cell (cell_inst.cell_index ()).bbox_no_update (m_layer);
}
private:
const db::Layout *mp_layout;
unsigned int m_layer;
};
}
bool
Cell::update_bbox (unsigned int layers)
{
unsigned int l;
// determine the bounding box
box_type org_bbox = m_bbox;
m_bbox = box_type ();
@ -308,10 +364,10 @@ Cell::update_bbox (unsigned int layers)
++o;
}
for (l = 0; l < layers; ++l) {
for (unsigned int l = 0; l < layers; ++l) {
// the per-layer bounding boxes
db::box_convert <cell_inst_type> bc (*mp_layout, l);
InternalPerLayerCellInstBoxConverter bc (mp_layout, l);
box_type lbox = o1_inst->bbox_from_raw_bbox (raw_box, bc);
if (! lbox.empty ()) {
@ -326,7 +382,7 @@ Cell::update_bbox (unsigned int layers)
}
db::box_convert <cell_inst_type, false> bc_we (*mp_layout);
InternalCellInstBoxConverter bc_we (mp_layout);
m_bbox_with_empty += o1_inst->bbox_from_raw_bbox (raw_box, bc_we);
}
@ -486,6 +542,17 @@ Cell::bbox (unsigned int l) const
}
}
const Cell::box_type &
Cell::bbox_no_update (unsigned int l) const
{
box_map::const_iterator b = m_bboxes.find (l);
if (b != m_bboxes.end ()) {
return b->second;
} else {
return ms_empty_box;
}
}
Cell::const_iterator
Cell::begin () const
{
@ -725,7 +792,7 @@ Cell::count_hier_levels () const
{
unsigned int l = 0;
for (const_iterator c = begin (); !c.at_end (); ++c) {
for (const_iterator c = m_instances.begin (); !c.at_end (); ++c) {
l = std::max (l, (unsigned int) mp_layout->cell (c->cell_index ()).m_hier_levels + 1);
}

View File

@ -522,21 +522,6 @@ public:
*/
bool is_shape_bbox_dirty () const;
/**
* @brief Updates the bbox
*
* This will update the bbox from the shapes and instances.
* This requires the bboxes of the child cells to be computed
* before. Practically this will be done by computing the
* bboxes bottom-up in the hierarchy.
* In addition, the number of hierarchy levels below is also
* updated.
*
* @param layers The max. number of layers in the child cells
* @return true, if the bounding box has changed.
*/
bool update_bbox (unsigned int layers);
/**
* @brief Sorts the shapes lists
*
@ -1110,6 +1095,23 @@ public:
*/
void move_shapes (db::Cell &source_cell, const db::LayerMapping &layer_mapping);
/**
* @brief Gets the current bounding box (with empty) without calling Layout::update
*
* This method is intended for internal purposes only.
*/
const box_type &bbox_with_empty_no_update () const
{
return m_bbox_with_empty;
}
/**
* @brief Gets the current per-layer bounding box without calling Layout::update
*
* This method is intended for internal purposes only.
*/
const box_type &bbox_no_update (unsigned int l) const;
protected:
/**
* @brief Standard constructor: create an empty cell object
@ -1135,6 +1137,7 @@ protected:
virtual Cell *clone (db::Layout &layout) const;
private:
friend class db::Layout;
cell_index_type m_cell_index;
mutable db::Layout *mp_layout;
shapes_map m_shapes_map;
@ -1170,9 +1173,9 @@ private:
}
/**
* @brief Return a reference to the instances object
* @brief Return a reference to the instances object
*/
instances_type &instances ()
const instances_type &instances () const
{
return m_instances;
}
@ -1180,7 +1183,7 @@ private:
/**
* @brief Return a reference to the instances object
*/
const instances_type &instances () const
instances_type &instances ()
{
return m_instances;
}
@ -1219,6 +1222,21 @@ private:
* @param force Force sorting, even if not strictly needed
*/
void sort_inst_tree (bool force);
/**
* @brief Updates the bbox
*
* This will update the bbox from the shapes and instances.
* This requires the bboxes of the child cells to be computed
* before. Practically this will be done by computing the
* bboxes bottom-up in the hierarchy.
* In addition, the number of hierarchy levels below is also
* updated.
*
* @param layers The max. number of layers in the child cells
* @return true, if the bounding box has changed.
*/
bool update_bbox (unsigned int layers);
};
/**

View File

@ -1524,6 +1524,84 @@ Instances::sort_child_insts (bool force)
std::sort (m_insts_by_cell_index.begin (), m_insts_by_cell_index.end (), cell_inst_compare_f<basic_inst_type> ());
}
namespace {
/**
* @brief An alternative box converter for CellInst which does not call layout.update
*
* Using this converter in cell.update_bbox() prevents recursive calls to layout.update.
*/
class InternalCellInstBoxConverter
{
public:
typedef db::Cell::box_type box_type;
typedef db::simple_bbox_tag complexity;
InternalCellInstBoxConverter (const db::Layout *layout)
: mp_layout (layout)
{
// .. nothing yet ..
}
db::Cell::box_type operator() (const db::Cell::cell_inst_type &cell_inst) const
{
return mp_layout->cell (cell_inst.cell_index ()).bbox_with_empty_no_update ();
}
private:
const db::Layout *mp_layout;
};
/**
* @brief An alternative box converter for CellInstArray with properties which does not call layout.update
*
* Using this converter in cell.update_bbox() prevents recursive calls to layout.update.
*/
struct InternalCellInstArrayWithPropertiesBoxConverter
{
typedef db::Cell::cell_inst_array_type cell_inst_array;
typedef db::Cell::box_type box_type;
typedef db::complex_bbox_tag complexity;
InternalCellInstArrayWithPropertiesBoxConverter (const db::Layout *layout)
: m_bc (layout)
{ }
box_type operator() (const db::object_with_properties<cell_inst_array> &array) const
{
return array.bbox (m_bc);
}
private:
InternalCellInstBoxConverter m_bc;
};
/**
* @brief An alternative box converter for CellInstArray which does not call layout.update
*
* Using this converter in cell.update_bbox() prevents recursive calls to layout.update.
*/
struct InternalCellInstArrayBoxConverter
{
typedef db::Cell::cell_inst_array_type cell_inst_array;
typedef db::Cell::box_type box_type;
typedef db::complex_bbox_tag complexity;
InternalCellInstArrayBoxConverter (const db::Layout *layout)
: m_bc (layout)
{ }
box_type operator() (const cell_inst_array &array) const
{
return array.bbox (m_bc);
}
private:
InternalCellInstBoxConverter m_bc;
};
}
void
Instances::sort_inst_tree (const Layout *g, bool force)
{
@ -1534,18 +1612,18 @@ Instances::sort_inst_tree (const Layout *g, bool force)
if (m_generic.any) {
if (is_editable ()) {
m_generic.stable_tree->sort (cell_inst_array_box_converter (*g));
m_generic.stable_tree->sort (InternalCellInstArrayBoxConverter (g));
} else {
m_generic.unstable_tree->sort (cell_inst_array_box_converter (*g));
m_generic.unstable_tree->sort (InternalCellInstArrayBoxConverter (g));
// since we use unstable instance trees in non-editable mode, we need to resort the child instances in this case
sort_child_insts (true);
}
}
if (m_generic_wp.any) {
if (is_editable ()) {
m_generic_wp.stable_tree->sort (cell_inst_wp_array_box_converter (*g));
m_generic_wp.stable_tree->sort (InternalCellInstArrayWithPropertiesBoxConverter (g));
} else {
m_generic_wp.unstable_tree->sort (cell_inst_wp_array_box_converter (*g));
m_generic_wp.unstable_tree->sort (InternalCellInstArrayWithPropertiesBoxConverter (g));
// since we use unstable instance trees in non-editable mode, we need to resort the child instances in this case
sort_child_insts (true);
}

View File

@ -1532,6 +1532,8 @@ Layout::rename_cell (cell_index_type id, const char *name)
bool
Layout::topological_sort ()
{
// NOTE: using cell.instances methods instead of the corresponding cell methods avoids a recursive update call
m_top_cells = 0;
m_top_down_list.clear ();
@ -1559,7 +1561,7 @@ Layout::topological_sort ()
// child cells.
for (const_iterator c = begin (); c != end (); ++c) {
if (c->parent_cells () == num_parents [c->cell_index ()]) {
if (c->instances ().parent_cells () == num_parents [c->cell_index ()]) {
m_top_down_list.push_back (c->cell_index ());
num_parents [c->cell_index ()] = std::numeric_limits<cell_index_type>::max ();
}
@ -1568,7 +1570,7 @@ Layout::topological_sort ()
// For all these a cells, increment the reported parent instance
// count in all the child cells.
for (cell_index_vector::const_iterator ii = m_top_down_list.begin () + n_top_down_cells; ii != m_top_down_list.end (); ++ii) {
for (cell_type::child_cell_iterator cc = cell (*ii).begin_child_cells (); ! cc.at_end (); ++cc) {
for (cell_type::child_cell_iterator cc = cell (*ii).instances ().begin_child_cells (); ! cc.at_end (); ++cc) {
tl_assert (num_parents [*cc] != std::numeric_limits<cell_index_type>::max ());
num_parents [*cc] += 1;
}
@ -1583,7 +1585,7 @@ Layout::topological_sort ()
}
// Determine the number of top cells
for (top_down_iterator e = m_top_down_list.begin (); e != m_top_down_list.end () && cell (*e).is_top (); ++e) {
for (top_down_iterator e = m_top_down_list.begin (); e != m_top_down_list.end () && cell (*e).instances ().is_top (); ++e) {
++m_top_cells;
}
@ -1815,6 +1817,8 @@ Layout::force_update_no_lock () const
void
Layout::update () const
{
tl::MutexLocker locker (&lock ());
// NOTE: the assumption is that either one thread is writing or
// multiple threads are reading. Hence, we do not need to lock hier_dirty() or bboxes_dirty().
// We still do double checking as another thread might do the update as well.
@ -1822,8 +1826,6 @@ Layout::update () const
return;
}
tl::MutexLocker locker (&lock ());
if (! under_construction ()) {
force_update_no_lock ();
}
@ -1886,13 +1888,14 @@ Layout::do_update ()
unsigned int layers = 0;
pr->set (0);
pr->set_desc (tl::to_string (tr ("Updating bounding boxes")));
for (bottom_up_iterator c = begin_bottom_up (); c != end_bottom_up (); ++c) {
for (bottom_up_iterator c = m_top_down_list.rbegin (); c != m_top_down_list.rend (); ++c) {
++*pr;
cell_type &cp (cell (*c));
if (cp.is_shape_bbox_dirty () || dirty_parents.find (*c) != dirty_parents.end ()) {
if (cp.update_bbox (layers)) {
// the bounding box has changed - need to insert parents into "dirty parents" list
for (cell_type::parent_cell_iterator p = cp.begin_parent_cells (); p != cp.end_parent_cells (); ++p) {
// NOTE: using "instances" instead of the cell directly avoids a recursive update call
for (cell_type::parent_cell_iterator p = cp.instances ().begin_parent_cells (); p != cp.instances ().end_parent_cells (); ++p) {
dirty_parents.insert (*p);
}
}
@ -1907,7 +1910,7 @@ Layout::do_update ()
tl::SelfTimer timer (tl::verbosity () > layout_base_verbosity + 10, "Sorting shapes");
pr->set (0);
pr->set_desc (tl::to_string (tr ("Sorting shapes")));
for (bottom_up_iterator c = begin_bottom_up (); c != end_bottom_up (); ++c) {
for (bottom_up_iterator c = m_top_down_list.rbegin (); c != m_top_down_list.rend (); ++c) {
++*pr;
cell_type &cp (cell (*c));
cp.sort_shapes ();
@ -1922,7 +1925,7 @@ Layout::do_update ()
size_t layers = 0;
pr->set (0);
pr->set_desc (tl::to_string (tr ("Sorting instances")));
for (bottom_up_iterator c = begin_bottom_up (); c != end_bottom_up (); ++c) {
for (bottom_up_iterator c = m_top_down_list.rbegin (); c != m_top_down_list.rend (); ++c) {
++*pr;
cell_type &cp (cell (*c));
bool force_sort_inst_tree = dirty_parents.find (*c) != dirty_parents.end ();