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. > @@ -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. > 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? 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. Thanks, Richard