diff mbox series

Fix thinko in early bail out in tree-switch-conversion.

Message ID a910024a-87d5-3431-752e-53c9996fb998@suse.cz
State New
Headers show
Series Fix thinko in early bail out in tree-switch-conversion. | expand

Commit Message

Martin Liška Aug. 30, 2019, 8:41 a.m. UTC 
Hi.

Thanks to Jakub, the patch addresses one memory leak in
bit_test_cluster::find_bit_tests (allocation of output).
And moreover, it implements proper guard when clustering
is not successful. In such situation we want a quick bail out.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-08-29  Martin Liska  <mliska@suse.cz>

	* tree-switch-conversion.c (jump_table_cluster::find_jump_tables):
	Bail out when we'll end up with the same number of clusters as
	at the beginning.
	(bit_test_cluster::find_bit_tests): Likewise for bit tests.
	(jump_table_cluster::can_be_handled): Remove the guard
	as it's already handled in ::is_enabled.  Allocate output
	after early bail out.
---
 gcc/tree-switch-conversion.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

Comments

Jeff Law Aug. 30, 2019, 4:28 p.m. UTC | #1 
On 8/30/19 2:41 AM, Martin Liška wrote:
> Hi.
> 
> Thanks to Jakub, the patch addresses one memory leak in
> bit_test_cluster::find_bit_tests (allocation of output).
> And moreover, it implements proper guard when clustering
> is not successful. In such situation we want a quick bail out.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2019-08-29  Martin Liska  <mliska@suse.cz>
> 
> 	* tree-switch-conversion.c (jump_table_cluster::find_jump_tables):
> 	Bail out when we'll end up with the same number of clusters as
> 	at the beginning.
> 	(bit_test_cluster::find_bit_tests): Likewise for bit tests.
> 	(jump_table_cluster::can_be_handled): Remove the guard
> 	as it's already handled in ::is_enabled.  Allocate output
> 	after early bail out.
> ---
>  gcc/tree-switch-conversion.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> 
OK
jeff
diff mbox series

Patch

diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
index 776db77f53a..988e923d7e8 100644
--- a/gcc/tree-switch-conversion.c
+++ b/gcc/tree-switch-conversion.c
@@ -1213,14 +1213,14 @@  jump_table_cluster::find_jump_tables (vec<cluster *> &clusters)
     }
 
   /* No result.  */
-  if (min[l].m_count == INT_MAX)
+  if (min[l].m_count == l)
     return clusters.copy ();
 
   vec<cluster *> output;
   output.create (4);
 
   /* Find and build the clusters.  */
-  for (int end = l;;)
+  for (unsigned int end = l;;)
     {
       int start = min[end].m_start;
 
@@ -1257,11 +1257,9 @@  jump_table_cluster::can_be_handled (const vec<cluster *> &clusters,
      make a sequence of conditional branches instead of a dispatch.
 
      The definition of "much bigger" depends on whether we are
-     optimizing for size or for speed.  */
-  if (!flag_jump_tables)
-    return false;
+     optimizing for size or for speed.
 
-  /* For algorithm correctness, jump table for a single case must return
+     For algorithm correctness, jump table for a single case must return
      true.  We bail out in is_beneficial if it's called just for
      a single case.  */
   if (start == end)
@@ -1311,9 +1309,6 @@  jump_table_cluster::is_beneficial (const vec<cluster *> &,
 vec<cluster *>
 bit_test_cluster::find_bit_tests (vec<cluster *> &clusters)
 {
-  vec<cluster *> output;
-  output.create (4);
-
   unsigned l = clusters.length ();
   auto_vec<min_cluster_item> min;
   min.reserve (l + 1);
@@ -1336,9 +1331,12 @@  bit_test_cluster::find_bit_tests (vec<cluster *> &clusters)
     }
 
   /* No result.  */
-  if (min[l].m_count == INT_MAX)
+  if (min[l].m_count == l)
     return clusters.copy ();
 
+  vec<cluster *> output;
+  output.create (4);
+
   /* Find and build the clusters.  */
   for (unsigned end = l;;)
     {