Re: PR 23551: why should we coalesce inlined variables?

[Alexandre Oliva <aoliva at redhat dot com>]

On Jul  9, 2007, Andrew MacLeod <amacleod@redhat.com> wrote:

> On Mon, 2007-07-09 at 14:44 -0300, Alexandre Oliva wrote:

>> 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.

But it's the same test condition, and the same testcase, after some
minimal manipulation.  This is enough of a reason for me to coalesce
it.  http://gcc.gnu.org/ml/gcc-patches/2007-07/msg00739.html

> You aren't following the whole thing then. I posted an example which
> explained it a little better a few minutes ago.

It still irrecoverably dropped the same bit of information, causing
information for foo to be generated incorrectly.

> You are completely ignoring the entire live range splitting issue
> which it addresses as well,

No, I'm bringing up *another* relevant point that has so far been
dismissed.

> Your simple example doesn't even cause any problems currently
> because we DONT coalesce foo and bar since they are user variables.

We do when foo is an inlined variable, and that's one of the testcases
I have at hand.

And then, even if it was not an inlined variable, we should coalesce
them, to avoid using 2GB of memory instead of 1GB in a very similar
testcase to the one that started all this uproar after I checked the
patch in.

> 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'm first getting at addressing fundamental design problems in which
we discard information that we need to generate correct debug info.
That's what the foo_3 assignment is about.  That's all my proposed
designs and the initial patch were meant to address.  I'm trying to
solve the problem I was asked to solve.

> 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.

With these PHI assignments in BB headers, outside the normal insn
flow, I have an intuition that the fact that the pseudos are assigned
to PHIs wouldn't matter at all.  But I haven't thought it through.

>> No, what I'm doing ATM is all planning.  I'm not making any decisions.

> the patch is tactical.

The patch is *not* part of the plan.  Not at all.  It was a first
attempt, that didn't work, but that fixed an inconsistency in the way
we dealt with user variables and that was shown to improve performance
in a number of cases and to not increase memory use significantly
before the report that came up after it was checked in.

>> 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.

This would only happen if I started coding before the design was
thought through.  I'm not doing that.

> 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.

Yup, that's how one does design, and that's how I'm approaching it.

>> 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?

Not dropping variables of inlined functions on the ground.  Retaining
correct debug information for them.

> I see it as 'improve debug info'

You see the PR number in the subject?  That's strongly related with
Red Hat Bugzilla #239065 and Sourceware (systemtap) bug 1155.

> and the first place to start are the big items which involves
> separating the root variable from the debug symbol. Everything
> evolves from there.

Yup.  Guess how I got to the idea of not coalescing out of SSA, and
emitting one pseudo per SSA?  At some point I got the impression *you*
were suggesting something along these lines, and that was before I
brought it up, FWIW.

> 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.

This will only work if I can recover information on at what point (or
points) each user variable was set to that value, and at what point
(or points) it stopped holding that value.  This is the point I need
to solve to be able to address the problems I have at hand.  And so
far your suggestions don't address that problem at all, because the
ranges are wrong for all variables that undergo copy-propagation.

> 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 already have a problem that your proposal isn't enough to fix.

> 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.

Yup.  Been there, done that, got misunderstood.

> Thats what Im talking about with the offer to assist.

Thanks for that, BTW.  Really appreciated.  Honestly (in case
it might somehow be taken as ironic)

> 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.

See way above, it's the very same issue.

-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
FSF Latin America Board Member         http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}