PR 23551: why should we coalesce inlined variables?

[Andrew MacLeod]

On Mon, 2007-07-09 at 13:24 -0300, Alexandre Oliva wrote: 
> On Jul  9, 2007, Andrew MacLeod <amacleod@redhat.com> wrote:

> > You are kidding right?
> 
> Not in principle, no.  The problems of coalescing SSA variables and
> optimizing away copies for PHI nodes appear to be equivalent to those
> of register allocation.  I don't see good reasons to solve the same
> problems twice in the translation of the same function.
> 

You might not, but reality begs to differ. You can argue this until you
are blue in the face, but the fact of the matter is coalescing in out of
ssa is going nowhere for a very long time.  We have lots of passes which
run multiple times. Think of it as coalescing running twice if you want.
once one trees and once on rtl.

Those copies never existed before SSA. Going into SSA form created the
PHIS. Going out of SSA form removes the PHIs. Generating all those
copies just to coalesce them away later is completely pointless. We'll
consume lots of memory for the instructions, Every optimization which
follows will have to sort through a whole pile of unnecessary copies for
no reason, and it will undoubtedly handcuff some optimizations.  I'm not
sure you understand the scale of the number of copies that would be
introduced which are trivially removable.  Until we have the ability to
handle them, it'll simply kill compilation times.

So Im stating that regardless of principles, we simply cannot allow
those copies into the RTL stream.  Go ahead and try it if you want, the
change isn't significant. But be prepared to wait a long time for spec
to finish.


> > You better write us a new register allocator before doing that!
> 
> Probably.  Maybe it's worth pointing out that this bit about not
> performing these SSA transformations is unrelated to my current
> project of improving debug info, it's just a plan to improve overall
> optimization.
> 

No, I'm saying the path you are heading down lead to death, destruction,
and gnashing of teeth. I suggested a different approach which will make
gobs of improvements on the debug info without affecting the compiler
whatsoever. In fact, it has always been on the list of things to go and
do, but like so many things, never gotten to it.
> 
> 
> Or have the register allocator take PHI notes in BB entry points into
> account, inserting copies in case it doesn't manage to assign both
> pseudos to the same location.  Which, if SSA coalescing can do, then
> so can a good register allocator.

every other optimization will have to understand what a PHI node is. You
are talking about RTl-SSA. most of the reachings defs will originate as
PHI nodes. And fine, but we still can't do that until the RTL-SSA
project is finished. And as far as I know, no one has even started it.


> 
> > we are by no means in any state to allow those kinds of unnecceary
> > copies through.
> 
> I realize that.  Please don't get the impression that long-term pipe
> dreams are in any way related with the debug info project.
> 

But there IS a connection. You are basing your assumptions that these
things will be resolved in time.  Im saying you need a more pragmatic
approach. we can generate WAY better debug info without bending through
all these hoops and long term requirements which may never come about.
And its not hard. Lets start THERE.



> 
> 
> Yup.  But then, SSA makes things over basic blocks work like magic, so
> I'm working first on getting stuff right inside basic blocks, to then
> figure out how the PHI function handling ought to look like.
> 

You are making tactical decisions instead of strategic planning. This is
how RTL backend became the mess it is today. base every decision on the
here and now need.  I claim we need to plan this better. 

> 
> 
> > foo_1 = 
> > bar_2 = 
> > bar_4 - bar_2 + 1
> > foo_5 = bar_2 - 1
> 
> > when we coalesced foo_3 and bar_2, to bar_2, we update the debug
> > list for bar_2 to be 'foo' and 'bar'.  then in out of ssa, we issue
> > the debug info for both 'foo' and 'bar' to match the range of bar_2.
> 
> Do you realize that debug info will be wrong for foo in this case,
> because it will appear to hold the same value as bar_2 starting at the
> assignment to bar_2, while it should still hold the foo_1 value?  This
> is precisely one of the problems I'm trying to address.
> 

Which is why I said there is some twiddling to do with overlapping
ranges. its not HARD to get right, its just a matter of conflict
resolution for the cases which have multiple debug symbols associated
with an ssa_name.


\
> 
> > this is at a cost. NOW the live range of bar_2 goes past bar_4, so NOW
> > bar_2 and bar_4 have to be different variables and cannot be coalesced
> > together to 'bar' anymore.  So we are now using a third variable
> 
> > foo = 
> > bar = 
> > bar.1 = bar + 1
> > foo = bar - 1
> 
> And we have to ensure that debug info for bar will map to bar.1, not
> to bar, at the point bar.1 is initialized.
> 

And that is so trivial it require no additional comment from me.

So in summary, I disagreed with the patch in the first place, my issues
were not resolved, and in fact you said you couldn't find a reasonable
testcase for me that showed improvement. I have provided another
approach that is not so controversial, will provide much better info
than we have, and will not force changes on so much of the compiler, and
will have extremely minimal impact on compile time. I am happy to
provide assistance and such if you want.  Once that part is done, we can
work on addressing further bits of debug info that incorrect and need to
be addressed. 
 
I would like to see the patch reverted, especially since we have seen a
demonstration of significant negative side effects.

Andrew