[Bug tree-optimization/21029] [4.1 Regression] vrp miscompiles Ada front-end, drops loop exit test in well-defined wrap-around circumstances

["rakdver at atrey dot karlin dot mff dot cuni dot cz" <gcc-bugzilla@gcc.gnu.org>]

------- Additional Comments From rakdver at atrey dot karlin dot mff dot cuni dot cz  2005-04-30 07:54 -------
Subject: Re: [PR tree-optimization/21029, RFC] harmful chrec type conversions

Hello,

> Alexandre Oliva wrote:
> > 
> > This is not a final patch; if the idea is considered sound, I'd simply
> > remove the if and the then-dead remaining code in chrec_convert.
> 
> The code following the if is not dead, at least it still is used in
> some cases after hand inlining count_ev_in_wider_type.  I would
> propose this patch that is about the same as yours
> 
> > Index: gcc/ChangeLog
> > from  Alexandre Oliva  <aoliva@redhat.com>
> > 
> > 	PR tree-optimization/21029
> > 	* tree-chrec.c (chrec_convert): Handle same-precision integral
> > 	types that differ in signedness like widening conversions.
> > 
> 
> with some more changes as follow:
> 
> 	* tree-chrec.c (chrec_convert): Handle same-precision integral
> 	types that differ in signedness like widening conversions.
> 	Inline count_ev_in_wider_type.
> 	* tree-chrec.h (count_ev_in_wider_type): Remove declaration.
> 	* tree-scalar-evolution.c (count_ev_in_wider_type): Removed.
> 
> Zdenek, does this change look right to you?

no.  There is one major bug -- 

> !   if (!evolution_function_is_affine_p (chrec))
>       {
> !       switch (TREE_CODE (chrec))
> ! 	{
> ! 	case POLYNOMIAL_CHREC:
> ! 	  return build_polynomial_chrec (CHREC_VARIABLE (chrec),
> ! 					 chrec_convert (type,
> ! 							CHREC_LEFT (chrec)),
> ! 					 chrec_convert (type,
> ! 							CHREC_RIGHT (chrec)));
> ! 

this is completely bogus -- you will always convert the
POLYNOMIAL_CHREC with nonconstant step, regardless of the correctness.

There are also a few minor changes that would be recommendable for the
final patch:

1) The can_count_ev_in_wider_type should be renamed to more appropriate
   name, given that it is now used even for shortening the value.
2) This function should be extended to really handle such conversions
   efficiently. I.e. to always allow shortening to unsigned types,
   and to always allow shortening to arbitrary types with -fwrapv.
   Also, I am not quite sure at the moment that the current code of
   can_count_ev_in_wider_type really works if the required conversion
   is not extending, this should be checked.

Zdenek

