diff mbox

[google/gcc-4_7,trunk] Fix problem with -fdebug-types-section and template instantiations

Message ID 20120826020806.F0D2DE041B@ccoutant.mtv.corp.google.com
State New
Headers show

Commit Message

Cary Coutant Aug. 26, 2012, 2:08 a.m. UTC
This patch is for trunk and the google/gcc-4_7 branch.

When a class template instantiation is moved into a separate type unit,
it can bring along a lot of other referenced types into the type unit,
especially if the template is derived from another (large) type that
does not have an actually have a type definition in a type unit of its
own. When there are many instantiations of the same template, we get
a lot of duplication, and in the worst case (a template with several
parameters, instantiated multiple times along each dimension), GCC
can end up taking a long time and exhausting available memory. Because
instantiations typically happen in .cc files, and the resulting type
definition is unlikely to be useful as a comdat type unit, this patch
avoids the problem by choosing not to move template instantiations
into type units.

Bootstraps and passes regression tests. Also tested with a large
internal test case that previously resulted in out-of-memory during
compilation.

Google ref b/7041390.


2012-08-25   Cary Coutant  <ccoutant@google.com>

gcc/
	* dwarf2out.c (is_template_instantiation): New function.
	(should_move_die_to_comdat): Reject types that are template
	instantiations.

Comments

Cary Coutant Aug. 26, 2012, 2:10 a.m. UTC | #1
[dropping the bogus address I accidentally typed on the command line]

