Re: [PATCH] Restrict jump threading statement simplifier to scalar types (PR71077)

[Richard Biener <richard dot guenther at gmail dot com>]

On Sat, Aug 20, 2016 at 1:25 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Fri, 19 Aug 2016, Yuri Rumyantsev wrote:
>
>> Hi,
>>
>> Here is a simple test-case to reproduce 176.gcc failure (I run it on
>> Haswell machine).
>> Using 20160819 compiler build we get:
>> gcc -O3 -m32 -mavx2 test.c -o test.ref.exe
>> /users/ysrumyan/isse_6866$ ./test.ref.exe
>> Aborted (core dumped)
>>
>> If I apply patch proposed by Patrick test runs properly
>> Instead of running we can check number of .jump thread.
>
> Thanks!  With this test case I was able to identify the cause of the
> wrong code generation.
>
> Turns out that the problem lies in fold-const.c.  It does not correctly
> fold VECTOR_CST comparisons that have a scalar boolean result type.  In
> particular fold_binary folds the comparison {1,1,1} != {0,0,0} to false
> which causes the threader to register an incorrect jump thread.  In
> general VEC1 != VEC2 gets folded as if it were VEC1 == VEC2.
>
> This patch fixes the problematic folding logic.
>
> Does this look OK to commit after bootstrap + regtesting?  The faulty
> logic was introduced by the fix for PR68542 (r232518) so I think it is
> present on the 6 branch as well.

The fold_relational_const changes are ok, please factor in changes to the
testcase / unit-testing as requested.  Ok also for the branch after it re-opens.

Thanks,
Richard.

> gcc/ChangeLog:
>
>         PR tree-optimization/71044
>         PR tree-optimization/68542
>         * fold-const.c (fold_relational_const): Fix folding of
>         VECTOR_CST comparisons that have a scalar boolean result type.
>         (test_vector_folding): New static function.
>         (fold_const_c_tests): Call it.
> ---
>  gcc/fold-const.c | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> index 30c1e0d..0271ac3 100644
> --- a/gcc/fold-const.c
> +++ b/gcc/fold-const.c
> @@ -13897,7 +13897,6 @@ fold_relational_const (enum tree_code code, tree type, tree op0, tree op1)
>        if (!VECTOR_TYPE_P (type))
>         {
>           /* Have vector comparison with scalar boolean result.  */
> -         bool result = true;
>           gcc_assert ((code == EQ_EXPR || code == NE_EXPR)
>                       && VECTOR_CST_NELTS (op0) == VECTOR_CST_NELTS (op1));
>           for (unsigned i = 0; i < VECTOR_CST_NELTS (op0); i++)
> @@ -13905,11 +13904,12 @@ fold_relational_const (enum tree_code code, tree type, tree op0, tree op1)
>               tree elem0 = VECTOR_CST_ELT (op0, i);
>               tree elem1 = VECTOR_CST_ELT (op1, i);
>               tree tmp = fold_relational_const (code, type, elem0, elem1);
> -             result &= integer_onep (tmp);
> +             if (tmp == NULL_TREE)
> +               return NULL_TREE;
> +             if (integer_zerop (tmp))
> +               return constant_boolean_node (false, type);
>             }
> -         if (code == NE_EXPR)
> -           result = !result;
> -         return constant_boolean_node (result, type);
> +         return constant_boolean_node (true, type);
>         }
>        unsigned count = VECTOR_CST_NELTS (op0);
>        tree *elts =  XALLOCAVEC (tree, count);
> @@ -14517,12 +14517,32 @@ test_arithmetic_folding ()
>                                    x);
>  }
>
> +/* Verify that various binary operations on vectors are folded
> +   correctly.  */
> +
> +static void
> +test_vector_folding ()
> +{
> +  tree inner_type = integer_type_node;
> +  tree type = build_vector_type (inner_type, 4);
> +  tree zero = build_zero_cst (type);
> +  tree one = build_one_cst (type);
> +
> +  /* Verify equality tests that return a scalar boolean result.  */
> +  tree res_type = boolean_type_node;
> +  ASSERT_FALSE (integer_nonzerop (fold_build2 (EQ_EXPR, res_type, zero, one)));
> +  ASSERT_TRUE (integer_nonzerop (fold_build2 (EQ_EXPR, res_type, zero, zero)));
> +  ASSERT_TRUE (integer_nonzerop (fold_build2 (NE_EXPR, res_type, zero, one)));
> +  ASSERT_FALSE (integer_nonzerop (fold_build2 (NE_EXPR, res_type, one, one)));
> +}
> +
>  /* Run all of the selftests within this file.  */
>
>  void
>  fold_const_c_tests ()
>  {
>    test_arithmetic_folding ();
> +  test_vector_folding ();
>  }
>
>  } // namespace selftest
> --
> 2.9.3.650.g20ba99f
>