On Fri, Jul 10, 2020 at 9:50 AM Martin Liška <mli...@suse.cz> wrote:
>
> As mentioned in the PR, we need to clean up orphan vector comparisons
> that tend to happen be gimplification of VEC_COND_EXPR.
>
> I've done that easily in expand_vector_comparison where I add these
> to a bitmap used in simple DCE.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
I don't like this much - the reason for the dead code is that the gimplifier,
while it manages to DCE the VEC_COND_EXPR because the value is not
needed, does not apply the same for the operands where only side-effects
would need to be kept.
But then if the vector comparisons would not be dead, the testcase
would still ICE even with your patch. And the reason for the ICE is
that vector lowering checks
if (!expand_vec_cmp_expr_p (TREE_TYPE (op0), type, code)
&& !expand_vec_cond_expr_p (type, TREE_TYPE (op0), code))
{
while RTL expansion has
/* For vector typed comparisons emit code to generate the desired
all-ones or all-zeros mask. */
if (TREE_CODE (ops->type) == VECTOR_TYPE)
{
tree ifexp = build2 (ops->code, ops->type, arg0, arg1);
if (VECTOR_BOOLEAN_TYPE_P (ops->type)
&& expand_vec_cmp_expr_p (TREE_TYPE (arg0), ops->type, ops->code))
return expand_vec_cmp_expr (ops->type, ifexp, target);
else
gcc_unreachable ();
so vector lowering doesn't lower vector comparisons when we can expand
via a VEC_COND_EXPR but the RTL expansion code asserts this will not
be needed.
Thus this should either be fixed by re-instantiating the RTL expansion code
or handling vector comparisons in gimple-isel.cc at least when they need
to be expanded as VEC_COND_EXPR.
Richard.
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> PR tree-optimization/96128
> * tree-vect-generic.c (expand_vector_comparison): Remove vector
> comparisons that don't have a usage.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/96128
> * gcc.target/s390/vector/pr96128.c: New test.
> ---
> .../gcc.target/s390/vector/pr96128.c | 35 +++++++++++++++++++
> gcc/tree-vect-generic.c | 4 ++-
> 2 files changed, 38 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/s390/vector/pr96128.c
>
> diff --git a/gcc/testsuite/gcc.target/s390/vector/pr96128.c
> b/gcc/testsuite/gcc.target/s390/vector/pr96128.c
> new file mode 100644
> index 00000000000..20abe5e515c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/vector/pr96128.c
> @@ -0,0 +1,35 @@
> +/* PR tree-optimization/96128 */
> +/* { dg-options "-march=z13" } */
> +
> +#define B_TEST(TYPE) { TYPE v __attribute__((vector_size(16))); (void)((v <
> v) < v); }
> +#ifdef __cplusplus
> +#define T_TEST(TYPE) { TYPE s; TYPE v __attribute__((vector_size(16)));
> __typeof((v<v)[0]) iv __attribute__((vector_size(16))); (void)((iv ? s : s) <
> v); }
> +#else
> +#define T_TEST(TYPE)
> +#endif
> +#define T(TYPE) B_TEST(TYPE) T_TEST(TYPE)
> +#ifdef __SIZEOF_INT128__
> +#define SIZEOF_MAXINT __SIZEOF_INT128__
> +#else
> +#define SIZEOF_MAXINT __SIZEOF_LONG_LONG__
> +#endif
> +
> +void f ()
> +{
> + T(short)
> + T(int)
> + T(long)
> + T(long long)
> +
> + T_TEST(float)
> + T_TEST(double)
> + /* Avoid trouble with non-power-of-two sizes.
> + Also avoid trouble with long double larger than integral types. */
> +#if !defined(__i386__) && !defined(__x86_64__) && !defined(__m68k__) \
> + && !defined(__ia64__) && !defined(__hppa__) \
> + && (__SIZEOF_LONG_DOUBLE__ & (__SIZEOF_LONG_DOUBLE__ - 1)) == 0 \
> + && __SIZEOF_LONG_DOUBLE__ <= 16 \
> + && __SIZEOF_LONG_DOUBLE__ <= SIZEOF_MAXINT
> + T_TEST(long double)
> +#endif
> +}
> diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c
> index f8bd26f2156..636104dea13 100644
> --- a/gcc/tree-vect-generic.c
> +++ b/gcc/tree-vect-generic.c
> @@ -415,7 +415,9 @@ expand_vector_comparison (gimple_stmt_iterator *gsi, tree
> type, tree op0,
> }
> }
>
> - if (!uses.is_empty () && vec_cond_expr_only)
> + if (uses.is_empty ())
> + bitmap_set_bit (dce_ssa_names, SSA_NAME_VERSION (lhs));
> + else if (vec_cond_expr_only)
> return NULL_TREE;
>
> tree t;
> --
> 2.27.0
>
