Message ID | 201204301619.q3UGJSX6024519@d06av02.portsmouth.uk.ibm.com |
---|---|
State | New |
Headers | show |
On Mon, Apr 30, 2012 at 6:19 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote: > Hello, > > in working on a fix for PR 52633, I noticed that tree-vect-patterns.c now > contains a number of copies of rather similar code (of which my patch > would have added another copy), so it seems to make sense to do a bit of > refactoring first. > > This patch introduced a new helper function "vect_same_loop_or_bb_p" > to decide whether a new statement is in the same loop or basic-block > (depending on whether we're currently doing loop-based or basic-block-based > vectorization) as an existing statement. The helper is then used everywhere > where this test is currently open-coded. > > As a side-effect, the patch actually contains two bug fixes: > > - The condition in some places > (!loop && gimple_bb (def_stmt) != BB_VINFO_BB (bb_vinfo) > && gimple_code (def_stmt) != GIMPLE_PHI) > > doesn't seem to make sense. It allows PHI statements from random > other basic blocks -- but those will have an uninitialized > vinfo_for_stmt, so if that test ever hits, we're going to crash. > > The check was introduced in this patch: > > http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00191.html > > which likewise introduces a similar check in vect_get_and_check_slp_defs: > > (!loop && gimple_bb (def_stmt) == BB_VINFO_BB (bb_vinfo) > && gimple_code (def_stmt) != GIMPLE_PHI)) > > This makes sense: basic-block vectorization operates only on statements > in the current basic block, but *not* on the incoming PHIs. > > It seems that the condition simply was incorrectly negated when moving > it over to the tree-vect-pattern.c uses. > > > - In vect_recog_widen_shift_pattern, the test for same loop / basic-block > is still missing. In my recent patch to PR 52870 I added the check to > vect_recog_widen_mult_pattern ... it is still missing in ...shift_pattern. > > > Tested on i386-linux-gnu and arm-linux-gnueabi with no regressions. > > OK for mainline? Ok. Thanks, Richard. > Bye, > Ulrich > > > ChangeLog: > > * tree-vect-patterns.c (vect_same_loop_or_bb_p): New function. > (vect_handle_widen_op_by_const): Use it instead of open-coding test. > (vect_recog_widen_mult_pattern): Likewise. > (vect_operation_fits_smaller_type): Likewise. > (vect_recog_over_widening_pattern): Likewise. > (vect_recog_widen_shift_pattern): Add to vect_same_loop_or_bb_p test. > > > Index: gcc-head/gcc/tree-vect-patterns.c > =================================================================== > --- gcc-head.orig/gcc/tree-vect-patterns.c 2012-04-26 15:53:48.000000000 +0200 > +++ gcc-head/gcc/tree-vect-patterns.c 2012-04-26 19:46:12.000000000 +0200 > @@ -84,6 +84,41 @@ new_pattern_def_seq (stmt_vec_info stmt_ > append_pattern_def_seq (stmt_info, stmt); > } > > +/* Check whether STMT2 is in the same loop or basic block as STMT1. > + Which of the two applies depends on whether we're currently doing > + loop-based or basic-block-based vectorization, as determined by > + the vinfo_for_stmt for STMT1 (which must be defined). > + > + If this returns true, vinfo_for_stmt for STMT2 is guaranteed > + to be defined as well. */ > + > +static bool > +vect_same_loop_or_bb_p (gimple stmt1, gimple stmt2) > +{ > + stmt_vec_info stmt_vinfo = vinfo_for_stmt (stmt1); > + loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo); > + bb_vec_info bb_vinfo = STMT_VINFO_BB_VINFO (stmt_vinfo); > + > + if (!gimple_bb (stmt2)) > + return false; > + > + if (loop_vinfo) > + { > + struct loop *loop = LOOP_VINFO_LOOP (loop_vinfo); > + if (!flow_bb_inside_loop_p (loop, gimple_bb (stmt2))) > + return false; > + } > + else > + { > + if (gimple_bb (stmt2) != BB_VINFO_BB (bb_vinfo) > + || gimple_code (stmt2) == GIMPLE_PHI) > + return false; > + } > + > + gcc_assert (vinfo_for_stmt (stmt2)); > + return true; > +} > + > /* Check whether NAME, an ssa-name used in USE_STMT, > is a result of a type promotion or demotion, such that: > DEF_STMT: NAME = NOP (name0) > @@ -400,16 +435,6 @@ vect_handle_widen_op_by_const (gimple st > { > tree new_type, new_oprnd, tmp; > gimple new_stmt; > - loop_vec_info loop_vinfo; > - struct loop *loop = NULL; > - bb_vec_info bb_vinfo; > - stmt_vec_info stmt_vinfo; > - > - stmt_vinfo = vinfo_for_stmt (stmt); > - loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo); > - bb_vinfo = STMT_VINFO_BB_VINFO (stmt_vinfo); > - if (loop_vinfo) > - loop = LOOP_VINFO_LOOP (loop_vinfo); > > if (code != MULT_EXPR && code != LSHIFT_EXPR) > return false; > @@ -425,12 +450,10 @@ vect_handle_widen_op_by_const (gimple st > return true; > } > > - if (TYPE_PRECISION (type) < (TYPE_PRECISION (*half_type) * 4) > - || !gimple_bb (def_stmt) > - || (loop && !flow_bb_inside_loop_p (loop, gimple_bb (def_stmt))) > - || (!loop && gimple_bb (def_stmt) != BB_VINFO_BB (bb_vinfo) > - && gimple_code (def_stmt) != GIMPLE_PHI) > - || !vinfo_for_stmt (def_stmt)) > + if (TYPE_PRECISION (type) < (TYPE_PRECISION (*half_type) * 4)) > + return false; > + > + if (!vect_same_loop_or_bb_p (stmt, def_stmt)) > return false; > > /* TYPE is 4 times bigger than HALF_TYPE, try widening operation for > @@ -564,16 +587,6 @@ vect_recog_widen_mult_pattern (VEC (gimp > VEC (tree, heap) *dummy_vec; > bool op1_ok; > bool promotion; > - loop_vec_info loop_vinfo; > - struct loop *loop = NULL; > - bb_vec_info bb_vinfo; > - stmt_vec_info stmt_vinfo; > - > - stmt_vinfo = vinfo_for_stmt (last_stmt); > - loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo); > - bb_vinfo = STMT_VINFO_BB_VINFO (stmt_vinfo); > - if (loop_vinfo) > - loop = LOOP_VINFO_LOOP (loop_vinfo); > > if (!is_gimple_assign (last_stmt)) > return NULL; > @@ -645,9 +658,7 @@ vect_recog_widen_mult_pattern (VEC (gimp > || gimple_assign_rhs_code (use_stmt) != NOP_EXPR) > return NULL; > > - if (!gimple_bb (use_stmt) > - || (loop && !flow_bb_inside_loop_p (loop, gimple_bb (use_stmt))) > - || (!loop && gimple_bb (use_stmt) != BB_VINFO_BB (bb_vinfo))) > + if (!vect_same_loop_or_bb_p (last_stmt, use_stmt)) > return NULL; > > use_lhs = gimple_assign_lhs (use_stmt); > @@ -952,14 +963,8 @@ vect_operation_fits_smaller_type (gimple > tree interm_type = NULL_TREE, half_type, tmp, new_oprnd, type; > gimple def_stmt, new_stmt; > bool first = false; > - loop_vec_info loop_info = STMT_VINFO_LOOP_VINFO (vinfo_for_stmt (stmt)); > - bb_vec_info bb_info = STMT_VINFO_BB_VINFO (vinfo_for_stmt (stmt)); > - struct loop *loop = NULL; > bool promotion; > > - if (loop_info) > - loop = LOOP_VINFO_LOOP (loop_info); > - > *op0 = NULL_TREE; > *op1 = NULL_TREE; > *new_def_stmt = NULL; > @@ -991,13 +996,9 @@ vect_operation_fits_smaller_type (gimple > { > first = true; > if (!type_conversion_p (oprnd, stmt, false, &half_type, &def_stmt, > - &promotion) > - || !promotion > - || !gimple_bb (def_stmt) > - || (loop && !flow_bb_inside_loop_p (loop, gimple_bb (def_stmt))) > - || (!loop && gimple_bb (def_stmt) != BB_VINFO_BB (bb_info) > - && gimple_code (def_stmt) != GIMPLE_PHI) > - || !vinfo_for_stmt (def_stmt)) > + &promotion) > + || !promotion > + || !vect_same_loop_or_bb_p (stmt, def_stmt)) > return false; > } > > @@ -1171,16 +1172,6 @@ vect_recog_over_widening_pattern (VEC (g > tree var = NULL_TREE, new_type = NULL_TREE, tmp, new_oprnd; > bool first; > tree type = NULL; > - loop_vec_info loop_vinfo; > - struct loop *loop = NULL; > - bb_vec_info bb_vinfo; > - stmt_vec_info stmt_vinfo; > - > - stmt_vinfo = vinfo_for_stmt (stmt); > - loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo); > - bb_vinfo = STMT_VINFO_BB_VINFO (stmt_vinfo); > - if (loop_vinfo) > - loop = LOOP_VINFO_LOOP (loop_vinfo); > > first = true; > while (1) > @@ -1212,9 +1203,7 @@ vect_recog_over_widening_pattern (VEC (g > } > > if (nuses != 1 || !is_gimple_assign (use_stmt) > - || !gimple_bb (use_stmt) > - || (loop && !flow_bb_inside_loop_p (loop, gimple_bb (use_stmt))) > - || (!loop && gimple_bb (use_stmt) != BB_VINFO_BB (bb_vinfo))) > + || !vect_same_loop_or_bb_p (stmt, use_stmt)) > return NULL; > > /* Create pattern statement for STMT. */ > @@ -1495,6 +1484,9 @@ vect_recog_widen_shift_pattern (VEC (gim > || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (use_stmt))) > return NULL; > > + if (!vect_same_loop_or_bb_p (last_stmt, use_stmt)) > + return NULL; > + > use_lhs = gimple_assign_lhs (use_stmt); > use_type = TREE_TYPE (use_lhs); > > -- > Dr. Ulrich Weigand > GNU Toolchain for Linux on System z and Cell BE > Ulrich.Weigand@de.ibm.com >
Index: gcc-head/gcc/tree-vect-patterns.c =================================================================== --- gcc-head.orig/gcc/tree-vect-patterns.c 2012-04-26 15:53:48.000000000 +0200 +++ gcc-head/gcc/tree-vect-patterns.c 2012-04-26 19:46:12.000000000 +0200 @@ -84,6 +84,41 @@ new_pattern_def_seq (stmt_vec_info stmt_ append_pattern_def_seq (stmt_info, stmt); } +/* Check whether STMT2 is in the same loop or basic block as STMT1. + Which of the two applies depends on whether we're currently doing + loop-based or basic-block-based vectorization, as determined by + the vinfo_for_stmt for STMT1 (which must be defined). + + If this returns true, vinfo_for_stmt for STMT2 is guaranteed + to be defined as well. */ + +static bool +vect_same_loop_or_bb_p (gimple stmt1, gimple stmt2) +{ + stmt_vec_info stmt_vinfo = vinfo_for_stmt (stmt1); + loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo); + bb_vec_info bb_vinfo = STMT_VINFO_BB_VINFO (stmt_vinfo); + + if (!gimple_bb (stmt2)) + return false; + + if (loop_vinfo) + { + struct loop *loop = LOOP_VINFO_LOOP (loop_vinfo); + if (!flow_bb_inside_loop_p (loop, gimple_bb (stmt2))) + return false; + } + else + { + if (gimple_bb (stmt2) != BB_VINFO_BB (bb_vinfo) + || gimple_code (stmt2) == GIMPLE_PHI) + return false; + } + + gcc_assert (vinfo_for_stmt (stmt2)); + return true; +} + /* Check whether NAME, an ssa-name used in USE_STMT, is a result of a type promotion or demotion, such that: DEF_STMT: NAME = NOP (name0) @@ -400,16 +435,6 @@ vect_handle_widen_op_by_const (gimple st { tree new_type, new_oprnd, tmp; gimple new_stmt; - loop_vec_info loop_vinfo; - struct loop *loop = NULL; - bb_vec_info bb_vinfo; - stmt_vec_info stmt_vinfo; - - stmt_vinfo = vinfo_for_stmt (stmt); - loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo); - bb_vinfo = STMT_VINFO_BB_VINFO (stmt_vinfo); - if (loop_vinfo) - loop = LOOP_VINFO_LOOP (loop_vinfo); if (code != MULT_EXPR && code != LSHIFT_EXPR) return false; @@ -425,12 +450,10 @@ vect_handle_widen_op_by_const (gimple st return true; } - if (TYPE_PRECISION (type) < (TYPE_PRECISION (*half_type) * 4) - || !gimple_bb (def_stmt) - || (loop && !flow_bb_inside_loop_p (loop, gimple_bb (def_stmt))) - || (!loop && gimple_bb (def_stmt) != BB_VINFO_BB (bb_vinfo) - && gimple_code (def_stmt) != GIMPLE_PHI) - || !vinfo_for_stmt (def_stmt)) + if (TYPE_PRECISION (type) < (TYPE_PRECISION (*half_type) * 4)) + return false; + + if (!vect_same_loop_or_bb_p (stmt, def_stmt)) return false; /* TYPE is 4 times bigger than HALF_TYPE, try widening operation for @@ -564,16 +587,6 @@ vect_recog_widen_mult_pattern (VEC (gimp VEC (tree, heap) *dummy_vec; bool op1_ok; bool promotion; - loop_vec_info loop_vinfo; - struct loop *loop = NULL; - bb_vec_info bb_vinfo; - stmt_vec_info stmt_vinfo; - - stmt_vinfo = vinfo_for_stmt (last_stmt); - loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo); - bb_vinfo = STMT_VINFO_BB_VINFO (stmt_vinfo); - if (loop_vinfo) - loop = LOOP_VINFO_LOOP (loop_vinfo); if (!is_gimple_assign (last_stmt)) return NULL; @@ -645,9 +658,7 @@ vect_recog_widen_mult_pattern (VEC (gimp || gimple_assign_rhs_code (use_stmt) != NOP_EXPR) return NULL; - if (!gimple_bb (use_stmt) - || (loop && !flow_bb_inside_loop_p (loop, gimple_bb (use_stmt))) - || (!loop && gimple_bb (use_stmt) != BB_VINFO_BB (bb_vinfo))) + if (!vect_same_loop_or_bb_p (last_stmt, use_stmt)) return NULL; use_lhs = gimple_assign_lhs (use_stmt); @@ -952,14 +963,8 @@ vect_operation_fits_smaller_type (gimple tree interm_type = NULL_TREE, half_type, tmp, new_oprnd, type; gimple def_stmt, new_stmt; bool first = false; - loop_vec_info loop_info = STMT_VINFO_LOOP_VINFO (vinfo_for_stmt (stmt)); - bb_vec_info bb_info = STMT_VINFO_BB_VINFO (vinfo_for_stmt (stmt)); - struct loop *loop = NULL; bool promotion; - if (loop_info) - loop = LOOP_VINFO_LOOP (loop_info); - *op0 = NULL_TREE; *op1 = NULL_TREE; *new_def_stmt = NULL; @@ -991,13 +996,9 @@ vect_operation_fits_smaller_type (gimple { first = true; if (!type_conversion_p (oprnd, stmt, false, &half_type, &def_stmt, - &promotion) - || !promotion - || !gimple_bb (def_stmt) - || (loop && !flow_bb_inside_loop_p (loop, gimple_bb (def_stmt))) - || (!loop && gimple_bb (def_stmt) != BB_VINFO_BB (bb_info) - && gimple_code (def_stmt) != GIMPLE_PHI) - || !vinfo_for_stmt (def_stmt)) + &promotion) + || !promotion + || !vect_same_loop_or_bb_p (stmt, def_stmt)) return false; } @@ -1171,16 +1172,6 @@ vect_recog_over_widening_pattern (VEC (g tree var = NULL_TREE, new_type = NULL_TREE, tmp, new_oprnd; bool first; tree type = NULL; - loop_vec_info loop_vinfo; - struct loop *loop = NULL; - bb_vec_info bb_vinfo; - stmt_vec_info stmt_vinfo; - - stmt_vinfo = vinfo_for_stmt (stmt); - loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo); - bb_vinfo = STMT_VINFO_BB_VINFO (stmt_vinfo); - if (loop_vinfo) - loop = LOOP_VINFO_LOOP (loop_vinfo); first = true; while (1) @@ -1212,9 +1203,7 @@ vect_recog_over_widening_pattern (VEC (g } if (nuses != 1 || !is_gimple_assign (use_stmt) - || !gimple_bb (use_stmt) - || (loop && !flow_bb_inside_loop_p (loop, gimple_bb (use_stmt))) - || (!loop && gimple_bb (use_stmt) != BB_VINFO_BB (bb_vinfo))) + || !vect_same_loop_or_bb_p (stmt, use_stmt)) return NULL; /* Create pattern statement for STMT. */ @@ -1495,6 +1484,9 @@ vect_recog_widen_shift_pattern (VEC (gim || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (use_stmt))) return NULL; + if (!vect_same_loop_or_bb_p (last_stmt, use_stmt)) + return NULL; + use_lhs = gimple_assign_lhs (use_stmt); use_type = TREE_TYPE (use_lhs);