PR 23551: why should we coalesce inlined variables?

[Andrew MacLeod]

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

> > Think of it as coalescing running twice if you want.  once one trees
> > and once on rtl.
> 
> Problem is that this initial coalescing has already been shown to get
> us worse performance in a number of situations.  But what do I know?
> 

we look at addressing them. Separating the root variable from the debug
symbol is the first step and opens up all kinds of opportunities, as
well as getting more things right right off the bat.


> I'm not heading down any particular path yet.  I'm probing
> possibilities and discussing some pipe dreams along with immediate
> needs.  I guess this is as unclear as the fact that I'm not opposing
> the reversal of the patch.  Quite the contrary.  I've posted one that
> goes even further than that, addressing the same memory explosion
> problem for the case in which that horrible testcase has all function
> calls inlined by hand.

Don't coalesce tasks. Revert the patch, and submit the memory savings
one on its own merit.  I don't even remember seeing it.

> > I suggested a different approach which will make gobs of
> > improvements on the debug info without affecting the compiler
> > whatsoever.
> 
> So far I see the proposed improvements result in incorrect debug
> information, which heavy users of debug information told me is worse
> than not having debug information at all.  So, more design is needed
> before I can start coding.

You aren't following the whole thing then. I posted an example which
explained it a little better a few minutes ago. If involved tracking
current def for debug info as we are emitting stuff. There is only ever
one current def.  And regardless, the end result is still better than
what we have today. You are completely ignoring the entire live range
splitting issue which it addresses as well, and I suspect is more of an
issue than your example. Your simple example doesn't even cause any
problems currently because we DONT coalesce foo and bar since they are
user variables.  So I think we probably get them right now anyway. Thats
why I want an example of a real testcase. It easy to make things up, but
you need to actually see that in out of ssa for it to be an actual
issue.

Since we havent really addressed debug info much, i think there is a lot
of low hanging fruit, and my argument is you are making a big grand
sweeping design and claims without ever have investigated how good or
bad what we have can be with some long overdue TLC. I wont deny we
havent paid enough attention to it, and it was on my next 12 months list
to address.

 
> 
> >> 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.
> 
> Nah, very few RTL optimizations work across BBs.

You are glossing over the wrinkles. there are in fact global
optimizations, and with the new DF infrastructure, maybe you will see
more. and many basic block optimizations start with a value which will
now be a PHI.

> 
> > You are basing your assumptions that these things will be resolved
> > in time.
> 
> I don't see where this impression comes from.  To me they're very
> unrelated.

The stop mixing the things you are talking about and stay on point.
talking about not coalescing things in out of ssa in your debug plan
certainly leads to that assumption.
 
> 
> >> 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.
> 
> No, what I'm doing ATM is all planning.  I'm not making any decisions.

the patch is tactical.

> I'm just past gathering requirements.  Now I'm trying to figure out
> how to retain the information I need within basic blocks.
> 
> When that's figured out, I expect figuring out how to perform
> confluence of PHI nodes will be nearly trivial.  If it's not, I'll
> work harder on that.  But until I have a plan for stuff to work within
> basic blocks, I don't see the point of trying to address the more
> complex scenarios.

Because then you discover, oh, if I had of done it this way, I wouldn't
have this problem I have to now hack around. You might not need need to
figure out every single end case (which would be nice), but you
certainly need to consider the major players, and PHI nodes are a major
players in SSA.


> 
> > 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.
> 
> Yes, I'm exploring that path, but I'm trying to fit it with the
> requirements before me, and I'm failing at that.

and what are the requirements before you?  I see it as 'improve debug
info' and the first place to start are the big items which involves
separating the root variable from the debug symbol. Everything evolves
from there. 

> 
> > Once that part is done, we can work on addressing further bits of
> > debug info that incorrect and need to be addressed.
>  
> Do you realize that this is contrary to the very "plan ahead" approach
> that you suggested to me, and that planning ahead is precisely what
> I'm trying to do right now?

here you are being argumentative again. The ability to associate a debug
name with an ssa_name which is different than the root name will enable
you to do so many things, that in the end, you likely only have really
difficult cases that are difficult to forsee. Just doing this will give
us so much better debug info for a minimal amount of work, and its hard
to see what the remaining problems are without more detailed
investigation.   I also presume that before implementing it, one would
think about it more than this cursory discussion involves and come back
and say, 'hey, what about this case', and we figure out how to handle
it.  Thats what Im talking about with the offer to assist.


>  
> > I would like to see the patch reverted,
> 
> No disagreement here, but can you please comment on the patch I
> proposed to revert and supersede the one I checked in?  It's not like
> there's an urgency in reverting that one before the revised one is
> even commented on, is there?

Use 2 patches for 2 tasks. revert the patch. submit new one for
consideration.  acceptance or rejection of the second patch shouldn't
affect the reversion of the first one.  I dont remember seeing it, so
repost it on its own merit.


> 
> > especially since we have seen a demonstration of significant
> > negative side effects.
> 
> Which applies to other scenarios that the proposed patch will also
> address.  Want me to come up with a testcase that shows the same
> explosion regardless of the patch I've checked in, and that the
> pending patch I proposed will reduce memory use as much as reverting
> the checked-in patch would do to the testcase that brought up all this
> discussion?

Im not even sure what you are saying here.  that we have explosion
problems in some cases? yes. do we want patches to fix them? yes. do I
want  a testcase that shows that if I revert a patch, I can still
produce the same explosion a different way? no.  supply a patch to fix
the memory explosion which is unrelated to the patch that needs
reverting.  its not *just* the explosion that I want it reverted for. I
have never been convinced it should go in as is.  I was considering it,
but hadn't seen anything to convince me it made a difference.

Andrew


>