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. >