This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Put scope blocks on a diet


On Nov 28, 2007 1:39 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Nov 27, 2007, "Richard Guenther" <richard.guenther@gmail.com> wrote:
>
> >> Let's please revert the patch that introduced the bug in the first
> >> place.  Then we can make an assessment of how much the memory
> >> optimization patch actually gets us without anyone being confused by
> >> the distortions caused by the bug in the patch.
>
> > ?  So you mean comparing the current state with the !inline_completed
> > part removed to a version that doesn't remove unused scope blocks at all?
> > That is, what remains in memory savings?
>
> Yes.  Not being fooled by the excessive memory savings caused by the
> bug.

Ok, so I have numbers with

Index: tree-ssa-live.c
===================================================================
--- tree-ssa-live.c     (revision 130489)
+++ tree-ssa-live.c     (working copy)
@@ -501,11 +501,7 @@ remove_unused_scope_block_p (tree scope)
         only the used variables for cfgexpand's memory packing saving quite
         a lot of memory.  */
       else if (debug_info_level == DINFO_LEVEL_NORMAL
-              || debug_info_level == DINFO_LEVEL_VERBOSE
-              /* Removing declarations before inlining is going to affect
-                 DECL_UID that in turn is going to affect hashtables and
-                 code generation.  */
-              || !cfun->after_inlining)
+              || debug_info_level == DINFO_LEVEL_VERBOSE)
        unused = false;

       else

on top of current trunk (which includes your patch).  And (no surprise here)
the regression is completely gone (remember, this is a non-debug build).

> > This particular part of the patch is just a workaround, and as you don't
> > have a testcase that breaks (apart from make bootstrap-debug)
>
> On what grounds do you disregard maek bootstrap-debug as a testcase?
> Do you know of any other mechanisms we have to detect changes in
> generated code caused by enabling debug information?

Well, a testcase is something that can be checked in the testsuite to
see if it regresses (I admit that -g vs. -g0 mismatches are very sensitive
to changes anywhere in the compiler and come and go once you start
reducing a testcase).

> > Nevertheless memory-usage for non-debug builds of applications are
> > an existing issue.
>
> I never said it wasn't.  I just don't think it justifies breaking the
> -g/-g0 principle.

Well, if we want to make that priciple "absolut", the only sane way to
implement it is to just always follow the same path during compilation
(that is, do as if -g was enabled) and only at the point of debug output
do nothing.  _That_ would fix it for sure - and would be more maintainable
as well.  Just - it might cause memory-usage and compile-time regressions
for formerly non-debug builds.

I don't see us taking the -g/-g0 principle to that level unless creating
debug information is _way_ cheaper than now.  (If the regression
for non-debug would be minor then I would be all for it)

> > I have a technically more sound patch (those were already posted as
> > queued for 4.4), and I am going to see if they fix the make bootstra-debug
> > regression.
>
> Is it going to cause decl uids to vary with or without debug info?
> This would be very unfortunate and IMHO highly undesirable.

Yes and no.  Code generated shouldn't depend on varying UIDs.  In
principle bootstrap comparison should survive even if we did

    DECL_UID (t) = next_decl_uid;
    next_decl_uid += random() % 13;

anything else I consider a bug in how we use DECL_UIDs.

That the DECL_UIDs printed in the dumps would be different with/without
debug info is unfortunate, but you can make it less worse by making some
room for extra UIDs per function (that is, "rounding" to the next 10000
before emitting the next function).

But surely you now won't make consistent DECL_UIDs in debug dump
-g/-g0 a correctness issue?

> > So if you consider that regression important enough I'd rather
> > go with a real fix than a workaround with the memory penalty.
>
> That's fine with me.  I'm not opposed to memory savings, obviously.
> What I'm opposed to is sacrificing fundamental requirements for the
> sake of memory savings.

If it is that fundamental the only way you can prove that code isn't different
-g vs. -g0 is to always go the debug path during compilation.  Anything else
doesn't make sense - or the issue isn't as black-and-white as you suggest.

> >> Also, considering how much scope information that patch causes us to
> >> unintentionally discard, do we consciously want to introduce such a
> >> major debug information regression in GCC 4.3, even with -O0?
>
> > I am not proposing to revert this part of the patch.
>
> I didn't see anyone saying "please revert one part of the patch".  I
> saw people panicing and jumping to demand the whole patch to be
> reverted, before it was even understood that this would bringing back
> the far more fundamental regression the patch was meant to fix.

Well, you disregard the discussion we had on IRC and you disregard
mails sent after the inital request to revert (the whole) patch.

I'll now try that make bootstrap-debug.

Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]