[PATCH] Fix various issues in vect_analyze_data_refs (PR middle-end/59150)

[Richard Biener]

On Thu, 6 Feb 2014, Jakub Jelinek wrote:

> Hi!
> 
> This patch fixes a double free on simd-lane access if
> get_vectype_for_scalar_type fails (the new dr is already in datarefs[i]
> and if we free_data_ref it, it will be free_data_ref'ed again at the end of
> the failed vectorization), fixes handling of clobbers (plugs a memory leak
> due to missed free_data_ref and more importantly, if we goto again, dr
> would be kept at the old dr and thus we'd basically remove all the remaining
> drs in a loop) and plugs a memory leak with simd_lane_access.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

> PS, just looking at the patch now again, in the light of PR59594
> the reordering of datarefs before goto again looks also wrong, we IMHO have
> to perform a vector ordered remove in that case, ok to handle it as a
> follow-up?

Yeah...

Thanks,
Richard.

> 2014-02-06  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/59150
> 	* tree-vect-data-refs.c (vect_analyze_data_refs): For clobbers, call
> 	free_data_ref on the dr first, and before goto again also set dr
> 	to the next dr.  For simd_lane_access, free old datarefs[i] before
> 	overwriting it.  For get_vectype_for_scalar_type failure, don't
> 	free_data_ref if simd_lane_access.
> 
> --- gcc/tree-vect-data-refs.c.jj	2014-02-05 15:28:10.000000000 +0100
> +++ gcc/tree-vect-data-refs.c	2014-02-05 18:30:53.323659288 +0100
> @@ -3297,12 +3297,13 @@ again:
>           clobber stmts during vectorization.  */
>        if (gimple_clobber_p (stmt))
>  	{
> +	  free_data_ref (dr);
>  	  if (i == datarefs.length () - 1)
>  	    {
>  	      datarefs.pop ();
>  	      break;
>  	    }
> -	  datarefs[i] = datarefs.pop ();
> +	  datarefs[i] = dr = datarefs.pop ();
>  	  goto again;
>  	}
>  
> @@ -3643,13 +3644,14 @@ again:
>        if (simd_lane_access)
>  	{
>  	  STMT_VINFO_SIMD_LANE_ACCESS_P (stmt_info) = true;
> +	  free_data_ref (datarefs[i]);
>  	  datarefs[i] = dr;
>  	}
>  
>        /* Set vectype for STMT.  */
>        scalar_type = TREE_TYPE (DR_REF (dr));
> -      STMT_VINFO_VECTYPE (stmt_info) =
> -                get_vectype_for_scalar_type (scalar_type);
> +      STMT_VINFO_VECTYPE (stmt_info)
> +	= get_vectype_for_scalar_type (scalar_type);
>        if (!STMT_VINFO_VECTYPE (stmt_info))
>          {
>            if (dump_enabled_p ())
> @@ -3669,7 +3671,8 @@ again:
>  	  if (gather || simd_lane_access)
>  	    {
>  	      STMT_VINFO_DATA_REF (stmt_info) = NULL;
> -	      free_data_ref (dr);
> +	      if (gather)
> +		free_data_ref (dr);
>  	    }
>  	  return false;
>          }
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer