Re: [PATCH] openmp-simd-clone: Match shift type

[Jakub Jelinek <jakub@redhat.com>]

On Fri, Jul 29, 2022 at 06:03:18PM +0100, Andrew Stubbs wrote:
> On 29/07/2022 16:59, Jakub Jelinek wrote:
> > Doing the fold_convert seems to be a wasted effort to me.
> > Can't this be done conditional on whether some change is needed at all
> > and just using gimple_build_assign with NOP_EXPR, so something like:
> 
> I'm just not familiar enough with this stuff to run fold_convert in my head
> with confidence.

The thing with fold_convert is that if some conversion is needed (and
fold_convert actually is strict, so even if the conversion is useless
but the type isn't exactly the same) it creates a NOP_EXPR around the
argument, and then gimple_build_assign notices it should create a NOP_EXPR
assign rhs op and just uses the argument of NOP_EXPR, where the NOP_EXPR
will be GC later.
Plus, if the conversion isn't needed, it creates an extra assignment that
will be only later in some other pass optimized away.
> 
> > 	  tree shift_cvt_conv = shift_cnt;
> > 	  if (!useless_type_conversion_p (TREE_TYPE (mask),
> > 					  TREE_TYPE (shift_cnt)))
> > 	    {
> > 	      shift_cnt_conv = make_ssa_name (TREE_TYPE (mask));
> > 	      g = gimple_build_assign (shift_cnt_conv, NOP_EXPR, shift_cnt);
> > 	      gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING);
> > 	    }
> > 
> 
> Your version gives the same output mine does, at least on amdgcn anyway.
> 
> Am I OK to commit this version?

Yes, thanks.

> openmp-simd-clone: Match shift types
> 
> Ensure that both parameters to vector shifts use the same mode.  This is most
> important for amdgcn where the masks are DImode.
> 
> gcc/ChangeLog:
> 
> 	* omp-simd-clone.cc (simd_clone_adjust): Convert shift_cnt to match
> 	the mask type.
> 
> Co-authored-by: Jakub Jelinek  <jakub@redhat.com>
> 
> diff --git a/gcc/omp-simd-clone.cc b/gcc/omp-simd-clone.cc
> index 32649bc3f9a..58bd68b129b 100644
> --- a/gcc/omp-simd-clone.cc
> +++ b/gcc/omp-simd-clone.cc
> @@ -1305,8 +1305,16 @@ simd_clone_adjust (struct cgraph_node *node)
>  				       build_int_cst (TREE_TYPE (iter1), c));
>  	      gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING);
>  	    }
> +	  tree shift_cnt_conv = shift_cnt;
> +	  if (!useless_type_conversion_p (TREE_TYPE (mask),
> +					  TREE_TYPE (shift_cnt)))
> +	    {
> +	      shift_cnt_conv = make_ssa_name (TREE_TYPE (mask));
> +	      g = gimple_build_assign (shift_cnt_conv, NOP_EXPR, shift_cnt);
> +	      gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING);
> +	    }
>  	  g = gimple_build_assign (make_ssa_name (TREE_TYPE (mask)),
> -				   RSHIFT_EXPR, mask, shift_cnt);
> +				   RSHIFT_EXPR, mask, shift_cnt_conv);
>  	  gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING);
>  	  mask = gimple_assign_lhs (g);
>  	  g = gimple_build_assign (make_ssa_name (TREE_TYPE (mask)),


	Jakub