| Message ID | 3369303.TJyCcmduk2@polaris |
|---|---|
| State | New |
| Headers | show |
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
> 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 = 0x7ffff68f5738, 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}
On Tue, May 31, 2016 at 11:46 AM, Eric Botcazou <ebotcazou@adacore.com> 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 = 0x7ffff68f5738, 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
> 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) <unnamed type> opt56.adb:9:29: note: can't determine vectorization factor. opt56.adb:6:4: note: vectorized 0 loops in function.
2016-05-31 12:25 GMT+03:00 Richard Biener <richard.guenther@gmail.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. 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 <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
Index: tree-vect-loop.c =================================================================== --- tree-vect-loop.c (revision 236877) +++ tree-vect-loop.c (working copy) @@ -5496,13 +5496,15 @@ vectorizable_reduction (gimple *stmt, gi scalar_dest = gimple_assign_lhs (stmt); scalar_type = TREE_TYPE (scalar_dest); - if (!POINTER_TYPE_P (scalar_type) && !INTEGRAL_TYPE_P (scalar_type) + if (!POINTER_TYPE_P (scalar_type) + && !INTEGRAL_TYPE_P (scalar_type) && !SCALAR_FLOAT_TYPE_P (scalar_type)) return false; - /* Do not try to vectorize bit-precision reductions. */ - if ((TYPE_PRECISION (scalar_type) - != GET_MODE_PRECISION (TYPE_MODE (scalar_type)))) + /* Do not try to vectorize boolean or bit-precision reductions. */ + if (TREE_CODE (scalar_type) == BOOLEAN_TYPE + || TYPE_PRECISION (scalar_type) + != GET_MODE_PRECISION (TYPE_MODE (scalar_type))) return false; /* All uses but the last are expected to be defined in the loop.
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). 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.