diff mbox series

diagnostic: Use vec instead of custom array reallocations for m_classification_history/m_push_list [PR116847]

Message ID ZvXRAR8J5E9keqrA@tucnak
State New
Headers show
Series diagnostic: Use vec instead of custom array reallocations for m_classification_history/m_push_list [PR116847] | expand

Commit Message

Jakub Jelinek Sept. 26, 2024, 9:24 p.m. UTC 
Hi!

diagnostic.h already relies on vec.h, it uses auto_vec in one spot.

The following patch converts m_classification_history and m_push_list
hand-managed arrays to vec templates.
The main advantage is exponential rather than linear reallocation,
e.g. with current libstdc++ headers if one includes all the standard
headers there could be ~ 300 reallocations of the m_classification_history
array (sure, not all of them will result in actually copying the data, but
still).
In addition to that it fixes some formatting issues in the code.

Bootstrapped on i686-linux so far, bootstrap/regtest on x86_64-linux and
i686-linux still pending, ok for trunk if it passes it?

2024-09-26  Jakub Jelinek  <jakub@redhat.com>

	PR libstdc++/116847
	* diagnostic.h (diagnostic_option_classifier): Change type
	of m_classification_history from diagnostic_classification_change_t *
	to vec<diagnostic_classification_change_t>.  Change type of
	m_push_list from int * to vec<int>.  Remove m_n_classification_history
	and m_n_push members.
	* diagnostic.cc (diagnostic_option_classifier::init): Set m_push_list
	to vNULL rather than nullptr.  Don't initialize m_n_push.  Initialize
	m_classification_history to vNULL.
	(diagnostic_option_classifier::fini): Call release () method on
	m_push_list instead of free on it.  Call release () on
	m_classification_history.  Don't clear m_n_push.
	(diagnostic_option_classifier::push): Adjust for m_push_list and
	m_classification_history being vectors rather than custom allocated
	arrays with counter.
	(diagnostic_option_classifier::pop): Likewise.
	(classify_diagnostic): Adjust for m_classification_history being
	vector rather than custom allocated array with counter.
	(update_effective_level_from_pragmas): Likewise.


	Jakub

Comments

David Malcolm Sept. 27, 2024, 1:32 p.m. UTC | #1 
On Thu, 2024-09-26 at 23:24 +0200, Jakub Jelinek wrote:
> Hi!
> 
> diagnostic.h already relies on vec.h, it uses auto_vec in one spot.
> 
> The following patch converts m_classification_history and m_push_list
> hand-managed arrays to vec templates.
> The main advantage is exponential rather than linear reallocation,
> e.g. with current libstdc++ headers if one includes all the standard
> headers there could be ~ 300 reallocations of the
> m_classification_history
> array (sure, not all of them will result in actually copying the
> data, but
> still).
> In addition to that it fixes some formatting issues in the code.
> 
> Bootstrapped on i686-linux so far, bootstrap/regtest on x86_64-linux
> and
> i686-linux still pending, ok for trunk if it passes it?

Thanks, yes please.

Dave
diff mbox series

Patch

--- gcc/diagnostic.h.jj	2024-09-21 12:28:13.452940750 +0200
+++ gcc/diagnostic.h	2024-09-26 15:42:05.696731787 +0200
@@ -287,14 +287,10 @@  private:
      binary-wise or end-to-front, to find the most recent
      classification for a given diagnostic, given the location of the
      diagnostic.  */
-  diagnostic_classification_change_t *m_classification_history;
-
-  /* The size of the above array.  */
-  int m_n_classification_history;
+  vec<diagnostic_classification_change_t> m_classification_history;
 
   /* For pragma push/pop.  */
-  int *m_push_list;
-  int m_n_push;
+  vec<int> m_push_list;
 };
 
 /* A bundle of options relating to printing the user's source code
--- gcc/diagnostic.cc.jj	2024-09-21 12:28:13.438940942 +0200
+++ gcc/diagnostic.cc	2024-09-26 16:11:51.664502977 +0200
@@ -143,8 +143,8 @@  diagnostic_option_classifier::init (int
   m_classify_diagnostic = XNEWVEC (diagnostic_t, n_opts);
   for (int i = 0; i < n_opts; i++)
     m_classify_diagnostic[i] = DK_UNSPECIFIED;
-  m_push_list = nullptr;
-  m_n_push = 0;
+  m_push_list = vNULL;
+  m_classification_history = vNULL;
 }
 
 void
@@ -152,8 +152,8 @@  diagnostic_option_classifier::fini ()
 {
   XDELETEVEC (m_classify_diagnostic);
   m_classify_diagnostic = nullptr;
-  free (m_push_list);
-  m_n_push = 0;
+  m_classification_history.release ();
+  m_push_list.release ();
 }
 
 /* Save all diagnostic classifications in a stack.  */
