diff --git a/include/hal_core/netlist/pins/pin_group.h b/include/hal_core/netlist/pins/pin_group.h index 2e125ff46b9..31f0f526cab 100644 --- a/include/hal_core/netlist/pins/pin_group.h +++ b/include/hal_core/netlist/pins/pin_group.h @@ -426,18 +426,20 @@ namespace hal * Remove a pin from the pin group. * * @param[in] pin - The pin to remove. - * @returns Ok on success, an error message otherwise. + * @returns true on success, false otherwise. */ - Result remove_pin(T* pin) + bool remove_pin(T* pin) { if (pin == nullptr) { - return ERR("'nullptr' given instead of a pin when trying to remove pin from pin group '" + m_name + "' with ID " + std::to_string(m_id)); + log_warning("pin_group", "'nullptr' given instead of a pin when trying to remove pin from pin group '{}' with ID {}.", m_name, m_id); + return false; } if (pin->m_group.first != this) { - return ERR("pin '" + pin->get_name() + "' with ID " + std::to_string(pin->get_id()) + " does not belong to pin group '" + m_name + "' with ID " + std::to_string(m_id)); + log_warning("pin_group", "pin '{}' with ID {} does not belong to pin group '{}' with ID {}.", pin->get_name(), pin->get_id(), m_name, m_id); + return false; } i32 index = pin->m_group.second; @@ -454,16 +456,24 @@ namespace hal } else { - auto it = std::next(m_pins.begin(), m_start_index - index); - it = m_pins.erase(it); - for (; it != m_pins.end(); it++) + if (m_pins.size()==1) { - std::get<1>((*it)->m_group)++; + m_pins.clear(); + m_next_index++; } - m_next_index++; - } + else + { + auto it = m_pins.begin(); + for (int i=m_start_index; i>index;i--) + { + std::get<1>((*(it++))->m_group)--; + } + m_pins.erase(it); + --m_start_index; + } + } - return OK({}); + return true; } /** diff --git a/src/netlist/decorators/netlist_modification_decorator.cpp b/src/netlist/decorators/netlist_modification_decorator.cpp index 73bd446de32..102ac6087c7 100644 --- a/src/netlist/decorators/netlist_modification_decorator.cpp +++ b/src/netlist/decorators/netlist_modification_decorator.cpp @@ -292,7 +292,7 @@ namespace hal // remove pin from current pin group auto current_pin_group = pin->get_group().first; // This get is safe, since we make sure that the pin is a valid pointer and part of the group - current_pin_group->remove_pin(pin).get(); + current_pin_group->remove_pin(pin); // delete old pin group incase it is now empty if (current_pin_group->get_pins().empty()) diff --git a/src/netlist/gate_library/gate_type.cpp b/src/netlist/gate_library/gate_type.cpp index 3a4050ed2f4..7c23a250fd2 100644 --- a/src/netlist/gate_library/gate_type.cpp +++ b/src/netlist/gate_library/gate_type.cpp @@ -370,6 +370,12 @@ namespace hal } PinGroup* pin_group; + if (!ascending && !pins.empty()) + { + // compensate for shifting the start index + start_index -= (pins.size()-1); + } + if (auto res = create_pin_group_internal(id, name, direction, type, ascending, start_index); res.is_error()) { return ERR_APPEND(res.get_error(), "could not create pin group '" + name + "' for gate type '" + m_name + "' with ID " + std::to_string(m_id)); @@ -379,13 +385,23 @@ namespace hal pin_group = res.get(); } - for (auto* pin : pins) + if (ascending) { - if (auto res = assign_pin_to_group(pin_group, pin, delete_empty_groups); res.is_error()) - { - assert(delete_pin_group(pin_group)); - return ERR(res.get_error()); - } + for (auto it = pins.begin(); it != pins.end(); ++it) + if (auto res = assign_pin_to_group(pin_group, *it, delete_empty_groups); res.is_error()) + { + assert(delete_pin_group(pin_group)); + return ERR(res.get_error()); + } + } + else + { + for (auto it = pins.rbegin(); it != pins.rend(); ++it) + if (auto res = assign_pin_to_group(pin_group, *it, delete_empty_groups); res.is_error()) + { + assert(delete_pin_group(pin_group)); + return ERR(res.get_error()); + } } return OK(pin_group); @@ -488,10 +504,9 @@ namespace hal if (PinGroup* pg = pin->get_group().first; pg != nullptr) { // remove from old group and potentially delete old group if empty - if (auto res = pg->remove_pin(pin); res.is_error()) + if (!pg->remove_pin(pin)) { - return ERR_APPEND(res.get_error(), - "could not assign pin '" + pin->get_name() + "' with ID " + std::to_string(pin->get_id()) + " to pin group '" + pin_group->get_name() + "' with ID " + return ERR("could not assign pin '" + pin->get_name() + "' with ID " + std::to_string(pin->get_id()) + " to pin group '" + pin_group->get_name() + "' with ID " + std::to_string(pin_group->get_id()) + " of gate type '" + m_name + "' with ID " + std::to_string(m_id) + ": unable to remove pin from pin group '" + pg->get_name() + "' with ID " + std::to_string(pg->get_id())); } diff --git a/src/netlist/module.cpp b/src/netlist/module.cpp index 0226d386288..79fc829e7c3 100644 --- a/src/netlist/module.cpp +++ b/src/netlist/module.cpp @@ -1243,6 +1243,12 @@ namespace hal } PinGroup* pin_group; + if (!ascending && !pins.empty()) + { + // compensate for shifting the start index + start_index -= (pins.size()-1); + } + if (auto res = create_pin_group_internal(id, name, direction, type, ascending, start_index); res.is_error()) { return ERR_APPEND(res.get_error(), "could not create pin group '" + name + "' for module '" + m_name + "' with ID " + std::to_string(m_id)); @@ -1252,13 +1258,23 @@ namespace hal pin_group = res.get(); } - for (auto* pin : pins) + if (ascending) { - if (!assign_pin_to_group(pin_group, pin, delete_empty_groups)) - { - assert(delete_pin_group(pin_group)); - return ERR("Assign pin to group failed."); - } + for (auto it = pins.begin(); it != pins.end(); ++it) + if (!assign_pin_to_group(pin_group, *it, delete_empty_groups)) + { + assert(delete_pin_group(pin_group)); + return ERR("Assign pin to group failed."); + } + } + else + { + for (auto it = pins.rbegin(); it != pins.rend(); ++it) + if (!assign_pin_to_group(pin_group, *it, delete_empty_groups)) + { + assert(delete_pin_group(pin_group)); + return ERR("Assign pin to group failed."); + } } PinChangedEvent(this,PinEvent::GroupCreate,pin_group->get_id()).send(); @@ -1390,7 +1406,7 @@ namespace hal if (PinGroup* pg = pin->get_group().first; pg != nullptr) { // remove from old group and potentially delete old group if empty - if (auto res = pg->remove_pin(pin); res.is_error()) + if (!pg->remove_pin(pin)) { log_warning("module", "could not assign pin '{}' with ID {} to pin group '{}' with ID {} of module '{}' with ID {}: unable to remove pin from pin group '{}' with ID {}", @@ -1574,7 +1590,7 @@ namespace hal PinGroup* pin_group = pin->get_group().first; assert(pin_group != nullptr); - if (auto res = pin_group->remove_pin(pin); res.is_error()) + if (!pin_group->remove_pin(pin)) { log_warning("module", "could not remove pin '{}' with ID {} from net '{}' with ID {}: failed to remove pin from pin group '{}' with ID {}", pin->get_name(), pin->get_id(), net->get_name(), net->get_id(), pin_group->get_name(), pin_group->get_id()); return false; diff --git a/tests/netlist/module.cpp b/tests/netlist/module.cpp index e8dd56823e4..5e4dc48c573 100644 --- a/tests/netlist/module.cpp +++ b/tests/netlist/module.cpp @@ -1110,6 +1110,9 @@ namespace hal { // create input pin group auto res = m_1->create_pin_group("I", {in_pin_0, in_pin_1}, PinDirection::input, PinType::none, false, 2); ASSERT_TRUE(res.is_ok()); + // descending group start index 2 + // in_pin_0 "I0" index 2 + // in_pin_1 "I1" index 1 PinGroup* in_group = res.get(); ASSERT_NE(in_group, nullptr); EXPECT_EQ(m_1->get_pin_groups().size(), 3); @@ -1138,6 +1141,9 @@ namespace hal { // move pins within group EXPECT_TRUE(m_1->move_pin_within_group(in_group, in_pin_1, 2)); + // descending group start index 2 + // in_pin_1 "I1" index 2 + // in_pin_0 "I0" index 1 EXPECT_EQ(in_pin_0->get_group(), std::pair(in_group, i32(1))); EXPECT_EQ(in_group->get_index(in_pin_0).get(), 1); EXPECT_EQ(in_pin_1->get_group(), std::pair(in_group, i32(2))); @@ -1145,34 +1151,45 @@ namespace hal { EXPECT_EQ(in_group->get_pin_at_index(1).get(), in_pin_0); EXPECT_EQ(in_group->get_pin_at_index(2).get(), in_pin_1); - EXPECT_TRUE(m_1->move_pin_within_group(in_group, in_pin_1, 1)); + EXPECT_TRUE(m_1->move_pin_within_group(in_group, in_pin_0, 2)); + // descending group start index 2 + // in_pin_0 "I0" index 2 + // in_pin_1 "I1" index 1 EXPECT_EQ(in_pin_0->get_group(), std::pair(in_group, i32(2))); EXPECT_EQ(in_group->get_index(in_pin_0).get(), 2); EXPECT_EQ(in_pin_1->get_group(), std::pair(in_group, i32(1))); EXPECT_EQ(in_group->get_index(in_pin_1).get(), 1); EXPECT_EQ(in_group->get_pin_at_index(2).get(), in_pin_0); - EXPECT_EQ(in_group->get_pin_at_index(1).get(), in_pin_1); + EXPECT_EQ(in_group->get_pin_at_index(1).get(), in_pin_1); // remove pin from group EXPECT_TRUE(m_1->remove_pin_from_group(in_group, in_pin_0)); - EXPECT_EQ(in_pin_1->get_group(), std::pair(in_group, i32(2))); - EXPECT_EQ(in_group->get_index(in_pin_1).get(), 2); - EXPECT_EQ(in_group->get_pin_at_index(2).get(), in_pin_1); + // descending group start index 1 + // in_pin_1 "I1" index 1 + EXPECT_EQ(in_group->get_start_index(), 1); + EXPECT_EQ(in_pin_1->get_group(), std::pair(in_group, i32(1))); + EXPECT_EQ(in_group->get_index(in_pin_1).get(), 1); + EXPECT_EQ(in_group->get_pin_at_index(1).get(), in_pin_1); EXPECT_EQ(in_pin_0->get_group().first->get_name(), in_pin_0->get_name()); EXPECT_EQ(in_pin_0->get_group().second, 0); + + // assign pin to group EXPECT_TRUE(m_1->assign_pin_to_group(in_group, in_pin_0)); + // descending group start index 2 + // in_pin_0 "I0" index 2 + // in_pin_1 "I1" index 1 EXPECT_EQ(in_group->size(), 2); - // assign pin to group + // assign same pin twice should not do anything EXPECT_TRUE(m_1->assign_pin_to_group(in_group, in_pin_0)); EXPECT_EQ(m_1->get_pin_groups().size(), 3); EXPECT_EQ(in_group->size(), 2); - EXPECT_EQ(in_pin_0->get_group(), std::pair(in_group, i32(1))); - EXPECT_EQ(in_group->get_index(in_pin_0).get(), 1); - EXPECT_EQ(in_pin_1->get_group(), std::pair(in_group, i32(2))); - EXPECT_EQ(in_group->get_index(in_pin_1).get(), 2); - EXPECT_EQ(in_group->get_pin_at_index(1).get(), in_pin_0); - EXPECT_EQ(in_group->get_pin_at_index(2).get(), in_pin_1); + EXPECT_EQ(in_pin_0->get_group(), std::pair(in_group, i32(2))); + EXPECT_EQ(in_group->get_index(in_pin_0).get(), 2); + EXPECT_EQ(in_pin_1->get_group(), std::pair(in_group, i32(1))); + EXPECT_EQ(in_group->get_index(in_pin_1).get(), 1); + EXPECT_EQ(in_group->get_pin_at_index(2).get(), in_pin_0); + EXPECT_EQ(in_group->get_pin_at_index(1).get(), in_pin_1); } } TEST_END @@ -1257,8 +1274,8 @@ namespace hal { std::make_tuple(ModuleEvent::event::submodule_added, test_mod, other_mod_sub->get_id()), std::make_tuple(ModuleEvent::event::submodule_removed, test_mod, other_mod_sub->get_id()), std::make_tuple(ModuleEvent::event::gate_assigned, test_mod, test_gate->get_id()), - std::make_tuple(ModuleEvent::event::pin_changed, test_mod, NO_DATA), - std::make_tuple(ModuleEvent::event::pin_changed, test_mod, NO_DATA), + std::make_tuple(ModuleEvent::event::pin_changed, test_mod, PinChangedEvent(test_mod,PinEvent::PinRename,3).associated_data()), + std::make_tuple(ModuleEvent::event::pin_changed, test_mod, PinChangedEvent(test_mod,PinEvent::PinRename,1).associated_data()), std::make_tuple(ModuleEvent::event::gate_removed, test_mod, test_gate->get_id()) };