On 18/06/20 00:55 +0100, Jonathan Wakely wrote:
Does your reverse iterator work correctly? It looks to me like it will
fail to visit the region_begin statement, because this loop will
terminate when the reverse iterator reaches the end() value, and will
not actually process that gsi:

-      gimple_stmt_iterator si = bb_vinfo->region_end;
-      gimple *stmt;
-      do
+      for (gimple *stmt : bb_vinfo->reverse_region_stmts ())

Adding const_iterator::operator--() and using
std::reverse_iterator<const_iterator> would fix that problem. The
std::reverse_iterator utility already solves the problem of getting
the begin and end of a reversed range correct.

You might need to add a couple of things to your const_iterator to
make it more like a conventional STL iterator in order to use
std::reverse_iterator, but IMHO it would be worth it. I can look at
that in the morning.

Using std::reverse_iterator won't work, because this iterator is an
"input iterator" i.e. single pass. Incrementing it modifies the
underlying data structure (the gimple_statement_iterator) so it can't
meet the requirements for a bidirectional iterator.

But fixing the reverse iterator to visit the last statement isn't
hard.

N.B. the fact that the bug in reverse iteration wasn't noticed
suggests missing tests :-)


Reply via email to