]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
bitmap: Speed up bitmap_list_view
authorRichard Sandiford <rdsandiford@googlemail.com>
Thu, 4 Jun 2026 15:40:45 +0000 (16:40 +0100)
committerRichard Sandiford <rdsandiford@googlemail.com>
Thu, 4 Jun 2026 15:40:45 +0000 (16:40 +0100)
For tree views, bitmap_clear essentially does:

- a conversion to a list view
- a list clear operation
- a (trivial) conversion back to a tree view

We can do that directly without the need for splay operations.

Doing that removes the only case where bitmap_elt_clear_from is called
for a tree view.  The function can therefore assert for a list view.

That in turn means that bitmap_list_view can inline
bitmap_tree_listify_from without having to handle a partial conversion.

The main optimisation here comes from avoiding the sorted_elements
array by building the list as we go.

We can also avoid the stack array by using the prev fields as a list
link.  Avoiding the stack array might not always be a win, since the
bitmap elements are less compact and because cache locality for the
stack should be good while it remains locally allocated.  My reasoning
was:

(1) The cache lines containing the stack prev fields are going to be
    written to when adding the elements to the list.

(2) If the tree is balanced, it's relatively unlikely that those cache
    lines would be evicted between the two updates.

(3) If the tree isn't well balanced and has a long left spine, we'd
    end up allocating heap memory for the vector, which would add
    a new set of overheads.

I'd be happy to switch back to a vector stack if that turns out
to be better after all.

It's possible to write this version of the algorithm in loop form,
but I thought the structure in the patch was easier to follow.

This gives a reproducible 20% improvement in an artificial test that
converts a tree containing 2,000,000 pseudo-random elements (keeping
the same sequence for all tests).  This testing also ensured that
the resulting list view was correct.

gcc/
* bitmap.cc (bitmap_elt_clear_from): Make static.  Require a
list view.
(bitmap_tree_listify_from): Delete.
(bitmap_list_view): Rewrite to avoid separate vectors.
(bitmap_clear): Switch to a list view before calling
bitmap_elt_clear_from and restore the original view afterwards.

gcc/bitmap.cc

index 005f0abdf6f70962e4dfbd807e7e7b35a516d7f4..6bf0ee99d0488ca5d9d2e16b993b8fa9796af101 100644 (file)
@@ -46,8 +46,6 @@ mem_alloc_description<bitmap_usage> bitmap_mem_desc;
    of bitmap_head.  */
 bitmap_obstack bitmap_head::crashme;
 
-static bitmap_element *bitmap_tree_listify_from (bitmap, bitmap_element *);
-
 /* Register new bitmap.  */
 void
 bitmap_register (bitmap b MEM_STAT_DECL)
@@ -167,18 +165,17 @@ bitmap_element_allocate (bitmap head)
 /* Remove ELT and all following elements from bitmap HEAD.
    Put the released elements in the freelist for HEAD.  */
 
