diff --git a/src/db/db/dbDevice.cc b/src/db/db/dbDevice.cc index 4e4b36e16..e1dc4eb58 100644 --- a/src/db/db/dbDevice.cc +++ b/src/db/db/dbDevice.cc @@ -163,11 +163,9 @@ void Device::connect_terminal (size_t terminal_id, Net *net) } } -double Device::parameter_value (size_t param_id) const +double Device::parameter_default_value (size_t param_id) const { - if (m_parameters.size () > param_id) { - return m_parameters [param_id]; - } else if (mp_device_class) { + if (mp_device_class) { const db::DeviceParameterDefinition *pd = mp_device_class->parameter_definition (param_id); if (pd) { return pd->default_value (); diff --git a/src/db/db/dbDevice.h b/src/db/db/dbDevice.h index bf7e4dd32..5ca07e915 100644 --- a/src/db/db/dbDevice.h +++ b/src/db/db/dbDevice.h @@ -282,8 +282,13 @@ class DB_PUBLIC Device /** * @brief Gets the value for the parameter with the given ID + * NOTE: this is a hot path in the netlist compare - the fast path (parameter + * present in the parameter vector) is inline, the fallback is not. */ - double parameter_value (size_t param_id) const; + double parameter_value (size_t param_id) const + { + return m_parameters.size () > param_id ? m_parameters [param_id] : parameter_default_value (param_id); + } /** * @brief Sets the value for the parameter with the given ID @@ -397,6 +402,12 @@ class DB_PUBLIC Device friend class Circuit; friend class Net; + /** + * @brief Gets the default value of the parameter with the given ID + * Returns 0 if there is no such parameter. + */ + double parameter_default_value (size_t param_id) const; + DeviceClass *mp_device_class; DeviceAbstract *mp_device_abstract; std::string m_name; diff --git a/src/db/db/dbDeviceClass.cc b/src/db/db/dbDeviceClass.cc index 8aeec11d0..1d67acaab 100644 --- a/src/db/db/dbDeviceClass.cc +++ b/src/db/db/dbDeviceClass.cc @@ -25,6 +25,8 @@ #include "dbNetlist.h" #include "tlClassRegistry.h" +#include + namespace db { @@ -309,29 +311,24 @@ size_t DeviceClass::terminal_id_for_name (const std::string &name) const // The default compare delegate static EqualDeviceParameters default_compare; -bool DeviceClass::less (const db::Device &a, const db::Device &b) +static const db::DeviceParameterCompareDelegate * +compare_delegate (const db::Device &a, const db::Device &b) { tl_assert (a.device_class () != 0); tl_assert (b.device_class () != 0); const db::DeviceParameterCompareDelegate *pcd = primary_device_class (a, b)->parameter_compare_delegate (); - if (! pcd) { - pcd = &default_compare; - } + return pcd ? pcd : &default_compare; +} - return pcd->less (a, b); +bool DeviceClass::less (const db::Device &a, const db::Device &b) +{ + return compare_delegate (a, b)->less (a, b); } bool DeviceClass::equal (const db::Device &a, const db::Device &b) { - tl_assert (a.device_class () != 0); - tl_assert (b.device_class () != 0); - - const db::DeviceParameterCompareDelegate *pcd = primary_device_class (a, b)->parameter_compare_delegate (); - if (! pcd) { - pcd = &default_compare; - } - + const auto *pcd = compare_delegate (a, b); return ! pcd->less (a, b) && ! pcd->less (b, a); } diff --git a/src/db/db/dbNetlistCompareGraph.cc b/src/db/db/dbNetlistCompareGraph.cc index 315257e80..c9081e77e 100644 --- a/src/db/db/dbNetlistCompareGraph.cc +++ b/src/db/db/dbNetlistCompareGraph.cc @@ -113,10 +113,15 @@ Transition::operator< (const Transition &other) const } if (device () != 0) { + + // a single three-way comparison replaces the "equals" plus "less" + // calls and skips the parameter comparison entirely for identical devices DeviceCompare dc; - if (! dc.equals (std::make_pair (device (), cat ()), std::make_pair (other.device (), other.cat ()))) { - return dc (std::make_pair (device (), cat ()), std::make_pair (other.device (), other.cat ())); + int cmp = dc.compare (std::make_pair (device (), cat ()), std::make_pair (other.device (), other.cat ())); + if (cmp != 0) { + return cmp < 0; } + } if (m_id1 != other.m_id1) { @@ -134,6 +139,12 @@ Transition::operator== (const Transition &other) const return false; } + // the ID comparison is a pure conjunction with the other predicates, so + // do the cheap field first + if (m_id1 != other.m_id1) { + return false; + } + if (is_for_subcircuit ()) { if ((subcircuit () != 0) != (other.subcircuit () != 0)) { @@ -147,7 +158,7 @@ Transition::operator== (const Transition &other) const } } - return (m_id1 == other.m_id1); + return true; } else { @@ -156,13 +167,16 @@ Transition::operator== (const Transition &other) const } if (device () != 0) { + + // a single three-way comparison replaces the "equals" call DeviceCompare dc; - if (! dc.equals (std::make_pair (device (), cat ()), std::make_pair (other.device (), other.cat ()))) { + if (dc.compare (std::make_pair (device (), cat ()), std::make_pair (other.device (), other.cat ())) != 0) { return false; } + } - return (m_id1 == other.m_id1 && m_id2 == other.m_id2); + return m_id2 == other.m_id2; } } diff --git a/src/db/db/dbNetlistCompareUtils.cc b/src/db/db/dbNetlistCompareUtils.cc index 88663faee..e40d0ccc8 100644 --- a/src/db/db/dbNetlistCompareUtils.cc +++ b/src/db/db/dbNetlistCompareUtils.cc @@ -197,9 +197,32 @@ DeviceCompare::equals (const std::pair &d1, const st if (d1.second != d2.second) { return false; } + // identical devices are equal under any tolerance scheme + if (d1.first == d2.first) { + return true; + } return db::DeviceClass::equal (*d1.first, *d2.first); } +int +DeviceCompare::compare (const std::pair &d1, const std::pair &d2) const +{ + if (d1.second != d2.second) { + return d1.second < d2.second ? -1 : 1; + } + // identical devices are equal under any tolerance scheme + if (d1.first == d2.first) { + return 0; + } + if (db::DeviceClass::less (*d1.first, *d2.first)) { + return -1; + } else if (db::DeviceClass::less (*d2.first, *d1.first)) { + return 1; + } else { + return 0; + } +} + // -------------------------------------------------------------------------------------------------------------------- // SubCircuitCompare implementation diff --git a/src/db/db/dbNetlistCompareUtils.h b/src/db/db/dbNetlistCompareUtils.h index aa33c9ba2..d4fd7cfe8 100644 --- a/src/db/db/dbNetlistCompareUtils.h +++ b/src/db/db/dbNetlistCompareUtils.h @@ -132,6 +132,15 @@ struct DB_PUBLIC DeviceCompare { bool operator() (const std::pair &d1, const std::pair &d2) const; bool equals (const std::pair &d1, const std::pair &d2) const; + + /** + * @brief Three-way comparison of the devices + * + * This combines "equals" and the "less" operation into a single call: + * it returns -1 if d1 < d2, 1 if d1 > d2 and 0 if both are equal + * under the same criterion "equals" applies. + */ + int compare (const std::pair &d1, const std::pair &d2) const; }; // -------------------------------------------------------------------------------------------------------------------- diff --git a/src/db/unit_tests/dbNetlistCompareTests.cc b/src/db/unit_tests/dbNetlistCompareTests.cc index 49a64c93b..804b4fa61 100644 --- a/src/db/unit_tests/dbNetlistCompareTests.cc +++ b/src/db/unit_tests/dbNetlistCompareTests.cc @@ -438,6 +438,131 @@ TEST(0_EqualDeviceParameters) EXPECT_EQ (dc.less (d2, d1), false); } +// a compare delegate with a fixed "less" result, independent of the parameters +// (like GenericDeviceParameterCompare, it overrides EqualDeviceParameters::less) +class FixedLessCompare + : public db::EqualDeviceParameters +{ +public: + FixedLessCompare (const db::Device *first, bool less_first_second, bool less_second_first) + : m_first (first), m_less_first_second (less_first_second), m_less_second_first (less_second_first) + { } + + virtual bool less (const db::Device &a, const db::Device &b) const + { + return &a == m_first ? m_less_first_second : m_less_second_first; + } + +private: + const db::Device *m_first; + bool m_less_first_second, m_less_second_first; +}; + +// asserts that compare, less and equal agree on the delegate's "less" answers +static void expect_compare (tl::TestBase *_this, const db::Device &a, const db::Device &b, bool less_ab, bool less_ba) +{ + // same category on both sides, so the functor takes the pure device decision + const size_t cat = 0; + db::DeviceCompare dc; + EXPECT_EQ (dc.compare (std::make_pair (&a, cat), std::make_pair (&b, cat)), less_ab ? -1 : (less_ba ? 1 : 0)); + EXPECT_EQ (dc.compare (std::make_pair (&b, cat), std::make_pair (&a, cat)), less_ba ? -1 : (less_ab ? 1 : 0)); + EXPECT_EQ (db::DeviceClass::less (a, b), less_ab); + EXPECT_EQ (db::DeviceClass::less (b, a), less_ba); + EXPECT_EQ (db::DeviceClass::equal (a, b), ! less_ab && ! less_ba); + EXPECT_EQ (db::DeviceClass::equal (b, a), ! less_ab && ! less_ba); +} + +TEST(0_DeviceClassCompare) +{ + db::DeviceClassMOS3Transistor dc; + + db::Device d1 (&dc); + db::Device d2 (&dc); + + // default delegate: primary parameters only, default tolerance + + d1.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 40.0); + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 40.0); + d1.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_W, 20.0); + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_W, 20.0); + + // non-primary parameters are not compared, and neither are parameters beyond the parameter definitions + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_AD, 1.0); + d1.set_parameter_value (100, 5.0); + + expect_compare (_this, d1, d2, false, false); + + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_W, 21.0); + expect_compare (_this, d1, d2, true, false); + + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_W, 20.0); + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 41.0); + expect_compare (_this, d1, d2, true, false); + + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 39.0); + expect_compare (_this, d1, d2, false, true); + + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 40.0); + + // explicit delegate with a tolerance and an ignored parameter + + db::EqualDeviceParameters *eqp = new db::EqualDeviceParameters (); + *eqp += db::EqualDeviceParameters (db::DeviceClassMOS3Transistor::param_id_L, 0.5, 0.0); + *eqp += db::EqualDeviceParameters (db::DeviceClassMOS3Transistor::param_id_W, true); // ignore W + dc.set_parameter_compare_delegate (eqp); + + // W is ignored + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_W, 30.0); + expect_compare (_this, d1, d2, false, false); + + // L is inside the absolute tolerance + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 40.4); + expect_compare (_this, d1, d2, false, false); + + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 40.6); + expect_compare (_this, d1, d2, true, false); + + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 39.4); + expect_compare (_this, d1, d2, false, true); + + d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 40.0); + + // a non-primary parameter is compared when listed in the compare set + *eqp += db::EqualDeviceParameters (db::DeviceClassMOS3Transistor::param_id_AD, 0.0, 0.0); + + expect_compare (_this, d1, d2, true, false); + + d1.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_AD, 1.0); + expect_compare (_this, d1, d2, false, false); + + // a parameter beyond the parameter definitions can be put into the compare set + // (d1 carries 5.0 there, d2 the default value 0.0) + *eqp += db::EqualDeviceParameters (100, 0.25, 0.0); + + expect_compare (_this, d1, d2, false, true); + + d2.set_parameter_value (100, 5.1); + expect_compare (_this, d1, d2, false, false); + + d2.set_parameter_value (100, 5.4); + expect_compare (_this, d1, d2, true, false); + + // a delegate subclass overriding "less" (like GenericDeviceParameterCompare does) + + dc.set_parameter_compare_delegate (new FixedLessCompare (&d1, false, false)); + expect_compare (_this, d1, d2, false, false); + + dc.set_parameter_compare_delegate (new FixedLessCompare (&d1, true, false)); + expect_compare (_this, d1, d2, true, false); + + dc.set_parameter_compare_delegate (new FixedLessCompare (&d1, false, true)); + expect_compare (_this, d1, d2, false, true); + + // "less" in both directions: compare takes the first one, so equal is false + dc.set_parameter_compare_delegate (new FixedLessCompare (&d1, true, true)); + expect_compare (_this, d1, d2, true, true); +} + TEST(0_NetNameEquivalence) { db::Netlist a, b;