On 6/12/20 1:43 PM, Richard Sandiford wrote:
Martin Liška <mli...@suse.cz> writes:
On 6/12/20 12:46 PM, Richard Sandiford wrote:
Martin Liška <mli...@suse.cz> writes:
Hello.

I'm working one extension of SLP which will allow to vectorize multiple
BBs. This is a first step where I abstract _bb_vec_info::region_begin and
_bb_vec_info::region end by providing an iterator.

Nice.

diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
index 6c830ad09f4..542d49402d2 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)
+    {
+    }
+
+    const_iterator &
+    operator++ (int)
+    {
+      gsi_next (&gsi); return *this;
+    }

Isn't this really operator++()?

It is, but post-increment ++ operators have one integer argument (so that
one can distinguish them from pre-increment operators:
https://en.cppreference.com/w/cpp/language/operator_incdec)

Sure, but what I mean by:

I.e. it returns a reference to the
modified iterator instead of the value at the original iterator.

is that the above implements the semantics of a preincrement, not a
postincrement, so it should be operator++() rather than operator++(int).

Ah, ok, I've got it.


In other words, the function (rightly) implements “++it” rather than “it++”.

With that fixed, it seems like a range-based for loop should work.

Yes, it works. The syntax is really neat:

for (gimple *stmt: *bb_vinfo)

I've tested vect.exp and i386.exp. May I install the patch after proper testing?

Thanks,
Martin


Thanks,
Richard


>From 977c96f27cbb6e6423c2546837ab50371df5f9d9 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-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.
	* tree-vectorizer.h: Add the new iterator and all related functions.
---
 gcc/tree-vect-patterns.c | 14 ++------------
 gcc/tree-vect-slp.c      | 28 ++++++++++------------------
 gcc/tree-vect-stmts.c    |  2 +-
 gcc/tree-vectorizer.h    | 38 ++++++++++++++++++++++++++++++++++++--
 4 files changed, 49 insertions(+), 33 deletions(-)

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));
     }
 }
 
@@ -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;
diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index 303410c2fc4..8646d7f77ab 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..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)
+    {
+    }
+
+    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); }
+
   _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

Reply via email to