From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa4.mentor.iphmx.com (esa4.mentor.iphmx.com [68.232.137.252]) by sourceware.org (Postfix) with ESMTPS id 73F4A385842C for ; Fri, 30 Jul 2021 07:18:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 73F4A385842C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com IronPort-SDR: L2Qmdjd08xrGSJvYOtIqDgZhrMBdYZAcuJZqsAFNQPxaY4liFLjretk+Y/ulpowmukNporFjgO RnrvgKQV7/40cYVHtKKULtOUfQwnJ6PRguqiwjkLp4efL8EOnlV7NzrI9vq+ILvw1URvIIaIk4 RdUkkqdcKTuIPkqko9Qu1B23PMfObblJlUQ4f8PV8w0s+DBYdYwFDtT5HIu4A3PLVLtct63cLA A9mfvkVJW1V8NY/b/IU1S3srWJGqR2qvvao0Lh/hZ1qNTLaDjQhJvGGan1p8z9zanY/Vt73rL0 ibLBkK6Y7jHNsxAjH17edGk+ X-IronPort-AV: E=Sophos;i="5.84,281,1620720000"; d="scan'208";a="64265323" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa4.mentor.iphmx.com with ESMTP; 29 Jul 2021 23:18:34 -0800 IronPort-SDR: snl/9/ySpM4NtmaqsAo6EhlLaYdgo6DByFIh3iI7ooPpMYxA4iC4ivbLfxdtV6GfKiAWoDhmcV N8gZbCJyail2o0jkBHki4fLm6d2dLsaAqwLulEeadqH6jF+6AxdBi+QiZFYeS3hBydcVbr65N/ BOyS+XclJwqJoLrGpmHkj8kddlPIq+c15OrSOoHWEuNON1fnp1uN9PvaIW+jiTHyOUE7f9Dhsi 4XlAqKZZxZbojQTrDdPgIJqxwYNMZWfxacJ5pVElVbiDGVLFTFcwffj2RHJn5dRC+++JRxwEDH bVE= From: Thomas Schwinge To: CC: Martin Sebor , , Jakub Jelinek , Jonathan Wakely , "Segher Boessenkool" , Richard Sandiford , Bill Schmidt , Subject: Re: [PATCH v4] Use range-based for loops for traversing loops In-Reply-To: References: <0a8b77ba-1d54-1eff-b54d-d2cb1e769e09@linux.ibm.com> <79a93b1f-6417-234c-a785-fdc598dfb92c@linux.ibm.com> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/27.1 (x86_64-pc-linux-gnu) Date: Fri, 30 Jul 2021 09:18:19 +0200 Message-ID: <878s1o8e2s.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: SVR-IES-MBX-07.mgc.mentorg.com (139.181.222.7) To svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Jul 2021 07:18:37 -0000 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 =3D NULL; > signed char *moved_orig_loop_num =3D 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 =3D=3D 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_cfu= n, basic_block entry_bb, /* Fix up orig_loop_num. If the block referenced in it has been mov= ed to dest_cfun, update orig_loop_num field, otherwise clear it. */ - class loop *dloop =3D NULL; + class loop *dloop; signed char *moved_orig_loop_num =3D 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 =3D=3D 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 =3D 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 =3D loop->num; > if (loop->header =3D=3D 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 !=3D NULL) > { > @@ -1723,7 +1718,7 @@ verify_loop_structure (void) > err =3D 1; > } > > - FOR_EACH_LOOP (loop, 0) > + for (auto loop : loops_list (cfun, 0)) > { > eloops =3D 0; > for (exit =3D loop->exits->next; exit->e; exit =3D 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 =3D 0; edge e; unsigned num =3D number_of_loops (cfun); @@ -1690,7 +1689,7 @@ verify_loop_structure (void) for (; exit; exit =3D exit->next_e) eloops++; - for (loop =3D bb->loop_father; + for (class loop *loop =3D bb->loop_father; loop !=3D 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, bo= ol 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 =3D 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 =3D ipa_fn_summaries->get (node); - class loop *loop; unsigned max_loop_predicates =3D opt_for_fn (node->decl, param_ipa_max_loop_predica= tes); @@ -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 =3D loops_for_fn (cfun)->tree_root->inner; + for (class loop *loop =3D loops_for_fn (cfun)->tree_root->inner; loop !=3D NULL; loop =3D loop->next) { predicate loop_stride =3D 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 ins= ide > 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 =3D 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 =3D false; + class loop *loop; FOR_EACH_VEC_ELT (*get_loops (cfun), i, loop) if (loop && loop->header =3D=3D 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 =3D=3D NULL) > { > loop->aux =3D xcalloc (1, sizeof (class loop_data)); > @@ -2203,7 +2203,7 @@ calculate_loop_reg_pressure (void) > bitmap_release (&curr_regs_live); > if (flag_ira_region =3D=3D IRA_REGION_MIXED > || flag_ira_region =3D=3D 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 =3D=3D NULL) > return; > - FOR_EACH_LOOP (loop, 0) > + for (auto loop : loops_list (cfun, 0)) > { > parent =3D 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 =3D=3D NULL) @@ -2151,7 +2151,7 @@ calculate_loop_reg_pressure (void) if (curr_loop =3D=3D current_loops->tree_root) continue; - for (loop =3D curr_loop; + for (class loop *loop =3D curr_loop; loop !=3D current_loops->tree_root; loop =3D 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 =3D 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 with_recursion(10); @@ -1941,7 +1941,7 @@ predict_loops (void) && (decl =3D gimple_call_fndecl (gsi_stmt (gsi))) !=3D NULL && recursive_call_p (current_function_decl, decl)) { - loop =3D bb->loop_father; + class loop *loop =3D bb->loop_father; while (loop && !with_recursion.add (loop)) loop =3D 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 restric= t > 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 =3D false; basic_block bb; control_dependences *cd =3D NULL; @@ -3384,6 +3383,7 @@ loop_distribution::execute (function *fun) /* Destroy loop bodies that could not be reused. Do this late a= s 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 =3D 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 =3D 0; unsigned int vect_loops_num; - class loop *loop; hash_table *simduid_to_vf_htab =3D NULL; hash_table *simd_array_to_simduid_htab =3D NU= LL; bool any_ifcvt_loops =3D false; @@ -1256,7 +1255,7 @@ vectorize_loops (void) if (any_ifcvt_loops) for (i =3D 1; i < number_of_loops (cfun); i++) { - loop =3D get_loop (cfun, i); + class loop *loop =3D get_loop (cfun, i); if (loop && loop->dont_vectorize) { gimple *g =3D vect_loop_vectorized_call (loop); @@ -1282,7 +1281,7 @@ vectorize_loops (void) loop_vec_info loop_vinfo; bool has_mask_store; - loop =3D get_loop (cfun, i); + class loop *loop =3D get_loop (cfun, i); if (!loop || !loop->aux) continue; loop_vinfo =3D (loop_vec_info) loop->aux; Gr=C3=BC=C3=9Fe Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955