-void
+static void
 bitmap_elt_clear_from (bitmap head, bitmap_element *elt)
 {
   bitmap_element *prev;
   bitmap_obstack *bit_obstack = head->obstack;
 
+  gcc_checking_assert (!head->tree_form);
+
   if (!elt)
     return;
 
-  if (head->tree_form)
-    elt = bitmap_tree_listify_from (head, elt);
-
   if (GATHER_STATISTICS)
     {
       int n = 0;
@@ -609,100 +606,65 @@ bitmap_tree_find_element (bitmap head, unsigned int indx)
 \f
 /* Converting bitmap views from linked-list to tree and vice versa.  */
 
-/* Splice element E and all elements with a larger index from
-   bitmap HEAD, convert the spliced elements to the linked-list
-   view, and return the head of the list (which should be E again),  */
+/* Convert bitmap HEAD from splay-tree view to linked-list view.  */
 
-static bitmap_element *
-bitmap_tree_listify_from (bitmap head, bitmap_element *e)
+void
+bitmap_list_view (bitmap head)
 {
-  bitmap_element *t, *erb;
-
-  /* Detach the right branch from E (all elements with indx > E->indx),
-     and splay E to the root.  */
-  erb = e->next;
-  e->next = NULL;
-  t = bitmap_tree_splay (head, head->first, e->indx);
-  gcc_checking_assert (t == e);
-
-  /* Because E has no right branch, and we rotated it to the root,
-     the left branch is the new root.  */
-  t = e->prev;
-  head->first = t;
-  head->current = t;
-  head->indx = (t != NULL) ? t->indx : 0;
-
-  /* Detach the tree from E, and re-attach the right branch of E.  */
-  e->prev = NULL;
-  e->next = erb;
+  gcc_assert (head->tree_form);
 
-  /* The tree is now valid again.  Now we need to "un-tree" E.
-     It is imperative that a non-recursive implementation is used
-     for this, because splay trees have a worst case depth of O(N)
-     for a tree with N nodes.  A recursive implementation could
-     result in a stack overflow for a sufficiently large, unbalanced
-     bitmap tree.  */
+  if (bitmap_element *node = head->first)
+    {
+      bitmap_element *last = nullptr;
 
-  auto_vec<bitmap_element *, 32> stack;
-  auto_vec<bitmap_element *, 32> sorted_elements;
-  bitmap_element *n = e;
+      /* STACK is a stack of nodes linked by their left child.  Each entry N
+        represents a set of nodes that contains N itself and all nodes in N's
+        right subtree.  The sets are ordered so that every element of the top
+        set comes before every element in the next set down, and so on.  */
+      bitmap_element *stack = nullptr;
 
-  while (true)
-    {
-      while (n != NULL)
+    add_subtree:
+      /* Add NODE and its left and right subtrees to the list.  Start by
+        moving down NODE's left spine, pushing each nonterminal node onto
+        the stack.  */
+      while (bitmap_element *left = node->prev)
        {
-         stack.safe_push (n);
-         n = n->prev;
+         node->prev = stack;
+         stack = node;
+         node = left;
        }
 
-      if (stack.is_empty ())
-       break;
-
-      n = stack.pop ();
-      sorted_elements.safe_push (n);
-      n = n->next;
-    }
-
-  gcc_assert (sorted_elements[0] == e);
-
-  bitmap_element *prev = NULL;
-  unsigned ix;
-  FOR_EACH_VEC_ELT (sorted_elements, ix, n)
-    {
-      if (prev != NULL)
-        prev->next = n;
-      n->prev = prev;
-      n->next = NULL;
-      prev = n;
-    }
-
-  return e;
-}
-
-/* Convert bitmap HEAD from splay-tree view to linked-list view.  */
+    add_node_and_right_subtree:
+      /* Add NODE and its right subtree to the list.  NODE is therefore the
+        next entry in the list.  */
+      if (last)
+       last->next = node;
+      else
+       head->first = node;
+      node->prev = last;
+      last = node;
 
-void
-bitmap_list_view (bitmap head)
-{
-  bitmap_element *ptr;
+      /* Move to NODE's right child and repeat the process.  */
+      node = node->next;
+      if (node)
+       goto add_subtree;
 
-  gcc_assert (head->tree_form);
+      /* Pop the top node from the stack and add it to the end of the list.  */
+      if (stack)
+       {
+         node = stack;
+         stack = stack->prev;
+         goto add_node_and_right_subtree;
+       }
 
-  ptr = head->first;
-  if (ptr)
-    {
-      while (ptr->prev)
-       bitmap_tree_rotate_right (ptr);
-      head->first = ptr;
-      head->first = bitmap_tree_listify_from (head, ptr);
+      if (!head->current)
+       {
+         head->current = head->first;
+         head->indx = head->first->indx;
+       }
     }
 
   head->tree_form = false;
-  if (!head->current)
-    {
-      head->current = head->first;
-      head->indx = head->current ? head->current->indx : 0;
-    }
 }
 
 /* Convert bitmap HEAD from linked-list view to splay-tree view.
@@ -734,16 +696,11 @@ bitmap_clear (bitmap head)
 {
   if (head->first == NULL)
     return;
-  if (head->tree_form)
-    {
-      bitmap_element *e, *t;
-      for (e = head->first; e->prev; e = e->prev)
-       /* Loop to find the element with the smallest index.  */ ;
-      t = bitmap_tree_splay (head, head->first, e->indx);
-      gcc_checking_assert (t == e);
-      head->first = t;
-    }
+  bool tree_form = head->tree_form;
+  if (tree_form)
+    bitmap_list_view (head);
   bitmap_elt_clear_from (head, head->first);
+  head->tree_form = tree_form;
 }
 \f
 /* Initialize a bitmap obstack.  If BIT_OBSTACK is NULL, initialize