Simplify switch bit test clustering algorithm

The current switch bit test clustering enumerates all possible case
clusters combinations to find ones that fit the bit test constrains
best.  This causes performance problems with very large switches.

For bit test clustering which happens naturally in word sized chunks
I don't think such an expensive algorithm is really needed.

This patch implements a simple greedy algorithm that walks
the sorted list and examines word sized windows and tries
to cluster them.

Surprisingly the new algorithm gives consistly better clusters
for the examples I tried.

For example from the gcc bootstrap:

old: 0-15 16-31 96-175
new: 0-31 96-175

I'm not fully sure why that is, probably some bug in the old
algorithm? This shows even up in the test suite where if-to-switch-6
now can generate a switch, as well as a case in switch-1.c

I don't have a proof that the new algorithm is always as good or better,
but so far at least I don't see any counter examples.

It also fixes the excessive compile time in PR117091,
however this was already fixed by an earlier patch
that doesn't run clustering when no targets have multiple
values.

gcc/ChangeLog:

	PR middle-end/117091
	* tree-switch-conversion.cc (bit_test_cluster::find_bit_tests):
	Change clustering algorithm to simple greedy.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/if-to-switch-6.c: Allow condition chain.
	* gcc.dg/tree-ssa/switch-1.c: Allow more bit tests.
	* gcc.dg/pr21643.c: Use -fno-bit-tests
	* gcc.target/aarch64/pr99988.c: Use -fno-bit-tests

diff --git a/gcc/testsuite/gcc.dg/pr21643.c b/gcc/testsuite/gcc.dg/pr21643.c
index 42517b5af1e5b4bc28944729b6ea940adaaf85e6..a722a83ecb59a7c391e6d8adfe41ba39b65cada2 100644 (file)
--- a/gcc/testsuite/gcc.dg/pr21643.c
+++ b/gcc/testsuite/gcc.dg/pr21643.c
@@ -1,6 +1,6 @@
 /* PR tree-optimization/21643 */
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-reassoc1-details --param logical-op-non-short-circuit=1" } */
+/* { dg-options "-O2 -fdump-tree-reassoc1-details --param logical-op-non-short-circuit=1 -fno-bit-tests" } */
 
 int
 f1 (unsigned char c)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/if-to-switch-6.c b/gcc/testsuite/gcc.dg/tree-ssa/if-to-switch-6.c
index b1640673eae15daac38e92beedcb961d3b5bd489..657af770e438d0ccc0a95b8a080c10d4eecd021b 100644 (file)
--- a/gcc/testsuite/gcc.dg/tree-ssa/if-to-switch-6.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/if-to-switch-6.c
@@ -39,4 +39,4 @@ int main(int argc, char **argv)
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-not "Condition chain" "iftoswitch" } } */
+/* { dg-final { scan-tree-dump "Condition chain" "iftoswitch" } } */

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/switch-1.c b/gcc/testsuite/gcc.dg/tree-ssa/switch-1.c
index 6f70c9de0c19b630ad30d3635787720faf2d8c15..f1654aba6d99d5f5a850e04d03c2ca9f46d96f78 100644 (file)
--- a/gcc/testsuite/gcc.dg/tree-ssa/switch-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/switch-1.c
@@ -107,4 +107,4 @@ int foo5 (int x)
   }
 }
 
-/* { dg-final { scan-tree-dump ";; GIMPLE switch case clusters: BT:10-62 600-700 JT:1000-1021 111111" "switchlower1" } } */
+/* { dg-final { scan-tree-dump ";; GIMPLE switch case clusters: BT:10-62 600-700 BT:1000-1021 111111" "switchlower1" } } */

diff --git a/gcc/testsuite/gcc.target/aarch64/pr99988.c b/gcc/testsuite/gcc.target/aarch64/pr99988.c
index 7cca4962944616ff24835d497ad6da235daeb0a8..c09ce67c0fa93367097111b1946cb57d703ea56c 100644 (file)
--- a/gcc/testsuite/gcc.target/aarch64/pr99988.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr99988.c
@@ -1,5 +1,5 @@
 /* { dg-do compile { target lp64 } } */
