2016-05-31 12:25 GMT+03:00 Richard Biener <richard.guent...@gmail.com>:
> On Tue, May 31, 2016 at 10:22 AM, Eric Botcazou <ebotca...@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.
This code appears when we try to disable boolean patterns. Boolean patterns
replace booleans with integers of proper size which allow us to simply determine
vectype using get_vectype_for_scalar_type. With no such replacement we
can't determine vectype just out of a scalar type (there are multiple possible
options and get_vectype_for_scalar_type use would result in a lot of redundant
conversions). So we delay vectype computation for them and compute it basing on
vectypes computed for their arguments.
Surely any missing vectype for a statement due to this delay is a bug. I didn't
notice STMT_VINFO_LIVE_P should be checked in addition to STMT_VINFO_RELEVANT_P.
Thanks,
Ilya
>
> Richard.
>
>> Tested on x86_64-suse-linux, OK for mainline and 6 branch?
>>
>>
>> 2016-05-31 Eric Botcazou <ebotca...@adacore.com>
>>
>> * tree-vect-loop.c (vectorizable_reduction): Also return false if the
>> scalar type is a boolean type.
>>
>>
>> 2016-05-31 Eric Botcazou <ebotca...@adacore.com>
>>
>> * gnat.dg/opt56.ad[sb]: New test.
>>
>> --
>> Eric Botcazou
