The following adds a ginsert_iterator class to reduce repetition
(most visible in the gimple_build API, see 2/3) and confusion
with respect to the iterator update parameter. In 3/3 you'll
see how adjustments play out eventually, I'd expect uses like
auto gii = ginsert_iterator::after (stmt);
gii.insert (stmt2);
tree val = gimple_build (&gii, ...);
but as you can see I make use of the fact that ginsert_iterator
is also a gimple_iterator in 3/3, so like gsi_replace (..) can
still be done on it. I have sofar refrained from adding a
default CTOR, plastered one ugly use without much thinking
by initializing with itself, IMO the use needs refactoring.
That said, I'm sending this RFC to start some bikeshedding,
in particular I'm not sure I like
gii.insert (...);
versus gsi_insert (&gii, ...). At least I did _not_ like
*gii = ...;
even though I like *gsi vs. gsi_stmt (gsi).
I do like the static method initializers, since IMO speaking
names are better than 'false'/'true' parameters. I'm not
so sure about the overloading, so ::start_stmt vs. ::start
There is currently no way to overload insert and insert_seq
because both gimple * and gimple_seq are the same type
(we should fix that, wrap gimple * in a struct for gimple_seq).
Comments?
I did not quickly find a place where it's very obviously
improving readability of code much (but the gimple_build API,
which is where I first thought of this). Maybe it just adds
choice and confusion which wouldn't be good (we can always
have an AI refactor away all remaining uses of gsi_insert_*
of course).
Thanks,
Richard.
* gimple-iterator.h (ginsert_iterator): New class.
---
gcc/gimple-iterator.cc | 38 ++++++++++++++++++++
gcc/gimple-iterator.h | 80 ++++++++++++++++++++++++++++++++++++------
2 files changed, 108 insertions(+), 10 deletions(-)
diff --git a/gcc/gimple-iterator.cc b/gcc/gimple-iterator.cc
index b6cf30aea6a..f2547ed7db1 100644
--- a/gcc/gimple-iterator.cc
+++ b/gcc/gimple-iterator.cc
@@ -1104,3 +1104,41 @@ gsi_safe_insert_seq_before (gimple_stmt_iterator *iter,
gimple_seq seq)
else
gsi_insert_seq_before (iter, seq, GSI_SAME_STMT);
}
+
+void
+ginsert_iterator::insert (gimple *stmt)
+{
+ if (m_before)
+ {
+ if (m_without_update)
+ gsi_insert_before_without_update (this, stmt, m_update);
+ else
+ gsi_insert_before (this, stmt, m_update);
+ }
+ else
+ {
+ if (m_without_update)
+ gsi_insert_after_without_update (this, stmt, m_update);
+ else
+ gsi_insert_after (this, stmt, m_update);
+ }
+}
+
+void
+ginsert_iterator::insert_seq (gimple_seq seq)
+{
+ if (m_before)
+ {
+ if (m_without_update)
+ gsi_insert_seq_before_without_update (this, seq, m_update);
+ else
+ gsi_insert_seq_before (this, seq, m_update);
+ }
+ else
+ {
+ if (m_without_update)
+ gsi_insert_seq_after_without_update (this, seq, m_update);
+ else
+ gsi_insert_seq_after (this, seq, m_update);
+ }
+}
diff --git a/gcc/gimple-iterator.h b/gcc/gimple-iterator.h
index 0c8acd94dfd..bace58fbda3 100644
--- a/gcc/gimple-iterator.h
+++ b/gcc/gimple-iterator.h
@@ -20,6 +20,16 @@ along with GCC; see the file COPYING3. If not see
#ifndef GCC_GIMPLE_ITERATOR_H
#define GCC_GIMPLE_ITERATOR_H
+enum gsi_iterator_update
+{
+ GSI_NEW_STMT = 2, /* Move the iterator to the first statement added. */
+ GSI_LAST_NEW_STMT, /* Move the iterator to the last statement added. */
+ GSI_SAME_STMT, /* Leave the iterator at the same statement. */
+ GSI_CONTINUE_LINKING /* Move iterator to whatever position is suitable
+ for linking other statements in the same
+ direction. */
+};
+
/* Iterator object for GIMPLE statement sequences. */
struct gimple_stmt_iterator
@@ -48,16 +58,6 @@ struct gphi_iterator : public gimple_stmt_iterator
}
};
-enum gsi_iterator_update
-{
- GSI_NEW_STMT = 2, /* Move the iterator to the first statement added. */
- GSI_LAST_NEW_STMT, /* Move the iterator to the last statement added. */
- GSI_SAME_STMT, /* Leave the iterator at the same statement. */
- GSI_CONTINUE_LINKING /* Move iterator to whatever position is suitable
- for linking other statements in the same
- direction. */
-};
-
extern void gsi_insert_seq_before_without_update (gimple_stmt_iterator *,
gimple_seq,
enum gsi_iterator_update);
@@ -435,4 +435,64 @@ gimple_seq_nondebug_singleton_p (gimple_seq seq)
return gsi_one_nondebug_before_end_p (gsi);
}
+struct ginsert_iterator : public gimple_stmt_iterator
+{
+ ginsert_iterator (const gimple_stmt_iterator &gsi, bool before,
+ enum gsi_iterator_update update, bool without_update)
+ : gimple_stmt_iterator (gsi), m_before (before),
+ m_without_update (without_update), m_update (update) {}
+
+ /* Always forward-insertion, thus GSI_SAME_STMT for before and
+ GSI_CONTINUE_LINKING for after. */
+ static ginsert_iterator before (gimple_stmt_iterator gsi)
+ {
+ return ginsert_iterator (gsi, true, GSI_SAME_STMT, false);
+ }
+ static ginsert_iterator before (gimple *stmt)
+ {
+ return ginsert_iterator (gsi_for_stmt (stmt),
+ true, GSI_SAME_STMT, false);
+ }
+ static ginsert_iterator before_without_update (gimple *stmt)
+ {
+ return ginsert_iterator (gsi_for_stmt (stmt),
+ true, GSI_SAME_STMT, true);
+ }
+ static ginsert_iterator after (gimple_stmt_iterator gsi)
+ {
+ return ginsert_iterator (gsi, false, GSI_CONTINUE_LINKING, false);
+ }
+ static ginsert_iterator after (gimple *stmt)
+ {
+ return ginsert_iterator (gsi_for_stmt (stmt),
+ false, GSI_CONTINUE_LINKING, false);
+ }
+ static ginsert_iterator after_without_update (gimple *stmt)
+ {
+ return ginsert_iterator (gsi_for_stmt (stmt),
+ false, GSI_CONTINUE_LINKING, true);
+ }
+ static ginsert_iterator after_labels (basic_block);
+
+ /* Always append? */
+ static ginsert_iterator phis (basic_block);
+
+ /* Always append? Force through a sequence? */
+ static ginsert_iterator on_edge (edge);
+
+ static ginsert_iterator append (gimple_seq& seq)
+ {
+ return ginsert_iterator (gsi_last (seq),
+ false, GSI_CONTINUE_LINKING, true);
+ }
+
+ void insert (gimple *);
+ void insert_seq (gimple_seq);
+
+private:
+ bool m_before;
+ bool m_without_update;
+ gsi_iterator_update m_update : 8;
+};
+
#endif /* GCC_GIMPLE_ITERATOR_H */
--
2.51.0
