Re: [patch] Fix segfault in vectorizer

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

On Tue, May 31, 2016 at 10:22 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> it's a regression present on the mainline and 6 branch: for the attached Ada
> testcase, optab_for_tree_code segfaults because the function is invoked on a
> NULL_TREE vectype_out from the vectorizer (vectorizable_reduction):
>
>   if (STMT_VINFO_VEC_REDUCTION_TYPE (stmt_info) == TREE_CODE_REDUCTION
>       || STMT_VINFO_VEC_REDUCTION_TYPE (stmt_info)
>                 == INTEGER_INDUC_COND_REDUCTION)
>     {
>       if (reduction_code_for_scalar_code (orig_code, &epilog_reduc_code))
>         {
>           reduc_optab = optab_for_tree_code (epilog_reduc_code, vectype_out,
>                                          optab_default);
>
> Naive attempts at working around the NULL_TREE result in bogus vectorization
> and GIMPLE verification failure so it seems clear that vectype_out ought not
> to be NULL_TREE here.
>
> The problems stems from vect_determine_vectorization_factor, namely:
>
>               /* Bool ops don't participate in vectorization factor
>                  computation.  For comparison use compared types to
>                  compute a factor.  */
>               if (TREE_CODE (scalar_type) == BOOLEAN_TYPE
>                   && is_gimple_assign (stmt)
>                   && gimple_assign_rhs_code (stmt) != COND_EXPR)
>                 {
>                   if (STMT_VINFO_RELEVANT_P (stmt_info))
>                     mask_producers.safe_push (stmt_info);
>                   bool_result = true;
>
>                   if (gimple_code (stmt) == GIMPLE_ASSIGN
>                       && TREE_CODE_CLASS (gimple_assign_rhs_code (stmt))
>                          == tcc_comparison
>                       && TREE_CODE (TREE_TYPE (gimple_assign_rhs1 (stmt)))
>                          != BOOLEAN_TYPE)
>                     scalar_type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>                   else
>                     {
>                      if (!analyze_pattern_stmt && gsi_end_p (pattern_def_si))
>                         {
>                           pattern_def_seq = NULL;
>                           gsi_next (&si);
>                         }
>                       continue;
>                     }
>                 }
>
> which leaves STMT_VINFO_VECTYPE set to NULL_TREE.  It would have been nice to
> say in the comment why boolean operations don't participate in vectorization
> factor computation; one can only assume that it's because they are somehow
> treated specially, so the proposed fix does the same in vectorizable_reduction
> (and is probably a no-op except for Ada because of the precision test).

Note that vect_determine_vectorization_factor is supposed to set the
vector type on all
stmts.  That it doesn't is a bug.  Do you run into the else branch?  I
think that should
only trigger with STMT_VINFO_RELEVANT_P and thus the stmt in mask_producers
which should later get post-processed and have the VECTYPE set.

But maybe Ilya can shed some more light on this (awkward) code.

Richard.

> Tested on x86_64-suse-linux, OK for mainline and 6 branch?
>
>
> 2016-05-31  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * tree-vect-loop.c (vectorizable_reduction): Also return false if the
>         scalar type is a boolean type.
>
>
> 2016-05-31  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gnat.dg/opt56.ad[sb]: New test.
>
> --
> Eric Botcazou