Hello,
I've only noticed a couple typos, and one minor remark. From my
perspective it's okay, but you still need the okay of a proper reviewer,
for which you might want to state the testing/regression state of this
patch relative to trunk. The remarks follow:
On Tue, 22 Oct 2019, Feng Xue OS wrote:
> > > +/* Suppose one condition branch, led by SKIP_HEAD, is not executed since
> > > + certain iteration of LOOP, check whether an SSA name (NAME) remains
> > > + unchanged in next interation. We call this characterisic as semi-
"iteration", "characteristic", remove "as"
> > > + /* Not consider redefinitions in excluded basic blocks. */
"Don't consider"
> > > + /* It is unnecessary to evaluate expression of the conditional
> > > statement
> > > + in new loop that contains only invariant branch. This expresion
> > > should
"expression"
> > > @@ -662,6 +1383,32 @@ tree_ssa_split_loops (void)
> > > }
> > > }
> > >
> > > + if (changed)
> > > + {
> > > + cleanup_tree_cfg ();
> > > + changed = false;
> > > + }
> > > +
> > > + /* Perform loop splitting for suitable if-conditions in all loops. */
> > > + FOR_EACH_LOOP (loop, LI_INCLUDE_ROOT)
> > > + loop->aux = NULL;
> > > +
> > > + FOR_EACH_LOOP (loop, LI_FROM_INNERMOST)
> > > + {
> > > + if (loop->aux)
> > > + {
> > > + loop_outer (loop)->aux = loop;
> > > + continue;
> > > + }
> > > +
> > > + if (!optimize_loop_for_size_p (loop)
> > > + && split_loop_on_cond (loop))
> > > + {
> > > + loop_outer (loop)->aux = loop;
> > > + changed = true;
> > > + }
> > > + }
> > > +
> > > FOR_EACH_LOOP (loop, LI_INCLUDE_ROOT)
> > > loop->aux = NULL;
I just wonder why you duplicated these three loops instead of integrating
the real body into the existing LI_FROM_INNERMOST loop. I would have
expected your "if (!optimize_loop_for_size_p && split_loop_on_cond)" block
to simply be the else block of the existing
"if (... conditions for normal loop splitting ...)" block.
Either way it's okay with me.
Ciao,
Michael.