> 
> Index: tree-chrec.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/tree-chrec.c,v
> retrieving revision 2.15
> diff -c -3 -p -r2.15 tree-chrec.c
> *** tree-chrec.c	21 Apr 2005 08:48:51 -0000	2.15
> --- tree-chrec.c	29 Apr 2005 19:31:51 -0000
> *************** tree 
> *** 1036,1085 ****
>   chrec_convert (tree type, 
>   	       tree chrec)
>   {
> !   tree ct;
> !   
>     if (automatically_generated_chrec_p (chrec))
>       return chrec;
> !   
>     ct = chrec_type (chrec);
>     if (ct == type)
>       return chrec;
>   
> !   if (TYPE_PRECISION (ct) < TYPE_PRECISION (type))
> !     return count_ev_in_wider_type (type, chrec);
> ! 
> !   switch (TREE_CODE (chrec))
>       {
> !     case POLYNOMIAL_CHREC:
> !       return build_polynomial_chrec (CHREC_VARIABLE (chrec),
> ! 				     chrec_convert (type,
> ! 						    CHREC_LEFT (chrec)),
> ! 				     chrec_convert (type,
> ! 						    CHREC_RIGHT (chrec)));
> ! 
> !     default:
> !       {
> ! 	tree res = fold_convert (type, chrec);
> ! 
> ! 	/* Don't propagate overflows.  */
> ! 	TREE_OVERFLOW (res) = 0;
> ! 	if (CONSTANT_CLASS_P (res))
> ! 	  TREE_CONSTANT_OVERFLOW (res) = 0;
> ! 
> ! 	/* But reject constants that don't fit in their type after conversion.
> ! 	   This can happen if TYPE_MIN_VALUE or TYPE_MAX_VALUE are not the
> ! 	   natural values associated with TYPE_PRECISION and TYPE_UNSIGNED,
> ! 	   and can cause problems later when computing niters of loops.  Note
> ! 	   that we don't do the check before converting because we don't want
> ! 	   to reject conversions of negative chrecs to unsigned types.  */
> ! 	if (TREE_CODE (res) == INTEGER_CST
> ! 	    && TREE_CODE (type) == INTEGER_TYPE
> ! 	    && !int_fits_type_p (res, type))
> ! 	  res = chrec_dont_know;
> ! 
> ! 	return res;
> !       }
>       }
>   }
>   
>   /* Returns the type of the chrec.  */
> --- 1036,1104 ----
>   chrec_convert (tree type, 
>   	       tree chrec)
>   {
> !   tree ct, base, step;
> !   struct loop *loop;
> ! 
>     if (automatically_generated_chrec_p (chrec))
>       return chrec;
> ! 
>     ct = chrec_type (chrec);
>     if (ct == type)
>       return chrec;
>   
> !   if (!evolution_function_is_affine_p (chrec))
>       {
> !       switch (TREE_CODE (chrec))
> ! 	{
> ! 	case POLYNOMIAL_CHREC:
> ! 	  return build_polynomial_chrec (CHREC_VARIABLE (chrec),
> ! 					 chrec_convert (type,
> ! 							CHREC_LEFT (chrec)),
> ! 					 chrec_convert (type,
> ! 							CHREC_RIGHT (chrec)));
> ! 
> ! 	default:
> ! 	  {
> ! 	    tree res = fold_convert (type, chrec);
> ! 
> ! 	    /* Don't propagate overflows.  */
> ! 	    TREE_OVERFLOW (res) = 0;
> ! 	    if (CONSTANT_CLASS_P (res))
> ! 	      TREE_CONSTANT_OVERFLOW (res) = 0;
> ! 
> ! 	    /* But reject constants that don't fit in their type after
> ! 	       conversion.  This can happen if TYPE_MIN_VALUE or
> ! 	       TYPE_MAX_VALUE are not the natural values associated
> ! 	       with TYPE_PRECISION and TYPE_UNSIGNED, and can cause
> ! 	       problems later when computing niters of loops.  Note
> ! 	       that we don't do the check before converting because we
> ! 	       don't want to reject conversions of negative chrecs to
> ! 	       unsigned types.  */
> ! 	    if (TREE_CODE (res) == INTEGER_CST
> ! 		&& TREE_CODE (type) == INTEGER_TYPE
> ! 		&& !int_fits_type_p (res, type))
> ! 	      res = chrec_dont_know;
> ! 
> ! 	    return res;
> ! 	  }
> ! 	}
>       }
> + 
> +   /* BASE and STEP are INTEGER_CSTs.  */
> +   base = CHREC_LEFT (chrec);
> +   step = CHREC_RIGHT (chrec);
> +   loop = current_loops->parray[CHREC_VARIABLE (chrec)];
> + 
> +   /* TODO -- if we knew the statement at that the conversion occurs,
> +      we could pass it to can_count_iv_in_wider_type and get a better
> +      result.  */
> +   step = can_count_iv_in_wider_type (loop, type, base, step, NULL_TREE);
> +   if (!step)
> +     return fold_convert (type, chrec);
> + 
> +   base = chrec_convert (type, base);
> + 
> +   return build_polynomial_chrec (CHREC_VARIABLE (chrec), base, step);
>   }
>   
>   /* Returns the type of the chrec.  */
> Index: tree-chrec.h
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/tree-chrec.h,v
> retrieving revision 2.8
> diff -c -3 -p -r2.8 tree-chrec.h
> *** tree-chrec.h	28 Apr 2005 05:38:34 -0000	2.8
> --- tree-chrec.h	29 Apr 2005 19:31:51 -0000
> *************** extern tree chrec_fold_plus (tree, tree,
> *** 68,74 ****
>   extern tree chrec_fold_minus (tree, tree, tree);
>   extern tree chrec_fold_multiply (tree, tree, tree);
>   extern tree chrec_convert (tree, tree);
> - extern tree count_ev_in_wider_type (tree, tree);
>   extern tree chrec_type (tree);
>   
>   /* Operations.  */
> --- 68,73 ----
> Index: tree-scalar-evolution.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/tree-scalar-evolution.c,v
> retrieving revision 2.21
> diff -c -3 -p -r2.21 tree-scalar-evolution.c
> *** tree-scalar-evolution.c	21 Apr 2005 08:48:53 -0000	2.21
> --- tree-scalar-evolution.c	29 Apr 2005 19:31:52 -0000
> *************** find_var_scev_info (tree var)
> *** 350,382 ****
>     return &res->chrec;
>   }
>   
> - /* Tries to express CHREC in wider type TYPE.  */
> - 
> - tree
> - count_ev_in_wider_type (tree type, tree chrec)
> - {
> -   tree base, step;
> -   struct loop *loop;
> - 
> -   if (!evolution_function_is_affine_p (chrec))
> -     return fold_convert (type, chrec);
> - 
> -   base = CHREC_LEFT (chrec);
> -   step = CHREC_RIGHT (chrec);
> -   loop = current_loops->parray[CHREC_VARIABLE (chrec)];
> - 
> -   /* TODO -- if we knew the statement at that the conversion occurs,
> -      we could pass it to can_count_iv_in_wider_type and get a better
> -      result.  */
> -   step = can_count_iv_in_wider_type (loop, type, base, step, NULL_TREE);
> -   if (!step)
> -     return fold_convert (type, chrec);
> -   base = chrec_convert (type, base);
> - 
> -   return build_polynomial_chrec (CHREC_VARIABLE (chrec),
> - 				 base, step);
> - }
> - 
>   /* Return true when CHREC contains symbolic names defined in
>      LOOP_NB.  */
>   
> --- 350,355 ----


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21029