On Wed, 28 Jan 2015, Kyrill Tkachov wrote: > Hi Richard, > > On 28/01/15 14:14, Richard Biener wrote: > > This fixes PR64829 where widening shift pattern detection fails to > > verify the widening operation is used only in the shift. > > > > Bootstrap and regtest running on x86_64-unknown-linux-gnu. > > This patch causes a testsuite fail on an arm-none-eabi cross: > FAIL: gcc.dg/vect/vect-widen-shift-u8.c scan-tree-dump-times vect > "vect_recog_widen_shift_pattern: detected" 2 > FAIL: gcc.dg/vect/vect-widen-shift-u8.c -flto -ffat-lto-objects > scan-tree-dump-times vect "vect_recog_widen_shift_pattern: detected" 2 > > The widen_shift pattern doesn't get recognised although the loop is still > vectorised. > Is this something we can avoid? > Or should the test be adjusted?
Hmm, I'm looking at it closer now - the issue is that the code tries to handle multiple widened shifts using the same pattern stmt (sth we generally fail to do - see bool pattern handling for a good example). But it doesn't consider uses that will not end up in the widen-shift patterns. But then we don't support vectorizing a statement with both using the original stmt and its pattern replacement (still somehow the code dealing with life analysis tries to handle it, but in a very weird non-working way - see vect_mark_relevant in the STMT_VINFO_IN_PATTERN_P && !used_in_pattern case. The widening multiplication pattern detection has the very same issue. I don't see an easy (and thus non-ugly) way out here apart from _not_ marking the extra conversion we insert as related stmt to the existing widening (but then we'll not be able to re-use that stmt as far as I can see - not easily at least without hacking pattern recog to add some on-the-side info). life analysis should then determine correctly whether the original widening is still life (uses both in patterns and non-patterns) or if it can be discarded. This if course delays CSE of the intermediate vec_unpack_lo/hi_exprs. But it handles the mixed uses - widening multiplication has own handling that is correct, just the one it inherits from vect_handle_widen_op_by_const is broken. Can you test? Thanks, Richard. Index: gcc/tree-vect-patterns.c =================================================================== --- gcc/tree-vect-patterns.c (revision 220205) +++ gcc/tree-vect-patterns.c (working copy) @@ -726,7 +726,7 @@ vect_recog_sad_pattern (vec<gimple> *stm static bool vect_handle_widen_op_by_const (gimple stmt, enum tree_code code, tree const_oprnd, tree *oprnd, - vec<gimple> *stmts, tree type, + gimple *wstmt, tree type, tree *half_type, gimple def_stmt) { tree new_type, new_oprnd; @@ -761,29 +761,11 @@ vect_handle_widen_op_by_const (gimple st && compare_tree_int (const_oprnd, TYPE_PRECISION (new_type)) == 1)) return false; - /* Use NEW_TYPE for widening operation. */ - if (STMT_VINFO_RELATED_STMT (vinfo_for_stmt (def_stmt))) - { - new_stmt = STMT_VINFO_RELATED_STMT (vinfo_for_stmt (def_stmt)); - /* Check if the already created pattern stmt is what we need. */ - if (!is_gimple_assign (new_stmt) - || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (new_stmt)) - || TREE_TYPE (gimple_assign_lhs (new_stmt)) != new_type) - return false; - - stmts->safe_push (def_stmt); - *oprnd = gimple_assign_lhs (new_stmt); - } - else - { - /* Create a_T = (NEW_TYPE) a_t; */ - *oprnd = gimple_assign_rhs1 (def_stmt); - new_oprnd = make_ssa_name (new_type); - new_stmt = gimple_build_assign (new_oprnd, NOP_EXPR, *oprnd); - STMT_VINFO_RELATED_STMT (vinfo_for_stmt (def_stmt)) = new_stmt; - stmts->safe_push (def_stmt); - *oprnd = new_oprnd; - } + /* Use NEW_TYPE for widening operation and create a_T = (NEW_TYPE) a_t; */ + *oprnd = gimple_assign_rhs1 (def_stmt); + new_oprnd = make_ssa_name (new_type); + *wstmt = gimple_build_assign (new_oprnd, NOP_EXPR, *oprnd); + *oprnd = new_oprnd; *half_type = new_type; return true; @@ -920,7 +902,7 @@ vect_recog_widen_mult_pattern (vec<gimpl if (TREE_CODE (oprnd1) == INTEGER_CST && TREE_CODE (half_type0) == INTEGER_TYPE && vect_handle_widen_op_by_const (last_stmt, MULT_EXPR, oprnd1, - &oprnd0, stmts, type, + &oprnd0, &new_stmt, type, &half_type0, def_stmt0)) { half_type1 = half_type0; @@ -934,6 +916,10 @@ vect_recog_widen_mult_pattern (vec<gimpl the smaller type into the larger type. */ if (TYPE_PRECISION (half_type0) != TYPE_PRECISION (half_type1)) { + /* If we already used up the single-stmt slot give up. */ + if (new_stmt) + return NULL; + tree* oprnd = NULL; gimple def_stmt = NULL; @@ -1734,7 +1720,7 @@ vect_recog_widen_shift_pattern (vec<gimp /* Check operand 0: it has to be defined by a type promotion. */ if (!type_conversion_p (oprnd0, last_stmt, false, &half_type0, &def_stmt0, - &promotion) + &promotion) || !promotion) return NULL; @@ -1764,8 +1750,9 @@ vect_recog_widen_shift_pattern (vec<gimp } /* Check if this a widening operation. */ + gimple wstmt = NULL; if (!vect_handle_widen_op_by_const (last_stmt, LSHIFT_EXPR, oprnd1, - &oprnd0, stmts, + &oprnd0, &wstmt, type, &half_type0, def_stmt0)) return NULL; @@ -1793,6 +1780,17 @@ vect_recog_widen_shift_pattern (vec<gimp var = vect_recog_temp_ssa_var (type, NULL); pattern_stmt = gimple_build_assign (var, WIDEN_LSHIFT_EXPR, oprnd0, oprnd1); + if (wstmt) + { + stmt_vec_info stmt_vinfo = vinfo_for_stmt (last_stmt); + loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo); + bb_vec_info bb_vinfo = STMT_VINFO_BB_VINFO (stmt_vinfo); + new_pattern_def_seq (stmt_vinfo, wstmt); + stmt_vec_info new_stmt_info + = new_stmt_vec_info (wstmt, loop_vinfo, bb_vinfo); + set_vinfo_for_stmt (wstmt, new_stmt_info); + STMT_VINFO_VECTYPE (new_stmt_info) = vectype; + } if (dump_enabled_p ()) dump_gimple_stmt_loc (MSG_NOTE, vect_location, TDF_SLIM, pattern_stmt, 0); @@ -3414,7 +3412,6 @@ vect_pattern_recog_1 (vect_recog_func_pt dump_printf_loc (MSG_NOTE, vect_location, "pattern recognized: "); dump_gimple_stmt (MSG_NOTE, TDF_SLIM, pattern_stmt, 0); - dump_printf (MSG_NOTE, "\n"); } /* Mark the stmts that are involved in the pattern. */ @@ -3441,7 +3438,6 @@ vect_pattern_recog_1 (vect_recog_func_pt dump_printf_loc (MSG_NOTE, vect_location, "additional pattern stmt: "); dump_gimple_stmt (MSG_NOTE, TDF_SLIM, pattern_stmt, 0); - dump_printf (MSG_NOTE, "\n"); } vect_mark_pattern_stmts (stmt, pattern_stmt, NULL_TREE);