On Wed, Nov 12, 2014 at 2:54 PM, Yuri Rumyantsev <[email protected]> wrote:
> Hi All,
>
> Here is the last part of patch which is related to vectorization
> improvements, namely
>
> 1. New sub-phase is added which delete dead predicate computations
> (such code can prevent loop vectorization since dead computation are
> not part of bool pattern tree). It is invoked only under
> FLAG_FORCE_VECTORIZE.
> 2. New sub-phase is added to provide single uses in tree correspondent
> to bool pattern (it takes place when the same predicate is used for
> PHI-node predication and predicated memory access through 'mask'). To
> do it some code replication is used. It is invoked only under
> FLAG_FORCE_VECTORIZE.
> 3. I added simple optimization for predicate_mem_writes - do not
> generate new mask if it has been produced for given size.
> 4. simple change was made in fold_build_cond_expr to simplify
> predicate of kind r != 0 where r has boolean type to simply 'r' which
> is accepted by bool pattern recognition.
Ick :/
In ifcvt_split_def_stmt I wonder why you copy the use_stmt as well?
>From the description it sounds like you want to copy def only.
Then,
+ /* Change one use of var to lhs. */
+ FOR_EACH_IMM_USE_FAST (use_p, imm_iter, var)
+ {
+ use_stmt = USE_STMT (use_p);
+ break;
+ }
I would think this should be simply
SET_USE (use_p, lhs);
break;
in stead of copying the stmt, adding it and then removing the "old".
What am I missing?
The "retry" code also looks odd - why do you walk the BB multiple
times instead of just doing sth like
while (!has_single_use (lhs))
{
gimple copy = ifcvt_split_def_stmt (def_stmt);
ifcvt_walk_pattern_tree (copy);
}
thus returning the copy you create and re-process it (the copy should
now have a single-use).
I think it would be nice to re-use some utility from tree-vect-patterns.c
for stmt_is_root_of_bool_pattern.
+/* Delete redundant statements produced by predication which prevents
+ loop vectorization. */
+
+static void
+ifcvt_local_dce (basic_block bb)
+{
...
+ /* Propagate liveness through arguments of live stmt. */
+ while (worklist.length () > 0)
+ {
+ stmt = worklist.pop ();
+ code = gimple_code (stmt);
+ if (gimple_code (stmt) == GIMPLE_PHI)
+ {
+ for (i = 0; i < gimple_phi_num_args (stmt); i++)
+ {
+ tree arg = PHI_ARG_DEF (stmt, i);
+ if (TREE_CODE (arg) == SSA_NAME)
You can use FOR_EACH_PHI_OR_STMT_USE (but watch out for
non-SSA name "uses" then).
+ /* Delete dead statements. */
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple_stmt_iterator stmt_it;
+
+ stmt = gsi_stmt (gsi);
+ if (gimple_plf (stmt, GF_PLF_2))
+ {
+ gimple_set_plf (stmt, GF_PLF_2, false);
+ continue;
no need to clear pass-local flags. You can't assume they are
not set at the start of your pass.
+ stmt_it = gsi_for_stmt (stmt);
err - simply use 'gsi' and do the gsi_next () only in the case of
GF_PLF_2.
The patchset doesn't seem to have any testcases - I'd like to see
some that exercise all cases you add handling for.
Richard.
> ChangeLog:
>
> 2014-11-12 Yuri Rumyantsev <[email protected]>
>
> * tree-if-conv.c (fold_build_cond_expr): Add conversion COND to
> SSA_NAME if it is comparison r != 0 and r has boolean type.
> (predicate_mem_writes): Introduce mask_vec to save mask correspondent
> to given size. Do not generate new mask if it exists for given size.
> (ifcvt_split_def_stmt): New function.
> (ifcvt_walk_pattern_tree): New function.
> (stmt_is_root_of_bool_pattern): New function.
> (ifcvt_repair_bool_pattern): New function.
> (ifcvt_local_dce): New function.
> (tree_if_conversion): Add calls of ifcvt_local_dce and
> ifcvt_repair_bool_pattern if FLAG_FORCE_VECTORIZE is true.
