Re: [patch] Fix segfault in vectorizer
> 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.
In fact this brings back PR tree-opt/68327 on the 6 branch:
(gdb) frame 0
#0 internal_error (gmsgid=gmsgid@entry=0x170cd58 "in %s, at %s:%d")
at /home/eric/svn/gcc-6-branch/gcc/diagnostic.c:1251
1251{
(gdb) frame 1
#1 0x0101c5c4 in fancy_abort (file=,
line=, function=)
at /home/eric/svn/gcc-6-branch/gcc/diagnostic.c:1327
1327 internal_error ("in %s, at %s:%d", function, trim_filename (file),
line);
(gdb) frame 2
#2 0x00b75f5e in vect_is_simple_use (operand=0x76c3f9d8,
vinfo=0x1ddb690, def_stmt=0x7fffd888, dt=0x7fffd86c,
vectype=0x7fffd878)
at /home/eric/svn/gcc-6-branch/gcc/tree-vect-stmts.c:8763
8763 gcc_assert (*vectype != NULL_TREE);
(gdb) p debug_gimple_stmt(*def_stmt)
b.0_2 = PHI
$5 = void
because we don't compute STMT_VINFO_VECTYPE for live PHI statement, but only
for relevant ones, so we need this too:
Index: tree-vect-loop.c
===
--- tree-vect-loop.c(revision 236983)
+++ tree-vect-loop.c(working copy)
@@ -216,7 +216,8 @@ vect_determine_vectorization_factor (loo
gcc_assert (stmt_info);
- if (STMT_VINFO_RELEVANT_P (stmt_info))
+ if (STMT_VINFO_RELEVANT_P (stmt_info)
+ || STMT_VINFO_LIVE_P (stmt_info))
{
gcc_assert (!STMT_VINFO_VECTYPE (stmt_info));
scalar_type = TREE_TYPE (PHI_RESULT (phi));
--
Eric Botcazou
Re: [patch] Fix segfault in vectorizer
On Tue, May 31, 2016 at 7:46 PM, Eric Botcazou wrote: >> 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. > > OK, thanks for the explanation. It would be nice to document that somewhere > in the code if this isn't already done. > >> 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. > > That works indeed and generates no regression. OK for mainline and 6 branch? Yes. Thanks, Richard. > > 2016-05-31 Eric Botcazou > > * tree-vect-loop.c (vect_determine_vectorization_factor): Also take > into account live statements for mask producers. > > > 2016-05-31 Eric Botcazou > > * gnat.dg/opt56.ad[sb]: New test. > > -- > Eric Botcazou
Re: [patch] Fix segfault in vectorizer
> 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.
OK, thanks for the explanation. It would be nice to document that somewhere
in the code if this isn't already done.
> 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.
That works indeed and generates no regression. OK for mainline and 6 branch?
2016-05-31 Eric Botcazou
* tree-vect-loop.c (vect_determine_vectorization_factor): Also take
into account live statements for mask producers.
2016-05-31 Eric Botcazou
* gnat.dg/opt56.ad[sb]: New test.
--
Eric BotcazouIndex: tree-vect-loop.c
===
--- tree-vect-loop.c (revision 236877)
+++ tree-vect-loop.c (working copy)
@@ -441,7 +441,8 @@ vect_determine_vectorization_factor (loo
&& is_gimple_assign (stmt)
&& gimple_assign_rhs_code (stmt) != COND_EXPR)
{
- if (STMT_VINFO_RELEVANT_P (stmt_info))
+ if (STMT_VINFO_RELEVANT_P (stmt_info)
+ || STMT_VINFO_LIVE_P (stmt_info))
mask_producers.safe_push (stmt_info);
bool_result = true;
Re: [patch] Fix segfault in vectorizer
2016-05-31 12:25 GMT+03:00 Richard Biener :
> On Tue, May 31, 2016 at 10:22 AM, Eric Botcazou 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
>>
>> * tree-vect-loop.c (vectorizable_reduction): Also return false if the
>> scalar type is a boolean type.
>>
>>
>> 2016-05-31 Eric Botcazou
>>
>> * gnat.dg/opt56.ad[sb]: New test.
>>
>> --
>> Eric Botcazou
Re: [patch] Fix segfault in vectorizer
> I recall that some STMT_VINFO_RELEVANT_P checks have a || > STMT_VINFO_DEF_TYPE () == vect_reduction_def > or VECTORIZABLE_CYCLE_DEF (STMT_VINFO_DEF_TYPE ()). It's rather || STMT_VINFO_LIVE_P in most cases and this works here, the vectorization is properly blocked: opt56.adb:9:29: note: not vectorized: different sized masks types in statement, vector(16) unsigned char and vector(4) opt56.adb:9:29: note: can't determine vectorization factor. opt56.adb:6:4: note: vectorized 0 loops in function. -- Eric Botcazou
Re: [patch] Fix segfault in vectorizer
On Tue, May 31, 2016 at 11:46 AM, Eric Botcazou wrote:
>> 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?
>
> Yes, for
>
> result_15 = _6 & result_3;
>
> wich is a BIT_AND_EXPR, hence accepted by vectorizable_reduction.
>
>> 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.
>
> OK, STMT_VINFO_RELEVANT_P is indeed not set:
>
> (gdb) p *stmt_info
> $4 = {type = undef_vec_info_type, live = true, in_pattern_p = false,
> stmt = 0x768f5738, vinfo = 0x2e14820, vectype = 0x0,
> vectorized_stmt = 0x0, data_ref_info = 0x0, dr_base_address = 0x0,
> dr_init = 0x0, dr_offset = 0x0, dr_step = 0x0, dr_aligned_to = 0x0,
> loop_phi_evolution_base_unchanged = 0x0, loop_phi_evolution_part = 0x0,
> related_stmt = 0x0, pattern_def_seq = 0x0, same_align_refs = {m_vec = 0x0},
> simd_clone_info = {m_vec = 0x0}, def_type = vect_reduction_def,
> slp_type = loop_vect, first_element = 0x0, next_element = 0x0,
> same_dr_stmt = 0x0, size = 0, store_count = 0, gap = 0, min_neg_dist = 0,
> relevant = vect_unused_in_scope, vectorizable = true,
> gather_scatter_p = false, strided_p = false, simd_lane_access_p = false,
> v_reduc_type = TREE_CODE_REDUCTION, num_slp_uses = 0}
I recall that some STMT_VINFO_RELEVANT_P checks have a ||
STMT_VINFO_DEF_TYPE () == vect_reduction_def
or VECTORIZABLE_CYCLE_DEF (STMT_VINFO_DEF_TYPE ()). Maybe that is missing here.
Richard.
> --
> Eric Botcazou
Re: [patch] Fix segfault in vectorizer
> 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?
Yes, for
result_15 = _6 & result_3;
wich is a BIT_AND_EXPR, hence accepted by vectorizable_reduction.
> 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.
OK, STMT_VINFO_RELEVANT_P is indeed not set:
(gdb) p *stmt_info
$4 = {type = undef_vec_info_type, live = true, in_pattern_p = false,
stmt = 0x768f5738, vinfo = 0x2e14820, vectype = 0x0,
vectorized_stmt = 0x0, data_ref_info = 0x0, dr_base_address = 0x0,
dr_init = 0x0, dr_offset = 0x0, dr_step = 0x0, dr_aligned_to = 0x0,
loop_phi_evolution_base_unchanged = 0x0, loop_phi_evolution_part = 0x0,
related_stmt = 0x0, pattern_def_seq = 0x0, same_align_refs = {m_vec = 0x0},
simd_clone_info = {m_vec = 0x0}, def_type = vect_reduction_def,
slp_type = loop_vect, first_element = 0x0, next_element = 0x0,
same_dr_stmt = 0x0, size = 0, store_count = 0, gap = 0, min_neg_dist = 0,
relevant = vect_unused_in_scope, vectorizable = true,
gather_scatter_p = false, strided_p = false, simd_lane_access_p = false,
v_reduc_type = TREE_CODE_REDUCTION, num_slp_uses = 0}
--
Eric Botcazou
Re: [patch] Fix segfault in vectorizer
On Tue, May 31, 2016 at 10:22 AM, Eric Botcazou 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
>
> * tree-vect-loop.c (vectorizable_reduction): Also return false if the
> scalar type is a boolean type.
>
>
> 2016-05-31 Eric Botcazou
>
> * gnat.dg/opt56.ad[sb]: New test.
>
> --
> Eric Botcazou
