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 <ja...@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