On Sat, Aug 25, 2012 at 7:08 PM, Cary Coutant <ccoutant@google.com> wrote:
> This patch is for trunk and the google/gcc-4_7 branch.
>
> When a class template instantiation is moved into a separate type unit,
> it can bring along a lot of other referenced types into the type unit,
> especially if the template is derived from another (large) type that
> does not have an actually have a type definition in a type unit of its
> own. When there are many instantiations of the same template, we get
> a lot of duplication, and in the worst case (a template with several
> parameters, instantiated multiple times along each dimension), GCC
> can end up taking a long time and exhausting available memory. Because
> instantiations typically happen in .cc files, and the resulting type
> definition is unlikely to be useful as a comdat type unit, this patch
> avoids the problem by choosing not to move template instantiations
> into type units.
>
> Bootstraps and passes regression tests. Also tested with a large
> internal test case that previously resulted in out-of-memory during
> compilation.
>
> Google ref b/7041390.
>
>
> 2012-08-25   Cary Coutant  <ccoutant@google.com>
>
> gcc/
>         * dwarf2out.c (is_template_instantiation): New function.
>         (should_move_die_to_comdat): Reject types that are template
>         instantiations.
>
>
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c     (revision 190681)
> +++ gcc/dwarf2out.c     (working copy)
> @@ -7277,6 +7277,21 @@ contains_subprogram_definition (dw_die_r
>    return 0;
>  }
>
> +/* Return non-zero if this DIE represents a template instantiation.  */
> +
> +static int
> +is_template_instantiation (dw_die_ref die)
> +{
> +  dw_die_ref c;
> +
> +  FOR_EACH_CHILD (die, c,
> +                 if (c->die_tag == DW_TAG_template_type_param
> +                     || c->die_tag == DW_TAG_template_value_param
> +                     || c->die_tag == DW_TAG_GNU_template_template_param)
> +                   return 1);
> +  return 0;
> +}
> +
>  /* Return non-zero if this is a type DIE that should be moved to a
>     COMDAT .debug_types section.  */
>
> @@ -7294,6 +7309,7 @@ should_move_die_to_comdat (dw_die_ref di
>        if (is_declaration_die (die)
>            || get_AT (die, DW_AT_abstract_origin)
>            || is_nested_in_subprogram (die)
> +         || is_template_instantiation (die)
>           || contains_subprogram_definition (die))
>          return 0;
>        return 1;
Cary Coutant Aug. 28, 2012, 5:31 p.m. UTC | #2
>> 2012-08-25   Cary Coutant  <ccoutant@google.com>
>>
>> gcc/
>>         * dwarf2out.c (is_template_instantiation): New function.
>>         (should_move_die_to_comdat): Reject types that are template
>>         instantiations.

I'm withdrawing this patch. While it does solve the problem in the
specific test case I had at hand, I've run some tests on other code
and found that overall it seems to increase debug size. I'm seeing
fewer type units and bigger .debug_info (as expected) but the total
size of the (fewer) type units is much larger than before -- most
likely due to non-template types that refer to template types, which
now must be copied into each type unit that references them.

I'll need a more surgical fix to this, but I'm having trouble creating
a small test case that demonstrates the problem I'm seeing in the
pathological test case I have, so it's been a real pain to analyze.

-cary
Cary Coutant Aug. 29, 2012, 1:07 a.m. UTC | #3
> I'll need a more surgical fix to this, but I'm having trouble creating
> a small test case that demonstrates the problem I'm seeing in the
> pathological test case I have, so it's been a real pain to analyze.

OK, I think I have a reasonably small test case that illustrates the
problem. Here we have a base class B that contains a class S. Add to
that a class template D derived from B, that contains a class SS
derived from S. Now add a function template in B whose template
parameter is one of the various SS subclasses:

$ cat t1.cc
class B {
 public:
  B();
  virtual ~B();
  virtual void set(int) = 0;
  struct S {
    int s1;
  };
  virtual S* new_s() = 0;
  template <typename T>
  T* get()
  { return static_cast<T*>(new_s()); }
  S* s_;
};

template <int a, int b>
class D : public B {
 public:
  const int a_ = a;
  const int b_ = b;
  D() : B() { }
  virtual ~D();
  int val() { return get<SS>()->s2; }
  virtual void set(int) { }
  struct SS : public S {
    int s2;
  };
  virtual SS* new_s() { return new SS; }
};

int foo()
{
  D<1, 1> d1;
  D<1, 2> d2;
  return d1.val() + d2.val();
}
$ g++ --std=c++11 -c -g -fdebug-types-sections t1.cc
$ readelf -wi t1.o

In the type unit for each of the instantiations of D, we'll find a
copy of the declaration for class B with all the instantiated member
functions -- B::get<D<1, 1>::SS> and B::get<D<1, 2>::SS> -- in each
copy. Scale this up to a base class with many such member functions,
and a derived class template instantiated many times across four
template value parameters, and GCC explodes.

I think the problem is that when copying the declaration for B into
the type units for D<1, 1> and D<1, 2>, we're also copying B's
children. I don't think we need to do that -- it should be sufficient
to have a single DW_TAG_class_type DIE for B with DW_AT_declaration
set, and no children. If the type unit for D<x, x> needs to reference
any of B's children, copy_decls_walk will find those and install them
under the declaration for B as needed.

I'm testing a patch now.

-cary
diff mbox

Patch

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 190681)
+++ gcc/dwarf2out.c	(working copy)
@@ -7277,6 +7277,21 @@  contains_subprogram_definition (dw_die_r
   return 0;
 }
 
+/* Return non-zero if this DIE represents a template instantiation.  */
+
+static int
+is_template_instantiation (dw_die_ref die)
+{
+  dw_die_ref c;
+
+  FOR_EACH_CHILD (die, c,
+		  if (c->die_tag == DW_TAG_template_type_param
+		      || c->die_tag == DW_TAG_template_value_param
+		      || c->die_tag == DW_TAG_GNU_template_template_param)
+		    return 1);
+  return 0;
+}
+
 /* Return non-zero if this is a type DIE that should be moved to a
    COMDAT .debug_types section.  */
 
@@ -7294,6 +7309,7 @@  should_move_die_to_comdat (dw_die_ref di
       if (is_declaration_die (die)
           || get_AT (die, DW_AT_abstract_origin)
           || is_nested_in_subprogram (die)
+	  || is_template_instantiation (die)
 	  || contains_subprogram_definition (die))
         return 0;
       return 1;