On Thu, 29 Aug 2019, Jakub Jelinek wrote:
> Hi!
>
> switchconv uses unsigned_type_for to get unsigned type to perform
> computations in, which is fine if you just do a comparison in that type or
> similar, but not when actually constructing range-like checks or doing
> further arithmetics on the type.
> The reassoc and fold-const range test optimization has range_check_type
> function for that, this patch makes use of that function, which among other
> things uses an INTEGER_TYPE instead of enums/booleans and verifies the
> INTEGER_TYPE minimum/maximum to make sure it properly wraps around.
> One issue is that this function can return NULL on some weird integral
> types (perhaps Ada special integers), so we need to punt in that case.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2019-08-29 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/91351
> * tree-cfg.c (generate_range_test): Use range_check_type instead of
> unsigned_type_for.
> * tree-cfgcleanup.c (convert_single_case_switch): Punt if
> range_check_type returns NULL.
> * tree-switch-conversion.c (switch_conversion::build_one_array):
> Use range_check_type instead of unsigned_type_for, don't perform
> linear opt if it returns NULL.
> (bit_test_cluster::find_bit_tests): Formatting fix.
> (bit_test_cluster::emit): Use range_check_type instead of
> unsigned_type_for.
> (switch_decision_tree::try_switch_expansion): Punt if range_check_type
> returns NULL.
>
> * g++.dg/opt/pr91351.C: New test.
>
> --- gcc/tree-cfg.c.jj 2019-07-29 12:56:38.971248016 +0200
> +++ gcc/tree-cfg.c 2019-08-28 19:37:50.843262628 +0200
> @@ -9221,7 +9221,7 @@ generate_range_test (basic_block bb, tre
> tree *lhs, tree *rhs)
> {
> tree type = TREE_TYPE (index);
> - tree utype = unsigned_type_for (type);
> + tree utype = range_check_type (type);
>
> low = fold_convert (utype, low);
> high = fold_convert (utype, high);
> --- gcc/tree-cfgcleanup.c.jj 2019-04-24 10:10:22.816535073 +0200
> +++ gcc/tree-cfgcleanup.c 2019-08-28 19:39:40.032680646 +0200
> @@ -101,6 +101,8 @@ convert_single_case_switch (gswitch *swt
> if (high)
> {
> tree lhs, rhs;
> + if (range_check_type (TREE_TYPE (index)) == NULL_TREE)
> + return false;
> generate_range_test (bb, index, low, high, &lhs, &rhs);
> cond = gimple_build_cond (LE_EXPR, lhs, rhs, NULL_TREE, NULL_TREE);
> }
> --- gcc/tree-switch-conversion.c.jj 2019-07-10 15:53:01.148520370 +0200
> +++ gcc/tree-switch-conversion.c 2019-08-28 19:45:18.062783134 +0200
> @@ -605,7 +605,9 @@ switch_conversion::build_one_array (int
> vec<constructor_elt, va_gc> *constructor = m_constructors[num];
> wide_int coeff_a, coeff_b;
> bool linear_p = contains_linear_function_p (constructor, &coeff_a,
> &coeff_b);
> - if (linear_p)
> + tree type;
> + if (linear_p
> + && (type = range_check_type (TREE_TYPE ((*constructor)[0].value))))
> {
> if (dump_file && coeff_a.to_uhwi () > 0)
> fprintf (dump_file, "Linear transformation with A = %" PRId64
> @@ -613,13 +615,12 @@ switch_conversion::build_one_array (int
> coeff_b.to_shwi ());
>
> /* We must use type of constructor values. */
> - tree t = unsigned_type_for (TREE_TYPE ((*constructor)[0].value));
> gimple_seq seq = NULL;
> - tree tmp = gimple_convert (&seq, t, m_index_expr);
> - tree tmp2 = gimple_build (&seq, MULT_EXPR, t,
> - wide_int_to_tree (t, coeff_a), tmp);
> - tree tmp3 = gimple_build (&seq, PLUS_EXPR, t, tmp2,
> - wide_int_to_tree (t, coeff_b));
> + tree tmp = gimple_convert (&seq, type, m_index_expr);
> + tree tmp2 = gimple_build (&seq, MULT_EXPR, type,
> + wide_int_to_tree (type, coeff_a), tmp);
> + tree tmp3 = gimple_build (&seq, PLUS_EXPR, type, tmp2,
> + wide_int_to_tree (type, coeff_b));
> tree tmp4 = gimple_convert (&seq, TREE_TYPE (name), tmp3);
> gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
> load = gimple_build_assign (name, tmp4);
> @@ -1351,7 +1352,7 @@ bit_test_cluster::find_bit_tests (vec<cl
> entire));
> }
> else
> - for (int i = end - 1; i >= start; i--)
> + for (int i = end - 1; i >= start; i--)
> output.safe_push (clusters[i]);
>
> end = start;
> @@ -1484,7 +1485,7 @@ bit_test_cluster::emit (tree index_expr,
> unsigned int i, j, k;
> unsigned int count;
>
> - tree unsigned_index_type = unsigned_type_for (index_type);
> + tree unsigned_index_type = range_check_type (index_type);
>
> gimple_stmt_iterator gsi;
> gassign *shift_stmt;
> @@ -1794,7 +1795,8 @@ switch_decision_tree::try_switch_expansi
> tree index_type = TREE_TYPE (index_expr);
> basic_block bb = gimple_bb (m_switch);
>
> - if (gimple_switch_num_labels (m_switch) == 1)
> + if (gimple_switch_num_labels (m_switch) == 1
> + || range_check_type (index_type) == NULL_TREE)
> return false;
>
> /* Find the default case target label. */
> --- gcc/testsuite/g++.dg/opt/pr91351.C.jj 2019-08-28 19:53:50.946352281
> +0200
> +++ gcc/testsuite/g++.dg/opt/pr91351.C 2019-08-28 19:53:30.277651740
> +0200
> @@ -0,0 +1,38 @@
> +// PR tree-optimization/91351
> +// { dg-do run }
> +// { dg-options "-O2 -fstrict-enums" }
> +
> +enum E { e0, e1, e2, e3, e4, e5, e6, e7, e8, e9, e10, e11, e12,
> + e13, e14, e15, e16, e17, e18, e19, e20, e21, e22, e23, e24, e25 };
> +
> +__attribute__((noipa)) void
> +foo ()
> +{
> + __builtin_abort ();
> +}
> +
> +__attribute__((noipa)) void
> +bar ()
> +{
> +}
> +
> +__attribute__((noipa)) void
> +baz (E e)
> +{
> + switch (e)
> + {
> + case e11:
> + case e12:
> + case e13: foo (); break;
> + case e24: break;
> + case e14:
> + case e15: break;
> + default: bar (); break;
> + }
> +}
> +
> +int
> +main ()
> +{
> + baz (e3);
> +}
>
> Jakub
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 247165 (AG München)
