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);
