On Mon, May 23, 2016 at 11:28 AM, Alan Hayward <alan.hayw...@arm.com> wrote:
> Vectorize inductions that are live after the loop.
>
> Stmts which are live (ie: defined inside a loop and then used after the
> loop)
> are not currently supported by the vectorizer. In many cases
> vectorization can
> still occur because the SCEV cprop pass will hoist the definition of the
> stmt
> outside of the loop before the vectorizor pass. However, there are various
> cases SCEV cprop cannot hoist, for example:
> for (i = 0; i < n; ++i)
> {
> ret = x[i];
> x[i] = i;
> }
> return i;
>
> Currently stmts are marked live using a bool, and the relevant state using
> an
> enum. Both these states are propagated to the definition of all uses of the
> stmt. Also, a stmt can be live but not relevant.
>
> This patch vectorizes a live stmt definition normally within the loop and
> then
> after the loop uses BIT_FIELD_REF to extract the final scalar value from
> the
> vector.
>
> This patch adds a new relevant state (vect_used_only_live) for when a stmt
> is
> used only outside the loop. The relevant state is still propagated to all
> it's
> uses, but the live bool is not (this ensures that
> vectorizable_live_operation
> is only called with stmts that really are live).
>
> Tested on x86 and aarch64.
@@ -6332,79 +6324,81 @@ vectorizable_live_operation (gimple *stmt,
stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
struct loop *loop = LOOP_VINFO_LOOP (loop_vinfo);
- tree op;
- gimple *def_stmt;
- ssa_op_iter iter;
+ imm_use_iterator imm_iter;
+ tree lhs, lhs_type, vec_lhs;
+ tree vectype = STMT_VINFO_VECTYPE (stmt_info);
+ int nunits = TYPE_VECTOR_SUBPARTS (vectype);
+ int ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits;
+ gimple *use_stmt;
gcc_assert (STMT_VINFO_LIVE_P (stmt_info));
+ if (STMT_VINFO_TYPE (stmt_info) == reduc_vec_info_type)
+ return true;
+
This is an odd check - it says the stmt is handled by
vectorizable_reduction. And your
return claims it is handled by vectorizable_live_operation ...
You removed the SIMD lane handling?
@@ -303,6 +335,16 @@ vect_stmt_relevant_p (gimple *stmt, loop_vec_info
loop_vinfo,
}
}
+ if (*live_p && *relevant == vect_unused_in_scope
+ && !is_simple_and_all_uses_invariant (stmt, loop_vinfo))
+ {
+ if (dump_enabled_p ())
+ dump_printf_loc (MSG_NOTE, vect_location,
+ "vec_stmt_relevant_p: live and not all uses "
+ "invariant.\n");
+ *relevant = vect_used_only_live;
+ }
But that's a missed invariant motion / code sinking opportunity then.
Did you have a
testcase for this?
@@ -618,57 +660,31 @@ vect_mark_stmts_to_be_vectorized (loop_vec_info
loop_vinfo)
}
/* Examine the USEs of STMT. For each USE, mark the stmt that defines it
- (DEF_STMT) as relevant/irrelevant and live/dead according to the
- liveness and relevance properties of STMT. */
+ (DEF_STMT) as relevant/irrelevant according to the relevance property
+ of STMT. */
stmt_vinfo = vinfo_for_stmt (stmt);
relevant = STMT_VINFO_RELEVANT (stmt_vinfo);
- live_p = STMT_VINFO_LIVE_P (stmt_vinfo);
-
- /* Generally, the liveness and relevance properties of STMT are
- propagated as is to the DEF_STMTs of its USEs:
- live_p <-- STMT_VINFO_LIVE_P (STMT_VINFO)
- relevant <-- STMT_VINFO_RELEVANT (STMT_VINFO)
-
- One exception is when STMT has been identified as defining a reduction
- variable; in this case we set the liveness/relevance as follows:
- live_p = false
- relevant = vect_used_by_reduction
- This is because we distinguish between two kinds of relevant stmts -
- those that are used by a reduction computation, and those that are
- (also) used by a regular computation. This allows us later on to
- identify stmts that are used solely by a reduction, and therefore the
- order of the results that they produce does not have to be kept. */
-
- def_type = STMT_VINFO_DEF_TYPE (stmt_vinfo);
- tmp_relevant = relevant;
- switch (def_type)
+
+ switch (STMT_VINFO_DEF_TYPE (stmt_vinfo))
{
you removed this comment. Is it no longer valid? Can you please
instead update it?
This is a tricky area.
@@ -1310,17 +1325,14 @@ vect_init_vector (gimple *stmt, tree val, tree
type, gimple_stmt_iterator *gsi)
In case OP is an invariant or constant, a new stmt that creates a vector def
needs to be introduced. VECTYPE may be used to specify a required type for
vector invariant. */
-
-tree
-vect_get_vec_def_for_operand (tree op, gimple *stmt, tree vectype)
+static tree
+vect_get_vec_def_for_operand_internal (tree op, gimple *stmt,
+ loop_vec_info loop_vinfo, tree vectype)
{
tree vec_oprnd;
...
+tree
+vect_get_vec_def_for_operand (tree op, gimple *stmt, tree vectype)
+{
+ stmt_vec_info stmt_vinfo = vinfo_for_stmt (stmt);
+ loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo);
+ return vect_get_vec_def_for_operand_internal (op, stmt, loop_vinfo, vectype);
+}
+
+tree
+vect_get_vec_def_for_operand_outside (tree op, loop_vec_info loop_vinfo)
+{
+ return vect_get_vec_def_for_operand_internal (op, NULL, loop_vinfo, NULL);
+}
I was trying to reduce the number of _get_vec_def variants, now you
add another one :/
I think as cleanup it would be nice to pass down loop_vinfo directly
(or rather the
more generic vec_info as needed by vect_is_simple_use). The stmt_vectype hack
that was required for bools should ideally be dropped as well as the
constant/external
path inserting init stmts via vect_init_vector (we shouldn't require
stmt for vect_get_vec_def_for_operand
at all).
Note that I think you mishandle SLP with your patch as
vect_get_vec_def_for_operand does not
work (reliably at least?) for pure-SLP defs.
vectorizable_live_operation needs access to the SLP tree.
I tried to merge vect_get_slp_defs (and its various variants) and
vect_get_vec_def_for_opernad (and its various variants) last stage3
but ran out of
time with the gazillions of cleanup opportunities that were surfacing ;)
Still overall I like the patch. Can you trim it down by first not
handling the is_simple_and_all_uses_invariant
case?
Thanks,
Richard.
> gcc/
> * tree-vect-loop.c (vect_analyze_loop_operations): Allow live stmts.
> (vectorizable_reduction): Check for new relevant state.
> (vectorizable_live_operation): vectorize live stmts using
> BIT_FIELD_REF. Remove special case for gimple assigns stmts.
> * tree-vect-stmts.c (is_simple_and_all_uses_invariant): New function.
> (vect_stmt_relevant_p): Check for stmts which are only used outside the
> loop.
> (process_use): Use of a stmt does not inherit it's live value.
> (vect_mark_stmts_to_be_vectorized): Simplify relevance inheritance.
> (vect_get_vec_def_for_operand): Split body into...
> (vect_get_vec_def_for_operand_internal): ...new function
> (vect_get_vec_def_for_operand_outside): New. Same as above but for
> stmts outside a loop.
> (vect_analyze_stmt): Check for new relevant state.
> *tree-vectorizer.h (vect_relevant): New entry for a stmt which is used
> outside the loop, but not inside it.
>
> testsuite/
> * gcc.dg/tree-ssa/pr64183.c: Ensure test does not vectorize.
> * testsuite/gcc.dg/vect/no-scevccp-vect-iv-2.c: Remove xfail.
> * gcc.dg/vect/vect-live-1.c: New test.
> * gcc.dg/vect/vect-live-2.c: New test.
> * gcc.dg/vect/vect-live-3.c: New test.
> * gcc.dg/vect/vect-live-4.c: New test.
>
>
> Cheers,
> Alan.
>
