Re: [PATCH] Fix up vect pattern handling (PR target/70021)
On Wed, 2 Mar 2016, Jakub Jelinek wrote:
> Hi!
>
> This patch fixes two issues:
> 1) reverts part of https://gcc.gnu.org/ml/gcc-patches/2011-06/msg02183.html
>changes, my understanding is that we don't emit or support what has been
>mentioned as rationale for that, now that we can stick multiple pattern
>stmts into a sequence; without this reversion, we mark both the pattern
>and normal stmt relevant and then when vectorizing use in one spot
>the pattern stmt and in another one the original stmt, while we clearly
>want to use the pattern stmt always; also, without the reversion we
>consider the original stmt as needed for VF computation and thus think
>the loop needs QImode elements in addition to SImode and DImode.
> 2) if the shift count is coming from stmt with corresponding pattern stmt,
>we shouldn't look through it, because we might again refer to the middle
>of a pattern stmt (this is similar to the recently committed patch); we
>don't need to punt completely in that case though, the code can just
>add a cast into the pattern sequence as it does in many other cases.
>
> Bootstrapped/regtested on {x86_64,i686,powerpc64,powerpc64le}-linux,
> bootstrap/regtest is still pending on {s390,s390x,aarch64,armv7hl}-linux,
> ok for trunk if it passes even there?
Ok. I've been trying to understand this code multiple times myself
and I'm happy to see it go ;) Even though in all cases I remember
the issue was elsewhere...
Thanks,
Richard.
> 2016-03-02 Jakub Jelinek
>
> PR target/70021
> * tree-vect-stmts.c (vect_mark_relevant): Remove USED_IN_PATTERN
> argument, if STMT_VINFO_IN_PATTERN_P (stmt_info), always mark
> the pattern no matter if it is used just by non-pattern, pattern
> or mix thereof.
> (process_use, vect_mark_stmts_to_be_vectorized): Adjust callers.
> * tree-vect-patterns.c (vect_recog_vector_vector_shift_pattern): If
> oprnd1 def_stmt is in pattern, don't look through it.
>
> * gcc.dg/vect/pr70021.c: New test.
> * gcc.target/i386/pr70021.c: New test.
>
> --- gcc/tree-vect-stmts.c.jj 2016-03-01 19:23:51.0 +0100
> +++ gcc/tree-vect-stmts.c 2016-03-02 15:40:53.777231771 +0100
> @@ -181,8 +181,7 @@ create_array_ref (tree type, tree ptr, s
>
> static void
> vect_mark_relevant (vec *worklist, gimple *stmt,
> - enum vect_relevant relevant, bool live_p,
> - bool used_in_pattern)
> + enum vect_relevant relevant, bool live_p)
> {
>stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
>enum vect_relevant save_relevant = STMT_VINFO_RELEVANT (stmt_info);
> @@ -202,62 +201,22 @@ vect_mark_relevant (vec *workl
> stmt itself should be marked. */
>if (STMT_VINFO_IN_PATTERN_P (stmt_info))
> {
> - bool found = false;
> - if (!used_in_pattern)
> -{
> - imm_use_iterator imm_iter;
> - use_operand_p use_p;
> - gimple *use_stmt;
> - tree lhs;
> - loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
> - struct loop *loop = LOOP_VINFO_LOOP (loop_vinfo);
> -
> - if (is_gimple_assign (stmt))
> -lhs = gimple_assign_lhs (stmt);
> - else
> -lhs = gimple_call_lhs (stmt);
> -
> - /* This use is out of pattern use, if LHS has other uses that are
> - pattern uses, we should mark the stmt itself, and not the
> pattern
> - stmt. */
> - if (lhs && TREE_CODE (lhs) == SSA_NAME)
> - FOR_EACH_IMM_USE_FAST (use_p, imm_iter, lhs)
> - {
> - if (is_gimple_debug (USE_STMT (use_p)))
> - continue;
> - use_stmt = USE_STMT (use_p);
> -
> - if (!flow_bb_inside_loop_p (loop, gimple_bb (use_stmt)))
> - continue;
> -
> - if (vinfo_for_stmt (use_stmt)
> - && STMT_VINFO_IN_PATTERN_P (vinfo_for_stmt (use_stmt)))
> - {
> - found = true;
> - break;
> - }
> - }
> -}
> + /* This is the last stmt in a sequence that was detected as a
> + pattern that can potentially be vectorized. Don't mark the stmt
> + as relevant/live because it's not going to be vectorized.
> + Instead mark the pattern-stmt that replaces it. */
>
> - if (!found)
> -{
> - /* This is the last stmt in a sequence that was detected as a
> - pattern that can potentially be vectorized. Don't mark the stmt
> - as relevant/live because it's not going to be vectorized.
> - Instead mark the pattern-stmt that replaces it. */
> -
> - pattern_stmt = STMT_VINFO_RELATED_STMT (stmt_info);
> -
> - if (dump_enabled_p ())
> -dump_printf_loc (MSG_NOTE, vect_location,
> - "last stmt in pattern. don't mark"
> -
[PATCH] Fix up vect pattern handling (PR target/70021)
Hi!
This patch fixes two issues:
1) reverts part of https://gcc.gnu.org/ml/gcc-patches/2011-06/msg02183.html
changes, my understanding is that we don't emit or support what has been
mentioned as rationale for that, now that we can stick multiple pattern
stmts into a sequence; without this reversion, we mark both the pattern
and normal stmt relevant and then when vectorizing use in one spot
the pattern stmt and in another one the original stmt, while we clearly
want to use the pattern stmt always; also, without the reversion we
consider the original stmt as needed for VF computation and thus think
the loop needs QImode elements in addition to SImode and DImode.
2) if the shift count is coming from stmt with corresponding pattern stmt,
we shouldn't look through it, because we might again refer to the middle
of a pattern stmt (this is similar to the recently committed patch); we
don't need to punt completely in that case though, the code can just
add a cast into the pattern sequence as it does in many other cases.
Bootstrapped/regtested on {x86_64,i686,powerpc64,powerpc64le}-linux,
bootstrap/regtest is still pending on {s390,s390x,aarch64,armv7hl}-linux,
ok for trunk if it passes even there?
2016-03-02 Jakub Jelinek
PR target/70021
* tree-vect-stmts.c (vect_mark_relevant): Remove USED_IN_PATTERN
argument, if STMT_VINFO_IN_PATTERN_P (stmt_info), always mark
the pattern no matter if it is used just by non-pattern, pattern
or mix thereof.
(process_use, vect_mark_stmts_to_be_vectorized): Adjust callers.
* tree-vect-patterns.c (vect_recog_vector_vector_shift_pattern): If
oprnd1 def_stmt is in pattern, don't look through it.
* gcc.dg/vect/pr70021.c: New test.
* gcc.target/i386/pr70021.c: New test.
--- gcc/tree-vect-stmts.c.jj2016-03-01 19:23:51.0 +0100
+++ gcc/tree-vect-stmts.c 2016-03-02 15:40:53.777231771 +0100
@@ -181,8 +181,7 @@ create_array_ref (tree type, tree ptr, s
static void
vect_mark_relevant (vec *worklist, gimple *stmt,
- enum vect_relevant relevant, bool live_p,
- bool used_in_pattern)
+ enum vect_relevant relevant, bool live_p)
{
stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
enum vect_relevant save_relevant = STMT_VINFO_RELEVANT (stmt_info);
@@ -202,62 +201,22 @@ vect_mark_relevant (vec *workl
stmt itself should be marked. */
if (STMT_VINFO_IN_PATTERN_P (stmt_info))
{
- bool found = false;
- if (!used_in_pattern)
-{
- imm_use_iterator imm_iter;
- use_operand_p use_p;
- gimple *use_stmt;
- tree lhs;
- loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
- struct loop *loop = LOOP_VINFO_LOOP (loop_vinfo);
-
- if (is_gimple_assign (stmt))
-lhs = gimple_assign_lhs (stmt);
- else
-lhs = gimple_call_lhs (stmt);
-
- /* This use is out of pattern use, if LHS has other uses that are
- pattern uses, we should mark the stmt itself, and not the pattern
- stmt. */
- if (lhs && TREE_CODE (lhs) == SSA_NAME)
- FOR_EACH_IMM_USE_FAST (use_p, imm_iter, lhs)
- {
- if (is_gimple_debug (USE_STMT (use_p)))
- continue;
- use_stmt = USE_STMT (use_p);
-
- if (!flow_bb_inside_loop_p (loop, gimple_bb (use_stmt)))
- continue;
-
- if (vinfo_for_stmt (use_stmt)
- && STMT_VINFO_IN_PATTERN_P (vinfo_for_stmt (use_stmt)))
- {
- found = true;
- break;
- }
- }
-}
+ /* This is the last stmt in a sequence that was detected as a
+pattern that can potentially be vectorized. Don't mark the stmt
+as relevant/live because it's not going to be vectorized.
+Instead mark the pattern-stmt that replaces it. */
- if (!found)
-{
- /* This is the last stmt in a sequence that was detected as a
- pattern that can potentially be vectorized. Don't mark the stmt
- as relevant/live because it's not going to be vectorized.
- Instead mark the pattern-stmt that replaces it. */
-
- pattern_stmt = STMT_VINFO_RELATED_STMT (stmt_info);
-
- if (dump_enabled_p ())
-dump_printf_loc (MSG_NOTE, vect_location,
- "last stmt in pattern. don't mark"
- " relevant/live.\n");
- stmt_info = vinfo_for_stmt (pattern_stmt);
- gcc_assert (STMT_VINFO_RELATED_STMT (stmt_info) == stmt);
- save_relevant = STMT_VINFO_RELEVANT (stmt_info);
- save_live_p = STMT_VINFO_LIVE_P (stmt_info);
- stmt = pattern_stmt;
-}
+ pattern_stmt =
