Re: [PATCH v4] Use range-based for loops for traversing loops

[Thomas Schwinge <thomas@codesourcery.com>]

Hi!

Thanks for this nice clean-up.

Curious why in some instances we're not removing the 'class loop *loop'
declaration, I had a look, and this may suggest some further clean-up?
(See below; so far, all untested!)


But first, is this transformation correct?

> --- a/gcc/tree-cfg.c
> +++ b/gcc/tree-cfg.c

> @@ -7752,9 +7747,9 @@ move_sese_region_to_fn (struct function *dest_cfun, basic_block entry_bb,
>
>    /* Fix up orig_loop_num.  If the block referenced in it has been moved
>       to dest_cfun, update orig_loop_num field, otherwise clear it.  */
> -  class loop *dloop;
> +  class loop *dloop = NULL;
>    signed char *moved_orig_loop_num = NULL;
> -  FOR_EACH_LOOP_FN (dest_cfun, dloop, 0)
> +  for (class loop *dloop : loops_list (dest_cfun, 0))
>      if (dloop->orig_loop_num)
>        {
>       if (moved_orig_loop_num == NULL)

We've got the original outer 'dloop' and a new separate inner 'dloop'
inside the 'for'.  The outer one now is only ever 'NULL'-initialized --
but then meant to be used in later code (not shown here)?  (I cannot
claim to understand this later code usage of 'dloop', though.  Maybe even
is there a pre-existing problem here?  Or, it's still too early on Friday
morning.)  If there is an actual problem, would the following restore the
original behavior?

    --- gcc/tree-cfg.c
    +++ gcc/tree-cfg.c
    @@ -7747,9 +7747,9 @@ move_sese_region_to_fn (struct function *dest_cfun, basic_block entry_bb,

       /* Fix up orig_loop_num.  If the block referenced in it has been moved
          to dest_cfun, update orig_loop_num field, otherwise clear it.  */
    -  class loop *dloop = NULL;
    +  class loop *dloop;
       signed char *moved_orig_loop_num = NULL;
    -  for (class loop *dloop : loops_list (dest_cfun, 0))
    +  for (dloop : loops_list (dest_cfun, 0))
         if (dloop->orig_loop_num)
           {
        if (moved_orig_loop_num == NULL)


Second, additional clean-up possible as follows?

> --- a/gcc/cfgloop.c
> +++ b/gcc/cfgloop.c

> @@ -1457,7 +1452,7 @@ verify_loop_structure (void)
>    auto_sbitmap visited (last_basic_block_for_fn (cfun));
>    bitmap_clear (visited);
>    bbs = XNEWVEC (basic_block, n_basic_blocks_for_fn (cfun));
> -  FOR_EACH_LOOP (loop, LI_FROM_INNERMOST)
> +  for (auto loop : loops_list (cfun, LI_FROM_INNERMOST))
>      {
>        unsigned n;
>
> @@ -1503,7 +1498,7 @@ verify_loop_structure (void)
>    free (bbs);
>
>    /* Check headers and latches.  */
> -  FOR_EACH_LOOP (loop, 0)
> +  for (auto loop : loops_list (cfun, 0))
>      {
>        i = loop->num;
>        if (loop->header == NULL)
> @@ -1629,7 +1624,7 @@ verify_loop_structure (void)
>      }
>
>    /* Check the recorded loop exits.  */
> -  FOR_EACH_LOOP (loop, 0)
> +  for (auto loop : loops_list (cfun, 0))
>      {
>        if (!loop->exits || loop->exits->e != NULL)
>       {
> @@ -1723,7 +1718,7 @@ verify_loop_structure (void)
>         err = 1;
>       }
>
> -      FOR_EACH_LOOP (loop, 0)
> +      for (auto loop : loops_list (cfun, 0))
>       {
>         eloops = 0;
>         for (exit = loop->exits->next; exit->e; exit = exit->next)

    --- gcc/cfgloop.c
    +++ gcc/cfgloop.c
    @@ -1398,7 +1398,6 @@ verify_loop_structure (void)
     {
       unsigned *sizes, i, j;
       basic_block bb, *bbs;
    -  class loop *loop;
       int err = 0;
       edge e;
       unsigned num = number_of_loops (cfun);
    @@ -1690,7 +1689,7 @@ verify_loop_structure (void)
              for (; exit; exit = exit->next_e)
                eloops++;

    -         for (loop = bb->loop_father;
    +         for (class loop *loop = bb->loop_father;
                   loop != e->dest->loop_father
                   /* When a loop exit is also an entry edge which
                      can happen when avoiding CFG manipulations

> --- a/gcc/ipa-fnsummary.c
> +++ b/gcc/ipa-fnsummary.c
> @@ -2923,7 +2923,7 @@ analyze_function_body (struct cgraph_node *node, bool early)
>        if (dump_file && (dump_flags & TDF_DETAILS))
>       flow_loops_dump (dump_file, NULL, 0);
>        scev_initialize ();
> -      FOR_EACH_LOOP (loop, 0)
> +      for (auto loop : loops_list (cfun, 0))
>       {
>         predicate loop_iterations = true;
>         sreal header_freq;

    --- gcc/ipa-fnsummary.c
    +++ gcc/ipa-fnsummary.c
    @@ -2916,7 +2916,6 @@ analyze_function_body (struct cgraph_node *node, bool early)
       if (nonconstant_names.exists () && !early)
         {
           ipa_fn_summary *s = ipa_fn_summaries->get (node);
    -      class loop *loop;
           unsigned max_loop_predicates = opt_for_fn (node->decl,
                                                 param_ipa_max_loop_predicates);

    @@ -2960,7 +2959,7 @@ analyze_function_body (struct cgraph_node *node, bool early)
           /* To avoid quadratic behavior we analyze stride predicates only
              with respect to the containing loop.  Thus we simply iterate
         over all defs in the outermost loop body.  */
    -      for (loop = loops_for_fn (cfun)->tree_root->inner;
    +      for (class loop *loop = loops_for_fn (cfun)->tree_root->inner;
           loop != NULL; loop = loop->next)
        {
          predicate loop_stride = true;

> --- a/gcc/loop-init.c
> +++ b/gcc/loop-init.c

> @@ -229,7 +228,7 @@ fix_loop_structure (bitmap changed_bbs)
>       loops, so that when we remove the loops, we know that the loops inside
>       are preserved, and do not waste time relinking loops that will be
>       removed later.  */
> -  FOR_EACH_LOOP (loop, LI_FROM_INNERMOST)
> +  for (auto loop : loops_list (cfun, LI_FROM_INNERMOST))
>      {
>        /* Detect the case that the loop is no longer present even though
>           it wasn't marked for removal.

    --- gcc/loop-init.c
    +++ gcc/loop-init.c
    @@ -201,7 +201,6 @@ fix_loop_structure (bitmap changed_bbs)
     {
       basic_block bb;
       int record_exits = 0;
    -  class loop *loop;
       unsigned old_nloops, i;

       timevar_push (TV_LOOP_INIT);
    @@ -279,6 +278,7 @@ fix_loop_structure (bitmap changed_bbs)

       /* Finally free deleted loops.  */
       bool any_deleted = false;
    +  class loop *loop;
       FOR_EACH_VEC_ELT (*get_loops (cfun), i, loop)
         if (loop && loop->header == NULL)
           {

> --- a/gcc/loop-invariant.c
> +++ b/gcc/loop-invariant.c
> @@ -2136,7 +2136,7 @@ calculate_loop_reg_pressure (void)
>    rtx link;
>    class loop *loop, *parent;
>
> -  FOR_EACH_LOOP (loop, 0)
> +  for (auto loop : loops_list (cfun, 0))
>      if (loop->aux == NULL)
>        {
>       loop->aux = xcalloc (1, sizeof (class loop_data));
> @@ -2203,7 +2203,7 @@ calculate_loop_reg_pressure (void)
>    bitmap_release (&curr_regs_live);
>    if (flag_ira_region == IRA_REGION_MIXED
>        || flag_ira_region == IRA_REGION_ALL)
> -    FOR_EACH_LOOP (loop, 0)
> +    for (auto loop : loops_list (cfun, 0))
>        {
>       EXECUTE_IF_SET_IN_BITMAP (&LOOP_DATA (loop)->regs_live, 0, j, bi)
>         if (! bitmap_bit_p (&LOOP_DATA (loop)->regs_ref, j))
> @@ -2217,7 +2217,7 @@ calculate_loop_reg_pressure (void)
>        }
>    if (dump_file == NULL)
>      return;
> -  FOR_EACH_LOOP (loop, 0)
> +  for (auto loop : loops_list (cfun, 0))
>      {
>        parent = loop_outer (loop);
>        fprintf (dump_file, "\n  Loop %d (parent %d, header bb%d, depth %d)\n",

    --- gcc/loop-invariant.c
    +++ gcc/loop-invariant.c
    @@ -2134,7 +2134,7 @@ calculate_loop_reg_pressure (void)
       basic_block bb;
       rtx_insn *insn;
       rtx link;
    -  class loop *loop, *parent;
    +  class loop *parent;

       for (auto loop : loops_list (cfun, 0))
         if (loop->aux == NULL)
    @@ -2151,7 +2151,7 @@ calculate_loop_reg_pressure (void)
           if (curr_loop == current_loops->tree_root)
        continue;

    -      for (loop = curr_loop;
    +      for (class loop *loop = curr_loop;
           loop != current_loops->tree_root;
           loop = loop_outer (loop))
        bitmap_ior_into (&LOOP_DATA (loop)->regs_live, DF_LR_IN (bb));

> --- a/gcc/predict.c
> +++ b/gcc/predict.c
> @@ -1949,7 +1949,7 @@ predict_loops (void)
>
>    /* Try to predict out blocks in a loop that are not part of a
>       natural loop.  */
> -  FOR_EACH_LOOP (loop, LI_FROM_INNERMOST)
> +  for (auto loop : loops_list (cfun, LI_FROM_INNERMOST))
>      {
>        basic_block bb, *bbs;
>        unsigned j, n_exits = 0;

    --- gcc/predict.c
    +++ gcc/predict.c
    @@ -1927,7 +1927,6 @@ predict_extra_loop_exits (edge exit_edge)
     static void
     predict_loops (void)
     {
    -  class loop *loop;
       basic_block bb;
       hash_set <class loop *> with_recursion(10);

    @@ -1941,7 +1941,7 @@ predict_loops (void)
            && (decl = gimple_call_fndecl (gsi_stmt (gsi))) != NULL
            && recursive_call_p (current_function_decl, decl))
          {
    -       loop = bb->loop_father;
    +       class loop *loop = bb->loop_father;
            while (loop && !with_recursion.add (loop))
              loop = loop_outer (loop);
          }

> --- a/gcc/tree-loop-distribution.c
> +++ b/gcc/tree-loop-distribution.c
> @@ -3312,7 +3312,7 @@ loop_distribution::execute (function *fun)
>
>    /* We can at the moment only distribute non-nested loops, thus restrict
>       walking to innermost loops.  */
> -  FOR_EACH_LOOP (loop, LI_ONLY_INNERMOST)
> +  for (auto loop : loops_list (cfun, LI_ONLY_INNERMOST))
>      {
>        /* Don't distribute multiple exit edges loop, or cold loop when
>           not doing pattern detection.  */

    --- gcc/tree-loop-distribution.c
    +++ gcc/tree-loop-distribution.c
    @@ -3293,7 +3293,6 @@ prepare_perfect_loop_nest (class loop *loop)
     unsigned int
     loop_distribution::execute (function *fun)
     {
    -  class loop *loop;
       bool changed = false;
       basic_block bb;
       control_dependences *cd = NULL;
    @@ -3384,6 +3383,7 @@ loop_distribution::execute (function *fun)
           /* Destroy loop bodies that could not be reused.  Do this late as we
         otherwise can end up refering to stale data in control dependences.  */
           unsigned i;
    +      class loop *loop;
           FOR_EACH_VEC_ELT (loops_to_be_destroyed, i, loop)
        destroy_loop (loop);


> --- a/gcc/tree-vectorizer.c
> +++ b/gcc/tree-vectorizer.c
> @@ -1194,7 +1194,7 @@ vectorize_loops (void)
>    /* If some loop was duplicated, it gets bigger number
>       than all previously defined loops.  This fact allows us to run
>       only over initial loops skipping newly generated ones.  */
> -  FOR_EACH_LOOP (loop, 0)
> +  for (auto loop : loops_list (cfun, 0))
>      if (loop->dont_vectorize)
>        {
>       any_ifcvt_loops = true;
> @@ -1213,7 +1213,7 @@ vectorize_loops (void)
>                 loop4 (copy of loop2)
>               else
>                 loop5 (copy of loop4)
> -        If FOR_EACH_LOOP gives us loop3 first (which has
> +        If loops' iteration gives us loop3 first (which has
>          dont_vectorize set), make sure to process loop1 before loop4;
>          so that we can prevent vectorization of loop4 if loop1
>          is successfully vectorized.  */

    --- gcc/tree-vectorizer.c
    +++ gcc/tree-vectorizer.c
    @@ -1172,7 +1172,6 @@ vectorize_loops (void)
       unsigned int i;
       unsigned int num_vectorized_loops = 0;
       unsigned int vect_loops_num;
    -  class loop *loop;
       hash_table<simduid_to_vf> *simduid_to_vf_htab = NULL;
       hash_table<simd_array_to_simduid> *simd_array_to_simduid_htab = NULL;
       bool any_ifcvt_loops = false;
    @@ -1256,7 +1255,7 @@ vectorize_loops (void)
       if (any_ifcvt_loops)
         for (i = 1; i < number_of_loops (cfun); i++)
           {
    -   loop = get_loop (cfun, i);
    +   class loop *loop = get_loop (cfun, i);
        if (loop && loop->dont_vectorize)
          {
            gimple *g = vect_loop_vectorized_call (loop);
    @@ -1282,7 +1281,7 @@ vectorize_loops (void)
           loop_vec_info loop_vinfo;
           bool has_mask_store;

    -      loop = get_loop (cfun, i);
    +      class loop *loop = get_loop (cfun, i);
           if (!loop || !loop->aux)
        continue;
           loop_vinfo = (loop_vec_info) loop->aux;


Grüße
 Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955