Re: [PATCH] Fix up switchconv for strict enum types (PR tree-optimization/91351)

[Richard Biener <rguenther at suse dot de>]

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  <jakub@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 <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 247165 (AG München)