On 18/06/20 16:25 +0200, Martin Liška wrote:
On 6/18/20 11:43 AM, Jonathan Wakely wrote:
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.
Nice catch from Jonathan!
I'm leaving the idea of std::reverse_iterator and make_iterator_range and I'm
testing
a patch version that I'll install (if there are no objections).
Yes, the std::reverse_iterator idea is a dead end, and I agree that
make_iterator_range doesn't seem like an improvement.
