"Kewen.Lin" <li...@linux.ibm.com> writes:
> diff --git a/gcc/function.c b/gcc/function.c
> index 2c8fa217f1f..3e92ee9c665 100644
> --- a/gcc/function.c
> +++ b/gcc/function.c
> @@ -4841,6 +4841,8 @@ allocate_struct_function (tree fndecl, bool abstract_p)
> binding annotations among them. */
> cfun->debug_nonbind_markers = lang_hooks.emits_begin_stmt
> && MAY_HAVE_DEBUG_MARKER_STMTS;
> +
> + cfun->pending_TODOs = 0;
The field is cleared on allocation. I think it would be better
to drop this, to avoid questions about why other fields aren't
similarly zero-initialised.
> }
>
> /* This is like allocate_struct_function, but pushes a new cfun for FNDECL
> diff --git a/gcc/function.h b/gcc/function.h
> index d55cbddd0b5..ffed6520bf9 100644
> --- a/gcc/function.h
> +++ b/gcc/function.h
> @@ -269,6 +269,13 @@ struct GTY(()) function {
> /* Value histograms attached to particular statements. */
> htab_t GTY((skip)) value_histograms;
>
> + /* Different from normal TODO_flags which are handled right at the
> + begin or the end of one pass execution, the pending_TODOs are
beginning
> + passed down in the pipeline until one of its consumers can
> + perform the requested action. Consumers should then clear the
> + flags for the actions that they have taken. */
> + unsigned int pending_TODOs;
> +
> /* For function.c. */
>
> /* Points to the FUNCTION_DECL of this function. */
> […]
> diff --git a/gcc/tree-ssa-loop-ivcanon.c b/gcc/tree-ssa-loop-ivcanon.c
> index 298ab215530..9a9076cee67 100644
> --- a/gcc/tree-ssa-loop-ivcanon.c
> +++ b/gcc/tree-ssa-loop-ivcanon.c
> @@ -1411,6 +1411,13 @@ tree_unroll_loops_completely_1 (bool
> may_increase_size, bool unroll_outer,
> bitmap_clear (father_bbs);
> bitmap_set_bit (father_bbs, loop_father->header->index);
> }
> + else if (unroll_outer
> + && !(cfun->pending_TODOs
> + & PENDING_TODO_force_next_scalar_cleanup))
> + {
> + /* Trigger scalar cleanup once any outermost loop gets unrolled. */
> + cfun->pending_TODOs |= PENDING_TODO_force_next_scalar_cleanup;
> + }
I can see it would make sense to test whether the flag is already set
if we were worried about polluting the cache line. But this test and
set isn't performance-sensitive, so I think it would be clearer to
remove the “&& …” part of the condition.
Nit: there should be no braces around the block, since it's a single
statement.
OK with those changes, thanks.
Richard
