On 6/12/20 3:22 PM, Richard Sandiford wrote:
Martin Liška <mli...@suse.cz> writes:
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 636ad59c001..eac372e6abc 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -5120,20 +5120,12 @@ vect_determine_precisions (vec_info *vinfo)
else
{
bb_vec_info bb_vinfo = as_a <bb_vec_info> (vinfo);
- gimple_stmt_iterator si = bb_vinfo->region_end;
- gimple *stmt;
- do
+ for (gimple *stmt: *bb_vinfo)
{
- if (!gsi_stmt (si))
- si = gsi_last_bb (bb_vinfo->bb);
- else
- gsi_prev (&si);
- stmt = gsi_stmt (si);
stmt_vec_info stmt_info = vinfo->lookup_stmt (stmt);
if (stmt_info && STMT_VINFO_VECTORIZABLE (stmt_info))
vect_determine_stmt_precisions (vinfo, stmt_info);
}
- while (stmt != gsi_stmt (bb_vinfo->region_begin));
}
}
This loop wants a reverse iterator: it starts at the end and walks
backwards. That's important because vect_determine_stmt_precisions
acts based on information recorded about later uses.
I thought that it may be a problematic change. Note that we don't a have
test coverage for the situation in testsuite (on x86_64). So I'm also
introducing reverse_iterator for this.
@@ -5492,10 +5484,8 @@ vect_pattern_recog (vec_info *vinfo)
else
{
bb_vec_info bb_vinfo = as_a <bb_vec_info> (vinfo);
- for (si = bb_vinfo->region_begin;
- gsi_stmt (si) != gsi_stmt (bb_vinfo->region_end); gsi_next (&si))
+ for (gimple *stmt: *bb_vinfo)
{
- gimple *stmt = gsi_stmt (si);
stmt_vec_info stmt_info = bb_vinfo->lookup_stmt (stmt);
if (!stmt_info || !STMT_VINFO_VECTORIZABLE (stmt_info))
continue;
Very minor, but I think it's more usual to have a space both sides of
the ":". (That's also the formatting used by libstdc++-v3, and ties in
nicely with the formatting of infix operators.) Same for the others.
Changed.
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index cdd6f6c5e5d..766598862d4 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -1342,7 +1342,7 @@ vect_init_vector_1 (vec_info *vinfo, stmt_vec_info
stmt_vinfo, gimple *new_stmt,
else
{
bb_vec_info bb_vinfo = dyn_cast <bb_vec_info> (vinfo);
- gimple_stmt_iterator gsi_region_begin = bb_vinfo->region_begin;
+ gimple_stmt_iterator gsi_region_begin = bb_vinfo->begin ().gsi;
gsi_insert_before (&gsi_region_begin, new_stmt, GSI_SAME_STMT);
}
}
Feels like this kind-of breaks the abstraction a bit.
Would it make sense instead to add the operators to gimple_stmt_iterator
itself and just make const_iterator a typedef of that?
Well, I'm planning to use the _bb_vec_info::const_iterator to jump in between
BBs, so it can't be a simple typedef.
We could then
reuse this work for BB iterators, etc.
diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
index 6c830ad09f4..8f80a7d1bce 100644
--- a/gcc/tree-vectorizer.h
+++ b/gcc/tree-vectorizer.h
@@ -787,12 +787,46 @@ loop_vec_info_for_loop (class loop *loop)
typedef class _bb_vec_info : public vec_info
{
public:
+ struct const_iterator
+ {
+ const_iterator (gimple_stmt_iterator _gsi): gsi (_gsi)
+ {
+ }
Again very minor, but I think the conventions say that this should
be defined on one line, like the later operator* is. Space before
":" here too.
Likewise changed here.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Martin
Thanks,
Richard
>From 8859cb13a20d91222103577f21af2cd029344a5d Mon Sep 17 00:00:00 2001
From: Martin Liska <mli...@suse.cz>
Date: Thu, 11 Jun 2020 13:25:40 +0200
Subject: [PATCH] vectorizer: add _bb_vec_info::const_iterator
gcc/ChangeLog:
* tree-vectorizer.h: Add the new const_iterator and
reverse_iterator and all related functions.
* tree-vect-patterns.c (vect_determine_precisions): Use the
iterator.
(vect_pattern_recog): Likewise.
* tree-vect-slp.c (_bb_vec_info::_bb_vec_info): Likewise.
(_bb_vec_info::~_bb_vec_info): Likewise.
(vect_slp_check_for_constructors): Likewise.
* tree-vect-stmts.c (vect_init_vector_1): Likewise.
---
gcc/tree-vect-patterns.c | 16 +++------
gcc/tree-vect-slp.c | 28 ++++++---------
gcc/tree-vect-stmts.c | 2 +-
gcc/tree-vectorizer.h | 74 ++++++++++++++++++++++++++++++++++++++--
4 files changed, 87 insertions(+), 33 deletions(-)
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 636ad59c001..e24dc848d7b 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -5120,20 +5120,14 @@ vect_determine_precisions (vec_info *vinfo)
else
{
bb_vec_info bb_vinfo = as_a <bb_vec_info> (vinfo);
- gimple_stmt_iterator si = bb_vinfo->region_end;
- gimple *stmt;
- do
+ for (_bb_vec_info::reverse_iterator it = bb_vinfo->rbegin ();
+ it != bb_vinfo->rend (); ++it)
{
- if (!gsi_stmt (si))
- si = gsi_last_bb (bb_vinfo->bb);
- else
- gsi_prev (&si);
- stmt = gsi_stmt (si);
+ gimple *stmt = *it;
stmt_vec_info stmt_info = vinfo->lookup_stmt (stmt);
if (stmt_info && STMT_VINFO_VECTORIZABLE (stmt_info))
vect_determine_stmt_precisions (vinfo, stmt_info);
}
- while (stmt != gsi_stmt (bb_vinfo->region_begin));
}
}
@@ -5492,10 +5486,8 @@ vect_pattern_recog (vec_info *vinfo)
else
{
bb_vec_info bb_vinfo = as_a <bb_vec_info> (vinfo);
- for (si = bb_vinfo->region_begin;
- gsi_stmt (si) != gsi_stmt (bb_vinfo->region_end); gsi_next (&si))
+ for (gimple *stmt : *bb_vinfo)
{
- gimple *stmt = gsi_stmt (si);
stmt_vec_info stmt_info = bb_vinfo->lookup_stmt (stmt);
if (!stmt_info || !STMT_VINFO_VECTORIZABLE (stmt_info))
continue;
diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index 303410c2fc4..cc4ff588a13 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -2551,15 +2551,11 @@ _bb_vec_info::_bb_vec_info (gimple_stmt_iterator region_begin_in,
vec_info_shared *shared)
: vec_info (vec_info::bb, init_cost (NULL), shared),
bb (gsi_bb (region_begin_in)),
- region_begin (region_begin_in),
- region_end (region_end_in)
+ m_region_begin (region_begin_in),
+ m_region_end (region_end_in)
{
- gimple_stmt_iterator gsi;
-
- for (gsi = region_begin; gsi_stmt (gsi) != gsi_stmt (region_end);
- gsi_next (&gsi))
+ for (gimple *stmt : *this)
{
- gimple *stmt = gsi_stmt (gsi);
gimple_set_uid (stmt, 0);
if (is_gimple_debug (stmt))
continue;
@@ -2575,10 +2571,9 @@ _bb_vec_info::_bb_vec_info (gimple_stmt_iterator region_begin_in,
_bb_vec_info::~_bb_vec_info ()
{
- for (gimple_stmt_iterator si = region_begin;
- gsi_stmt (si) != gsi_stmt (region_end); gsi_next (&si))
+ for (gimple *stmt : *this)
/* Reset region marker. */
- gimple_set_uid (gsi_stmt (si), -1);
+ gimple_set_uid (stmt, -1);
bb->aux = NULL;
}
@@ -3012,16 +3007,13 @@ vect_bb_vectorization_profitable_p (bb_vec_info bb_vinfo)
static void
vect_slp_check_for_constructors (bb_vec_info bb_vinfo)
{
- gimple_stmt_iterator gsi;
-
- for (gsi = bb_vinfo->region_begin;
- gsi_stmt (gsi) != gsi_stmt (bb_vinfo->region_end); gsi_next (&gsi))
+ for (gimple *stmt : *bb_vinfo)
{
- gassign *stmt = dyn_cast <gassign *> (gsi_stmt (gsi));
- if (!stmt || gimple_assign_rhs_code (stmt) != CONSTRUCTOR)
+ gassign *assign = dyn_cast <gassign *> (stmt);
+ if (!assign || gimple_assign_rhs_code (assign) != CONSTRUCTOR)
continue;
- tree rhs = gimple_assign_rhs1 (stmt);
+ tree rhs = gimple_assign_rhs1 (assign);
if (!VECTOR_TYPE_P (TREE_TYPE (rhs))
|| maybe_ne (TYPE_VECTOR_SUBPARTS (TREE_TYPE (rhs)),
CONSTRUCTOR_NELTS (rhs))
@@ -3029,7 +3021,7 @@ vect_slp_check_for_constructors (bb_vec_info bb_vinfo)
|| uniform_vector_p (rhs))
continue;
- stmt_vec_info stmt_info = bb_vinfo->lookup_stmt (stmt);
+ stmt_vec_info stmt_info = bb_vinfo->lookup_stmt (assign);
BB_VINFO_GROUPED_STORES (bb_vinfo).safe_push (stmt_info);
}
}
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index cdd6f6c5e5d..766598862d4 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -1342,7 +1342,7 @@ vect_init_vector_1 (vec_info *vinfo, stmt_vec_info stmt_vinfo, gimple *new_stmt,
else
{
bb_vec_info bb_vinfo = dyn_cast <bb_vec_info> (vinfo);
- gimple_stmt_iterator gsi_region_begin = bb_vinfo->region_begin;
+ gimple_stmt_iterator gsi_region_begin = bb_vinfo->begin ().gsi;
gsi_insert_before (&gsi_region_begin, new_stmt, GSI_SAME_STMT);
}
}
diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
index 6c830ad09f4..921bae680b2 100644
--- a/gcc/tree-vectorizer.h
+++ b/gcc/tree-vectorizer.h
@@ -787,12 +787,82 @@ loop_vec_info_for_loop (class loop *loop)
typedef class _bb_vec_info : public vec_info
{
public:
+ struct const_iterator
+ {
+ const_iterator (gimple_stmt_iterator _gsi) : gsi (_gsi) {}
+
+ const const_iterator &
+ operator++ ()
+ {
+ gsi_next (&gsi); return *this;
+ }
+
+ gimple *operator* () const { return gsi_stmt (gsi); }
+
+ bool
+ operator== (const const_iterator& other) const
+ {
+ return gsi_stmt (gsi) == gsi_stmt (other.gsi);
+ }
+
+ bool
+ operator!= (const const_iterator& other) const
+ {
+ return !(*this == other);
+ }
+
+ gimple_stmt_iterator gsi;
+ };
+
+ const_iterator begin () const { return const_iterator (m_region_begin); }
+ const_iterator end () const { return const_iterator (m_region_end); }
+
+ struct reverse_iterator
+ {
+ reverse_iterator (gimple_stmt_iterator _gsi) : gsi (_gsi) {}
+
+ const reverse_iterator &
+ operator++ ()
+ {
+ gsi_prev (&gsi); return *this;
+ }
+
+ gimple *operator* () const { return gsi_stmt (gsi); }
+
+ bool
+ operator== (const reverse_iterator& other) const
+ {
+ return gsi_stmt (gsi) == gsi_stmt (other.gsi);
+ }
+
+ bool
+ operator!= (const reverse_iterator& other) const
+ {
+ return !(*this == other);
+ }
+
+ gimple_stmt_iterator gsi;
+ };
+
+ reverse_iterator rbegin () const
+ {
+ reverse_iterator it = reverse_iterator (m_region_end);
+ if (*it == NULL)
+ return reverse_iterator (gsi_last_bb (m_region_end.bb));
+ else
+ return it;
+ }
+
+ reverse_iterator rend () const { return reverse_iterator (m_region_begin); }
+
_bb_vec_info (gimple_stmt_iterator, gimple_stmt_iterator, vec_info_shared *);
~_bb_vec_info ();
basic_block bb;
- gimple_stmt_iterator region_begin;
- gimple_stmt_iterator region_end;
+
+private:
+ gimple_stmt_iterator m_region_begin;
+ gimple_stmt_iterator m_region_end;
} *bb_vec_info;
#define BB_VINFO_BB(B) (B)->bb
--
2.26.2
