On Tue, 2 Feb 2021, Jakub Jelinek wrote:
> Hi!
>
> The following testcase ICEs, because after the veclower pass which is the
> last point which lowers unsupported vector operations to supported ones
> (or scalars) match.pd simplifies a supported vector operation into
> unsupported one (vec << 1 >> 1 into vec & { ~1, ... }).
> Guarding each match.pd pattern that could do it manually would be IMHO a
> nightmare and it could affect hundreds of them, so this patch instead
> performs the verification and punting in the infrastructure (of course only
> in PROP_gimple_lvec state which is set after the vector lowering).
> The function attempts to match expand_vector_operations_1 (except I haven't
> added VEC_PERM_EXPR, VEC_COND_EXPR and COND_EXPR cases there, so those
> aren't checked and can be added later if we find it a problem).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
So I fear this only covers parts of the paths simplifications can
end up used. Now one question is whether we want to allow
"invalid" intermediate transforms if a final match makes it
"valid" again. If not then doing the verification in
gimple-match.c itself might be preferable. OTOH I think that
most patterns should verify themselves - it's also odd that
v << 1 >> 1 is supported but not v & vec-cst - we might want to
require some basic support from targets. I realize the PR is
one of the odd V1mode cases which we might want to support better
during RTL expansion (or require targets to be more sane here).
So in the end I don't like the patch much, not at this stage
anyway.
Thanks,
Richard.
> 2021-02-02 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/98287
> * gimple-match.h (check_gimple_lvec): New declaration.
> * gimple-match-head.c (check_gimple_lvec): New function.
> (maybe_push_res_to_seq): Use it.
> * gimple-fold.c: Include tree-pass.h.
> (replace_stmt_with_simplification): Use it.
>
> * gcc.dg/pr98287.c: New test.
>
> --- gcc/gimple-match.h.jj 2021-01-04 10:25:38.456238093 +0100
> +++ gcc/gimple-match.h 2021-02-01 13:19:30.393860766 +0100
> @@ -335,6 +335,7 @@ extern tree (*mprts_hook) (gimple_match_
>
> bool gimple_simplify (gimple *, gimple_match_op *, gimple_seq *,
> tree (*)(tree), tree (*)(tree));
> +bool check_gimple_lvec (gimple_match_op *);
> tree maybe_push_res_to_seq (gimple_match_op *, gimple_seq *,
> tree res = NULL_TREE);
> void maybe_build_generic_op (gimple_match_op *);
> --- gcc/gimple-match-head.c.jj 2021-01-16 19:46:53.511672837 +0100
> +++ gcc/gimple-match-head.c 2021-02-01 13:18:58.676225427 +0100
> @@ -549,6 +549,109 @@ build_call_internal (internal_fn fn, gim
> res_op->op_or_null (4));
> }
>
> +/* In PROP_gimple_lvec mode, perform extra checking if RES_OP and return
> + true if RES_OP is not appropriate - would require vector operations that
> + would need to be lowered before expansion. */
> +
> +bool
> +check_gimple_lvec (gimple_match_op *res_op)
> +{
> + enum tree_code code = res_op->code;
> + enum gimple_rhs_class rhs_class = get_gimple_rhs_class (code);
> + if (rhs_class != GIMPLE_UNARY_RHS && rhs_class != GIMPLE_BINARY_RHS)
> + return false;
> +
> + tree rhs1 = res_op->op_or_null (0);
> + tree rhs2 = res_op->op_or_null (1);
> + tree type = res_op->type;
> +
> + if (!VECTOR_TYPE_P (type) || !VECTOR_TYPE_P (TREE_TYPE (rhs1)))
> + return false;
> +
> + /* A scalar operation pretending to be a vector one. */
> + if (VECTOR_BOOLEAN_TYPE_P (type)
> + && !VECTOR_MODE_P (TYPE_MODE (type))
> + && TYPE_MODE (type) != BLKmode
> + && (TREE_CODE_CLASS (code) != tcc_comparison
> + || (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (rhs1))
> + && !VECTOR_MODE_P (TYPE_MODE (TREE_TYPE (rhs1)))
> + && TYPE_MODE (TREE_TYPE (rhs1)) != BLKmode)))
> + return false;
> +
> + machine_mode mode = TYPE_MODE (type);
> + optab op;
> + switch (code)
> + {
> + CASE_CONVERT:
> + case FLOAT_EXPR:
> + case FIX_TRUNC_EXPR:
> + case VIEW_CONVERT_EXPR:
> + return false;
> +
> + case VEC_UNPACK_FLOAT_HI_EXPR:
> + case VEC_UNPACK_FLOAT_LO_EXPR:
> + case VEC_PACK_FLOAT_EXPR:
> + return false;
> +
> + case WIDEN_SUM_EXPR:
> + case VEC_WIDEN_PLUS_HI_EXPR:
> + case VEC_WIDEN_PLUS_LO_EXPR:
> + case VEC_WIDEN_MINUS_HI_EXPR:
> + case VEC_WIDEN_MINUS_LO_EXPR:
> + case VEC_WIDEN_MULT_HI_EXPR:
> + case VEC_WIDEN_MULT_LO_EXPR:
> + case VEC_WIDEN_MULT_EVEN_EXPR:
> + case VEC_WIDEN_MULT_ODD_EXPR:
> + case VEC_UNPACK_HI_EXPR:
> + case VEC_UNPACK_LO_EXPR:
> + case VEC_UNPACK_FIX_TRUNC_HI_EXPR:
> + case VEC_UNPACK_FIX_TRUNC_LO_EXPR:
> + case VEC_PACK_TRUNC_EXPR:
> + case VEC_PACK_SAT_EXPR:
> + case VEC_PACK_FIX_TRUNC_EXPR:
> + case VEC_WIDEN_LSHIFT_HI_EXPR:
> + case VEC_WIDEN_LSHIFT_LO_EXPR:
> + return false;
> +
> + case LSHIFT_EXPR:
> + case RSHIFT_EXPR:
> + case LROTATE_EXPR:
> + case RROTATE_EXPR:
> + if (!VECTOR_MODE_P (mode))
> + return true;
> + if (!VECTOR_INTEGER_TYPE_P (TREE_TYPE (rhs2)))
> + {
> + op = optab_for_tree_code (code, type, optab_scalar);
> + if (op && optab_handler (op, mode) != CODE_FOR_nothing)
> + return false;
> + }
> + op = optab_for_tree_code (code, type, optab_vector);
> + if (op && optab_handler (op, mode) != CODE_FOR_nothing)
> + return false;
> + return true;
> +
> + case MULT_HIGHPART_EXPR:
> + if (!VECTOR_MODE_P (mode)
> + || !can_mult_highpart_p (mode, TYPE_UNSIGNED (type)))
> + return true;
> + return false;
> +
> + default:
> + if (!VECTOR_MODE_P (mode))
> + return true;
> + op = optab_for_tree_code (code, type, optab_default);
> + if (op == unknown_optab
> + && code == NEGATE_EXPR
> + && INTEGRAL_TYPE_P (TREE_TYPE (type)))
> + op = optab_for_tree_code (MINUS_EXPR, type, optab_default);
> + if (op && optab_handler (op, mode) != CODE_FOR_nothing)
> + return false;
> + return true;
> + }
> +
> + return false;
> +}
> +
> /* Push the exploded expression described by RES_OP as a statement to
> SEQ if necessary and return a gimple value denoting the value of the
> expression. If RES is not NULL then the result will be always RES
> @@ -597,6 +700,12 @@ maybe_push_res_to_seq (gimple_match_op *
>
> if (res_op->code.is_tree_code ())
> {
> + if (VECTOR_TYPE_P (res_op->type)
> + && cfun
> + && (cfun->curr_properties & PROP_gimple_lvec) != 0
> + && check_gimple_lvec (res_op))
> + return NULL_TREE;
> +
> if (!res)
> {
> if (gimple_in_ssa_p (cfun))
> --- gcc/gimple-fold.c.jj 2021-01-22 11:41:56.510499898 +0100
> +++ gcc/gimple-fold.c 2021-02-01 13:22:11.992002840 +0100
> @@ -66,6 +66,7 @@ along with GCC; see the file COPYING3.
> #include "tree-vector-builder.h"
> #include "tree-ssa-strlen.h"
> #include "varasm.h"
> +#include "tree-pass.h"
>
> enum strlen_range_kind {
> /* Compute the exact constant string length. */
> @@ -5678,6 +5679,11 @@ replace_stmt_with_simplification (gimple
> if (!inplace
> || gimple_num_ops (stmt) > get_gimple_rhs_num_ops (res_op->code))
> {
> + if (VECTOR_TYPE_P (res_op->type)
> + && cfun
> + && (cfun->curr_properties & PROP_gimple_lvec) != 0
> + && check_gimple_lvec (res_op))
> + return false;
> maybe_build_generic_op (res_op);
> gimple_assign_set_rhs_with_ops (gsi, res_op->code,
> res_op->op_or_null (0),
> --- gcc/testsuite/gcc.dg/pr98287.c.jj 2021-02-01 13:32:22.735981006 +0100
> +++ gcc/testsuite/gcc.dg/pr98287.c 2021-02-01 13:32:01.155229122 +0100
> @@ -0,0 +1,19 @@
> +/* PR tree-optimization/98287 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-ccp -fno-tree-forwprop -Wno-psabi -w" } */
> +
> +typedef unsigned long __attribute__((__vector_size__ (8))) V;
> +V v;
> +
> +static __attribute__((noinline, noclone)) V
> +bar (unsigned short s)
> +{
> + return v >> s << s | v >> s >> 63;
> +}
> +
> +unsigned long
> +foo (void)
> +{
> + V x = bar (1);
> + return x[0];
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
