<a...@codesourcery.com> writes:
> GCN vector sizes range between 64 and 512 bytes, none of which have
> correspondingly sized integer modes.  This breaks a number of assumptions
> throughout the compiler, but I don't really want to create modes just for this
> purpose.
>
> Instead, this patch fixes up the cases that I've found, so far, such that the
> compiler tries something else, or fails to optimize, rather than just ICE.
>
> 2018-09-05  Andrew Stubbs  <a...@codesourcery.com>
>             Kwok Cheung Yeung  <k...@codesourcery.com>
>           Jan Hubicka  <j...@suse.cz>
>           Martin Jambor  <mjam...@suse.cz>
>
>       gcc/
>       * combine.c (gen_lowpart_or_truncate): Return clobber if there is
>       not a integer mode if the same size as x.
>       (gen_lowpart_for_combine): Fail if there is no integer mode of the
>       same size.
>       * expr.c (expand_expr_real_1): Force first operand to be in memory
>       if it is a vector register and the result is in BLKmode.
>       * tree-vect-stmts.c (vectorizable_store): Don't ICE when
>       int_mode_for_size fails.
>       (vectorizable_load): Likewise.
> ---
>  gcc/combine.c         | 13 ++++++++++++-
>  gcc/expr.c            |  8 ++++++++
>  gcc/tree-vect-stmts.c |  8 ++++----
>  3 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/combine.c b/gcc/combine.c
> index a2649b6..cbf9dae 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -8621,7 +8621,13 @@ gen_lowpart_or_truncate (machine_mode mode, rtx x)
>      {
>        /* Bit-cast X into an integer mode.  */
>        if (!SCALAR_INT_MODE_P (GET_MODE (x)))
> -     x = gen_lowpart (int_mode_for_mode (GET_MODE (x)).require (), x);
> +     {
> +       enum machine_mode imode =
> +         int_mode_for_mode (GET_MODE (x)).require ();
> +       if (imode == BLKmode)
> +         return gen_rtx_CLOBBER (mode, const0_rtx);
> +       x = gen_lowpart (imode, x);

require () will ICE if there isn't an integer mode and always returns
a scalar_int_mode, so this looks like a no-op.  I think you want
something like:

    scalar_int_mode imode;
    if (!int_mode_for_mode (GET_MODE (x)).exists (&imode))
      ...

> @@ -11698,6 +11704,11 @@ gen_lowpart_for_combine (machine_mode omode, rtx x)
>    if (omode == imode)
>      return x;
>  
> +  /* This can happen when there is no integer mode corresponding
> +     to a size of vector mode.  */
> +  if (omode == BLKmode)
> +    goto fail;
> +
>    /* We can only support MODE being wider than a word if X is a
>       constant integer or has a mode the same size.  */
>    if (maybe_gt (GET_MODE_SIZE (omode), UNITS_PER_WORD)

This seems like it's working around a bug in ther caller.

> diff --git a/gcc/expr.c b/gcc/expr.c
> index cd5cf12..776254a 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -10569,6 +10569,14 @@ expand_expr_real_1 (tree exp, rtx target, 
> machine_mode tmode,
>                         || maybe_gt (bitpos + bitsize,
>                                      GET_MODE_BITSIZE (mode2)));
>  
> +     /* If the result is in BLKmode and the underlying object is a
> +        vector in a register, and the size of the vector is larger than
> +        the largest integer mode, then we must force OP0 to be in memory
> +        as this is assumed in later code.  */
> +     if (REG_P (op0) && VECTOR_MODE_P (mode2) && mode == BLKmode
> +         && maybe_gt (bitsize, MAX_FIXED_MODE_SIZE))
> +       must_force_mem = 1;
> +
>       /* Handle CONCAT first.  */
>       if (GET_CODE (op0) == CONCAT && !must_force_mem)
>         {

Are you sure this is still needed after:

2018-06-04  Richard Sandiford  <richard.sandif...@linaro.org>

        * expr.c (expand_expr_real_1): Force the operand into memory if
        its TYPE_MODE is BLKmode and if there is no integer mode for
        the number of bits being extracted.

If so, what case is it handling differently?

> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> index 8d94fca..607a2bd 100644
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -6702,12 +6702,12 @@ vectorizable_store (stmt_vec_info stmt_info, 
> gimple_stmt_iterator *gsi,
>                    supported.  */
>                 unsigned lsize
>                   = group_size * GET_MODE_BITSIZE (elmode);
> -               elmode = int_mode_for_size (lsize, 0).require ();
>                 unsigned int lnunits = const_nunits / group_size;
>                 /* If we can't construct such a vector fall back to
>                    element extracts from the original vector type and
>                    element size stores.  */
> -               if (mode_for_vector (elmode, lnunits).exists (&vmode)
> +               if (int_mode_for_size (lsize, 0).exists (&elmode)
> +                   && mode_for_vector (elmode, lnunits).exists (&vmode)
>                     && VECTOR_MODE_P (vmode)
>                     && targetm.vector_mode_supported_p (vmode)
>                     && (convert_optab_handler (vec_extract_optab,
> @@ -7839,11 +7839,11 @@ vectorizable_load (stmt_vec_info stmt_info, 
> gimple_stmt_iterator *gsi,
>                    to a larger load.  */
>                 unsigned lsize
>                   = group_size * TYPE_PRECISION (TREE_TYPE (vectype));
> -               elmode = int_mode_for_size (lsize, 0).require ();
>                 unsigned int lnunits = const_nunits / group_size;
>                 /* If we can't construct such a vector fall back to
>                    element loads of the original vector type.  */
> -               if (mode_for_vector (elmode, lnunits).exists (&vmode)
> +               if (int_mode_for_size (lsize, 0).exists (&elmode)
> +                   && mode_for_vector (elmode, lnunits).exists (&vmode)
>                     && VECTOR_MODE_P (vmode)
>                     && targetm.vector_mode_supported_p (vmode)
>                     && (convert_optab_handler (vec_init_optab, vmode, elmode)

These two are OK independently of the rest (if that's convenient).

Thanks,
Richard

Reply via email to