Skip to content

Commit

Permalink
[RR_GRAPH] Fixed Bug in LLVM17 Build
Browse files Browse the repository at this point in the history
The std::sort function requires operands to be "pointer-like", this
requires that de-referencing a const pointer should be allowed.

This was not an issue in the past, but in LLVM17 this property is
required. The edge_sort_iterator did not follow this property.

Fixing this by naively allowing this property led to a 10% performance
degredation due to requiring pointers to be allocated and deallocated.

To fix this for now, made the sort use std::stable_sort instead. The
performance degredation is quite small and may be worth it.
  • Loading branch information
AlexandreSinger committed Apr 2, 2024
1 parent 633193b commit 973acb7
Showing 1 changed file with 9 additions and 2 deletions.
11 changes: 9 additions & 2 deletions libs/librrgraph/src/base/rr_graph_storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,13 @@ class edge_sort_iterator {
using pointer = edge_swapper*;
using difference_type = ssize_t;

// In order for this class to be used as an iterator within the std library,
// it needs to "act" like a pointer. One thing that it should do is that a
// const variable of this type should be de-referenceable. Therefore, this
// method should be const method; however, this requires modifying the class
// and may yield worst performance. For now the std::stable_sort allows this
// but in the future it may not. If this breaks, this is why.
// See issue #2517 and PR #2522
edge_swapper& operator*() {
return this->swapper_;
}
Expand Down Expand Up @@ -419,7 +426,7 @@ size_t t_rr_graph_storage::count_rr_switches(
// values.
//
// This sort is safe to do because partition_edges() has not been invoked yet.
std::sort(
std::stable_sort(
edge_sort_iterator(this, 0),
edge_sort_iterator(this, edge_dest_node_.size()),
edge_compare_dest_node());
Expand Down Expand Up @@ -527,7 +534,7 @@ void t_rr_graph_storage::partition_edges(const vtr::vector<RRSwitchId, t_rr_swit
// by assign_first_edges()
// - Edges within a source node have the configurable edges before the
// non-configurable edges.
std::sort(
std::stable_sort(
edge_sort_iterator(this, 0),
edge_sort_iterator(this, edge_src_node_.size()),
edge_compare_src_node_and_configurable_first(rr_switches));
Expand Down

0 comments on commit 973acb7

Please sign in to comment.