@@ -161,8 +161,7 @@  diagnostic_option_classifier::fini ()
 void
 diagnostic_option_classifier::push ()
 {
-  m_push_list = (int *) xrealloc (m_push_list, (m_n_push + 1) * sizeof (int));
-  m_push_list[m_n_push ++] = m_n_classification_history;
+  m_push_list.safe_push (m_classification_history.length ());
 }
 
 /* Restore the topmost classification set off the stack.  If the stack
@@ -173,19 +172,13 @@  diagnostic_option_classifier::pop (locat
 {
   int jump_to;
 
-  if (m_n_push)
-    jump_to = m_push_list [-- m_n_push];
+  if (!m_push_list.is_empty ())
+    jump_to = m_push_list.pop ();
   else
     jump_to = 0;
 
-  const int i = m_n_classification_history;
-  m_classification_history =
-    (diagnostic_classification_change_t *) xrealloc (m_classification_history, (i + 1)
-						     * sizeof (diagnostic_classification_change_t));
-  m_classification_history[i].location = where;
-  m_classification_history[i].option = jump_to;
-  m_classification_history[i].kind = DK_POP;
-  m_n_classification_history ++;
+  diagnostic_classification_change_t v = { where, jump_to, DK_POP };
+  m_classification_history.safe_push (v);
 }
 
 /* Initialize the diagnostic message outputting machinery.  */
@@ -880,31 +873,27 @@  classify_diagnostic (const diagnostic_co
      the pragmas were.  */
   if (where != UNKNOWN_LOCATION)
     {
-      int i;
+      unsigned i;
 
       /* Record the command-line status, so we can reset it back on DK_POP. */
       if (old_kind == DK_UNSPECIFIED)
 	{
-	  old_kind = !context->option_enabled_p (option_id)
-	    ? DK_IGNORED : DK_ANY;
+	  old_kind = (!context->option_enabled_p (option_id)
+		      ? DK_IGNORED : DK_ANY);
 	  m_classify_diagnostic[option_id.m_idx] = old_kind;
 	}
 
-      for (i = m_n_classification_history - 1; i >= 0; i --)
-	if (m_classification_history[i].option == option_id.m_idx)
+      diagnostic_classification_change_t *p;
+      FOR_EACH_VEC_ELT_REVERSE (m_classification_history, i, p)
+	if (p->option == option_id.m_idx)
 	  {
-	    old_kind = m_classification_history[i].kind;
+	    old_kind = p->kind;
 	    break;
 	  }
 
-      i = m_n_classification_history;
-      m_classification_history =
-	(diagnostic_classification_change_t *) xrealloc (m_classification_history, (i + 1)
-							 * sizeof (diagnostic_classification_change_t));
-      m_classification_history[i].location = where;
-      m_classification_history[i].option = option_id.m_idx;
-      m_classification_history[i].kind = new_kind;
-      m_n_classification_history ++;
+      diagnostic_classification_change_t v
+	= { where, option_id.m_idx, new_kind };
+      m_classification_history.safe_push (v);
     }
   else
     m_classify_diagnostic[option_id.m_idx] = new_kind;
@@ -1033,7 +1022,7 @@  diagnostic_t
 diagnostic_option_classifier::
 update_effective_level_from_pragmas (diagnostic_info *diagnostic) const
 {
-  if (m_n_classification_history <= 0)
+  if (m_classification_history.is_empty ())
     return DK_UNSPECIFIED;
 
   /* Iterate over the locations, checking the diagnostic disposition
@@ -1043,27 +1032,26 @@  update_effective_level_from_pragmas (dia
   for (location_t loc: diagnostic->m_iinfo.m_ilocs)
     {
       /* FIXME: Stupid search.  Optimize later. */
-      for (int i = m_n_classification_history - 1; i >= 0; i --)
+      unsigned int i;
+      diagnostic_classification_change_t *p;
+      FOR_EACH_VEC_ELT_REVERSE (m_classification_history, i, p)
 	{
-	  const diagnostic_classification_change_t &hist
-	    = m_classification_history[i];
-
-	  location_t pragloc = hist.location;
+	  location_t pragloc = p->location;
 	  if (!linemap_location_before_p (line_table, pragloc, loc))
 	    continue;
 
-	  if (hist.kind == (int) DK_POP)
+	  if (p->kind == (int) DK_POP)
 	    {
 	      /* Move on to the next region.  */
-	      i = hist.option;
+	      i = p->option;
 	      continue;
 	    }
 
-	  diagnostic_option_id option = hist.option;
+	  diagnostic_option_id option = p->option;
 	  /* The option 0 is for all the diagnostics.  */
 	  if (option == 0 || option == diagnostic->option_id)
 	    {
-	      diagnostic_t kind = hist.kind;
+	      diagnostic_t kind = p->kind;
 	      if (kind != DK_UNSPECIFIED)
 		diagnostic->kind = kind;
 	      return kind;