Hi again,
On Sat, Feb 08, 2020 at 11:54:48AM +0100, Uros Bizjak wrote:
> On Fri, Feb 7, 2020 at 5:41 PM Segher Boessenkool
> <seg...@kernel.crashing.org> wrote:
> > On Thu, Feb 06, 2020 at 12:13:35PM +0100, Uros Bizjak wrote:
> > > The names of split_before_sched2 ("split4") and split_before_regstack
> > > ("split3") do not reflect their insertion point in the sequence of passes,
> > > where split_before_regstack follows split_before_sched2. Reorder the code
> > > and rename the passes to reflect the reality.
> >
> > Renaming them to other splitN doesn't help much :-/ Having stable names
> > is more important (some archs actually use these names), I'd say. But
>
> True, this is why I took care to found and fix all references to
> existing names. It is a simple substitution of old name with a new.
Yeah, and reading the patch is a bit challenging ;-) If we had real
names here, this would be simpler. Something for the future.
> I'm not familiar with tree passes, how the situation is handled there.
> If a new instance of the same pass is inserted before existing pass,
> does name of the existing pass get changed?
I don't know either.
> > > +bool
> > > +pass_split_before_regstack::gate (function *)
> > > +{
> > > +#if HAVE_ATTR_length && defined (STACK_REGS)
> > > + /* If flow2 creates new instructions which need splitting
> > > + and scheduling after reload is not done, they might not be
> > > + split until final which doesn't allow splitting
> > > + if HAVE_ATTR_length. */
> > > + return !enable_split_before_sched2 ();
> > > +#else
> > > + return false;
> > > +#endif
> > > +}
> >
> > flow.c was deleted in 2006... Is this split still needed at all? If
> > so, change the comment please? :-)
>
> Hm, I don't know in general, but x86 needs a late split at the point
> where instantiated registers won't change in the insn.
Right, but the explanation given in the comment is very out of date.
> As you suggested in the other mail, the issue with many late split
> passes (passes 3-5) should be rewiewed, considering also other (e.g.
> non-stack regs) targets.
Yeah. But that is not a stage 4 thing. Your patch is safe for stage 4
as far as I can see.
Segher
