Martin Liška <[email protected]> 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
