<[email protected]> 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 <[email protected]>
> Kwok Cheung Yeung <[email protected]>
> Jan Hubicka <[email protected]>
> Martin Jambor <[email protected]>
>
> 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 <[email protected]>
* 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