-/* { dg-options "-O2 -mbranch-protection=standard" } */
+/* { dg-options "-O2 -mbranch-protection=standard -fno-bit-tests" } */
 /* { dg-final { scan-assembler-times {bti j} 13 } } */
 int a;
 int c();

diff --git a/gcc/tree-switch-conversion.cc b/gcc/tree-switch-conversion.cc
index 852419b2f4becc6407007a1aca4d9984b8c8a095..9b4ddcd0146d2bc7410c104f6553dc3a924dbbd7 100644 (file)
--- a/gcc/tree-switch-conversion.cc
+++ b/gcc/tree-switch-conversion.cc
@@ -1783,55 +1783,62 @@ bit_test_cluster::find_bit_tests (vec<cluster *> &clusters, int max_c)
     return clusters.copy ();
 
   unsigned l = clusters.length ();
-  auto_vec<min_cluster_item> min;
-  min.reserve (l + 1);
+  vec<cluster *> output;
 
-  min.quick_push (min_cluster_item (0, 0, 0));
+  output.create (l);
 
-  for (unsigned i = 1; i <= l; i++)
+  /* Look at sliding BITS_PER_WORD sized windows in the switch value space
+     and determine if they are suitable for a bit test cluster.  Worst case
+     this can examine every value BITS_PER_WORD-1 times.  */
+  unsigned end;
+  for (unsigned i = 0; i < l; i += end)
     {
-      /* Set minimal # of clusters with i-th item to infinite.  */
-      min.quick_push (min_cluster_item (INT_MAX, INT_MAX, INT_MAX));
+      HOST_WIDE_INT values = 0;
+      hash_set<basic_block> targets;
+      cluster *start_cluster = clusters[i];
 
-      for (unsigned j = 0; j < i; j++)
+      end = 0;
+      while (i + end < l)
        {
-         if (min[j].m_count + 1 < min[i].m_count
-             && can_be_handled (clusters, j, i - 1))
-           min[i] = min_cluster_item (min[j].m_count + 1, j, INT_MAX);
+         cluster *end_cluster = clusters[i + end];
+
+         /* Does value range fit into the BITS_PER_WORD window?  */
+         HOST_WIDE_INT w = cluster::get_range (start_cluster->get_low (),
+                                               end_cluster->get_high ());
+         if (w == 0 || w > BITS_PER_WORD)
+           break;
+
+         /* Compute # of values tested for new case.  */
+         HOST_WIDE_INT r = 1;
+         if (!end_cluster->is_single_value_p ())
+           r = cluster::get_range (end_cluster->get_low (),
+                                   end_cluster->get_high ());
+         if (r == 0)
+           break;
+
+         /* Check for max # of targets.  */
+         if (targets.elements() == m_max_case_bit_tests
+             && !targets.contains (end_cluster->m_case_bb))
+           break;
+
+         targets.add (end_cluster->m_case_bb);
+         values += r;
+         end++;
        }
 
-      gcc_checking_assert (min[i].m_count != INT_MAX);
-    }
-
-  /* No result.  */
-  if (min[l].m_count == l)
-    return clusters.copy ();
-
-  vec<cluster *> output;
-  output.create (4);
-
-  /* Find and build the clusters.  */
-  for (unsigned end = l;;)
-    {
-      int start = min[end].m_start;
-
-      if (is_beneficial (clusters, start, end - 1))
+      if (is_beneficial (values, targets.elements ()))
        {
-         bool entire = start == 0 && end == clusters.length ();
-         output.safe_push (new bit_test_cluster (clusters, start, end - 1,
-                                                 entire));
+         output.safe_push (new bit_test_cluster (clusters, i, i + end - 1,
+                                                 i == 0 && end == l));
        }
       else
-       for (int i = end - 1; i >= start; i--)
+       {
          output.safe_push (clusters[i]);
-
-      end = start;
-
-      if (start <= 0)
-       break;
+         /* ??? Might be able to skip more.  */
+         end = 1;
+       }
     }
 
-  output.reverse ();
   return output;
 }