On Tue, 4 Jul 2023, Richard Biener wrote:
> On Mon, 3 Jul 2023, Andre Vieira (lists) wrote:
>
> > Hi,
> >
> > This patch makes the vectorizer treat any vector widening IFN as simple,
> > like
> > it did with the tree codes VEC_WIDEN_*.
> >
> > I wasn't sure whether I should make all IFN's simple and then exclude some
> > (like GOMP_ ones), or include more than just the new widening IFNs. But
> > since
> > this is the only behaviour that changed with the ifn patch, I decided to
> > only
> > special case the widening IFNs for now. Let me know if you have different
> > thoughts on this.
> >
> > Bootstrapped and regression tested on aarch64-unknow-linux-gnu.
>
> But could we expand all VEC_WIDEN_* with scalar operands properly? Can
> we do that for the IFNs? I think that's what we will end up doing then?
>
> How do we end up with !STMT_VINFO_LIVE_P here anyway?
>
> vectorizable_live_operation does
>
> /* If STMT is not relevant and it is a simple assignment and its inputs
> are
> invariant then it can remain in place, unvectorized. The original
> last
> scalar value that it computes will be used. */
> if (!STMT_VINFO_RELEVANT_P (stmt_info))
> {
> gcc_assert (is_simple_and_all_uses_invariant (stmt_info,
> loop_vinfo));
> if (dump_enabled_p ())
> dump_printf_loc (MSG_NOTE, vect_location,
> "statement is simple and uses invariant. Leaving
> in "
> "place.\n");
> return true;
I _think_ the issue is that we do
pr83089.c:15:23: note: mark relevant 0, live 1: zy_24 = hb_20 + 1;
pr83089.c:15:23: note: last stmt in pattern. don't mark relevant/live.
and instead mark
pr83089.c:15:23: note: worklist: examine stmt: patt_47 = .VEC_WIDEN_PLUS
(el_36, 1);
so we used the is_simple_and_all_uses_invariant predicate on the
"wrong" stmt in the vect_stmt_relevant_p call.
I think we want something like below. We then get
pr83089.c:15:23: note: init: stmt relevant? zy_24 = hb_20 + 1;
pr83089.c:15:23: note: vec_stmt_relevant_p: used out of loop.
pr83089.c:15:23: note: vect_is_simple_use: operand (int) el_36, type of
def: external
pr83089.c:15:23: note: mark relevant 0, live 1: zy_24 = hb_20 + 1;
pr83089.c:15:23: note: last stmt in pattern. don't mark relevant/live.
pr83089.c:15:23: note: vec_stmt_relevant_p: forcing live patern stmt
relevant.
pr83089.c:15:23: note: mark relevant 1, live 1: patt_47 =
.VEC_WIDEN_PLUS (el_36, 1);
that's not optimal from a code gen perspective but it's how the
vectorizer works (we can't cancel a pattern).
I'm going to test this.
Richard.
diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
index a0c39268bf0..c3e6f2d34ed 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -261,11 +261,26 @@ vect_mark_relevant (vec<stmt_vec_info> *worklist,
stmt_vec_info stmt_info,
dump_printf_loc (MSG_NOTE, vect_location,
"last stmt in pattern. don't mark"
" relevant/live.\n");
+
stmt_vec_info old_stmt_info = stmt_info;
stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);
gcc_assert (STMT_VINFO_RELATED_STMT (stmt_info) == old_stmt_info);
save_relevant = STMT_VINFO_RELEVANT (stmt_info);
save_live_p = STMT_VINFO_LIVE_P (stmt_info);
+
+ if (live_p && relevant == vect_unused_in_scope)
+ {
+ if (dump_enabled_p ())
+ dump_printf_loc (MSG_NOTE, vect_location,
+ "vec_stmt_relevant_p: forcing live patern stmt "
+ "relevant.\n");
+ relevant = vect_used_only_live;
+ }
+
+ if (dump_enabled_p ())
+ dump_printf_loc (MSG_NOTE, vect_location,
+ "mark relevant %d, live %d: %G", relevant, live_p,
+ stmt_info->stmt);
}
STMT_VINFO_LIVE_P (stmt_info) |= live_p;
