[Bug pch/53880] New: [4.8 Regression] compile time regression when generating precompiled headers for boost

[Bug pch/53880] New: [4.8 Regression] compile time regression when generating precompiled headers for boost

[markus at trippelsdorf dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

             Bug #: 53880
           Summary: [4.8 Regression] compile time regression when
                    generating precompiled headers for boost
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: pch
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: markus@trippelsdorf.de


There is a huge compile time regression in 4.8 when one generates 
a precompiled header for boost's /math/special_functions.hpp.

gcc-4.7:
 % time c++ -c -o pch.hpp.gch /usr/include/boost/math/special_functions.hpp
c++ -c -o pch.hpp.gch /usr/include/boost/math/special_functions.hpp  2.26s user
0.13s system 99% cpu 2.407 total

gcc-4.8:
 % time c++ -c -o pch.hpp.gch /usr/include/boost/math/special_functions.hpp
c++ -c -o pch.hpp.gch /usr/include/boost/math/special_functions.hpp  172.42s
user 0.37s system 99% cpu 2:53.44 total

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[markus at trippelsdorf dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #1 from Markus Trippelsdorf <markus at trippelsdorf dot de> 2012-07-06 15:54:43 UTC ---
Boost version is 1.49.
(When I use --save-temps and use the resulting .ii file instead, the regression
is no longer reproducible)

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[markus at trippelsdorf dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

Markus Trippelsdorf <markus at trippelsdorf dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dodji at gcc dot gnu.org

--- Comment #2 from Markus Trippelsdorf <markus at trippelsdorf dot de> 2012-07-06 16:47:20 UTC ---
Started with rev.186977:

commit 611f1003dbf4ebb341c2eda0fcc0e058c421eb6b
Author: dodji <dodji@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Mon Apr 30 11:43:43 2012 +0000

    Switch -ftrack-macro-expansion=2 on by default.

    This switches the compiler to -ftrack-macro-expansion=2 by default.
...


Using -ftrack-macro-expansion=0 "fixes" the issue.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[markus at trippelsdorf dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #3 from Markus Trippelsdorf <markus at trippelsdorf dot de> 2012-07-08 07:08:13 UTC ---
Unfortunately jimis' patch from Bug 53525 doesn't help much:

c++ -c -o pch.hpp.gch /usr/include/boost/math/special_functions.hpp  156.69s
user 0.33s system 99% cpu 2:37.50 total

From "perf top":

 99.78%  cc1plus       [.] gt_pch_p_9line_maps
  0.04%  cc1plus       [.] htab_find_with_hash
  0.02%  cc1plus       [.] gt_pch_p_14lang_tree_node
  0.02%  libc-2.16.so  [.] fwrite_unlocked   ▒
  0.01%  libc-2.16.so  [.] __mempcpy
  0.01%  [kernel]      [k] do_timer 

Zoom into gt_pch_p_9line_maps:

       │         size_t l3 = (size_t)(((*x).info_macro).used);
       │         if ((*x).info_macro.maps != NULL) {
       │           size_t i3;
       │           for (i3 = 0; i3 != (size_t)(l3); i3++) {
  3.89 │109:   cmp    %rcx,%rbx
  3.69 │       nop
  3.15 │     ↓ je     188
  3.11 │112:   mov    0x18(%rbp),%rax
  3.04 │       add    $0x1,%rbx
       │           break;
       │         }
       │     }
       │
       │     void
       │     gt_pch_p_9line_maps (ATTRIBUTE_UNUSED void *this_obj,
  2.76 │11a:   lea    (%rbx,%rbx,4),%r15
  3.53 │       shl    $0x3,%r15
       │       {
       │         size_t l3 = (size_t)(((*x).info_macro).used);
       │         if ((*x).info_macro.maps != NULL) {
       │           size_t i3;
       │           for (i3 = 0; i3 != (size_t)(l3); i3++) {
       │             switch (((*x).info_macro.maps[i3]).reason ==
LC_ENTER_MACRO)
  3.46 │       lea    (%rax,%r15,1),%rdi
  3.42 │       cmpb   $0x4,0x4(%rdi)
  4.40 │     ↑ jne    100
       │               case 1:
       │                 {
       │                   size_t l4 = (size_t)(2 *
((*x).info_macro.maps[i3].d.macro).n_tokens);
       │                   {
       │                     union tree_node * x5 =
       │                       ((*x).info_macro.maps[i3].d.macro.macro) ?
HT_IDENT_TO_GCC_IDENT (HT_NODE (((*x).info_macro.maps[i3].d.macro.macro)))
  4.38 │       mov    0x8(%rdi),%rsi
  4.25 │       xor    %edx,%edx
  2.22 │       lea    -0x18(%rsi),%r8
  2.16 │       test   %rsi,%rsi
  2.68 │       cmovne %r8,%rdx
       │                     if ((void *)((*x).info_macro.maps) == this_obj)
  2.72 │       cmp    %rax,%r14
       │                 break;
       │               case 1:
       │                 {
       │                   size_t l4 = (size_t)(2 *
((*x).info_macro.maps[i3].d.macro).n_tokens);
       │                   {
       │                     union tree_node * x5 =
  2.62 │       mov    %rdx,0x18(%rsp)
       │                       ((*x).info_macro.maps[i3].d.macro.macro) ?
HT_IDENT_TO_GCC_IDENT (HT_NODE (((*x).info_macro.maps[i3].d.macro.macro)))
       │                     if ((void *)((*x).info_macro.maps) == this_obj)
  2.68 │     ↓ je     1a0
       │                       op (&(x5), cookie);
       │                     (*x).info_macro.maps[i3].d.macro.macro = (x5) ?
CPP_HASHNODE (GCC_IDENT_TO_HT_IDENT ((x5))) : NULL;
  2.68 │147:   lea    0x18(%rdx),%rsi
  2.85 │       xor    %eax,%eax
  3.39 │       test   %rdx,%rdx
  3.11 │       cmovne %rsi,%rax
  3.23 │       mov    %rax,0x8(%rdi)
       │                   }
       │                   if ((*x).info_macro.maps[i3].d.macro.macro_locations
!= NULL) {
  5.10 │       mov    0x18(%rbp),%rax
  4.93 │       add    %rax,%r15
  5.48 │       cmpq   $0x0,0x18(%r15)
  3.57 │     ↑ je     109
       │                     size_t i4;
       │                     for (i4 = 0; i4 != (size_t)(l4); i4++) {
       │                     }
       │                     if ((void *)((*x).info_macro.maps) == this_obj)
  3.20 │       cmp    %rax,%r14
  2.82 │     ↑ jne    109
       │                       op
(&((*x).info_macro.maps[i3].d.macro.macro_locations), cookie);
       │       mov    %rcx,0x8(%rsp)
       │       lea    0x18(%r15),%rdi
       │       mov    %r13,%rsi
       │     → callq  *%r12
       │       mov    0x8(%rsp),%rcx
       │       }

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.0

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |paolo.carlini at oracle dot
                   |                            |com

--- Comment #4 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-07-23 13:46:43 UTC ---
Dodji, just in case isn't clear already, this is the PR about
-ftrack-macro-expansion + PCHs which I mentioned in Prague...

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

Steven Bosscher <steven at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-07-23
                 CC|                            |steven at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #5 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-23 21:28:56 UTC ---
What are the callers of gt_pch_p_9line_maps?

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #6 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-23 22:46:19 UTC ---
FWIW this shows up in GCC's own libstdc++ PCHs also.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[markus at trippelsdorf dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #7 from Markus Trippelsdorf <markus at trippelsdorf dot de> 2012-07-24 06:49:57 UTC ---
(In reply to comment #5)
> What are the callers of gt_pch_p_9line_maps?

It is only called from gt_pch_nx_line_maps of ./gcc/gtype-desc.c (generated):

gt_pch_nx_line_maps (void *x_p)
{
  struct line_maps * const x = (struct line_maps *)x_p;
  if (gt_pch_note_object (x, x, gt_pch_p_9line_maps, gt_ggc_e_9line_maps))
    {
      {
        size_t l0 = (size_t)(((*x).info_ordinary).used);
        if ((*x).info_ordinary.maps != NULL) {
          size_t i0;
          for (i0 = 0; i0 != (size_t)(l0); i0++) {
            switch (((*x).info_ordinary.maps[i0]).reason == LC_ENTER_MACRO)
              {
              case 0:
                gt_pch_n_S ((*x).info_ordinary.maps[i0].d.ordinary.to_file);
                break;
              case 1:
                {
                  size_t l1 = (size_t)(2 *
((*x).info_ordinary.maps[i0].d.macro).n_tokens);
                  {
                    union tree_node * const x2 =
                      ((*x).info_ordinary.maps[i0].d.macro.macro) ?
HT_IDENT_TO_GCC_IDENT (HT_NODE (((*x).info_ordinary.maps[i0].d.macro.macro))) :
NULL;
                    gt_pch_n_9tree_node (x2);
                  }
                  if ((*x).info_ordinary.maps[i0].d.macro.macro_locations !=
NULL) {
                    size_t i1;
                    for (i1 = 0; i1 != (size_t)(l1); i1++) {
                    }
                    gt_pch_note_object
((*x).info_ordinary.maps[i0].d.macro.macro_locations, x, gt_pch_p_9line_maps,
gt_types_enum_last);
                  }
                }
                break;
              default:
                break;
              }
          }
          gt_pch_note_object ((*x).info_ordinary.maps, x, gt_pch_p_9line_maps,
gt_types_enum_last);
        }
      }
      {
        size_t l3 = (size_t)(((*x).info_macro).used);
        if ((*x).info_macro.maps != NULL) {
          size_t i3;
          for (i3 = 0; i3 != (size_t)(l3); i3++) {
            switch (((*x).info_macro.maps[i3]).reason == LC_ENTER_MACRO)
              {
              case 0:
                gt_pch_n_S ((*x).info_macro.maps[i3].d.ordinary.to_file);
                break;
              case 1:
                {
                  size_t l4 = (size_t)(2 *
((*x).info_macro.maps[i3].d.macro).n_tokens);
                  {
                    union tree_node * const x5 =
                      ((*x).info_macro.maps[i3].d.macro.macro) ?
HT_IDENT_TO_GCC_IDENT (HT_NODE (((*x).info_macro.maps[i3].d.macro.macro))) :
NULL;
                    gt_pch_n_9tree_node (x5);
                  }
                  if ((*x).info_macro.maps[i3].d.macro.macro_locations != NULL)
{
                    size_t i4;
                    for (i4 = 0; i4 != (size_t)(l4); i4++) {
                    }
                    gt_pch_note_object
((*x).info_macro.maps[i3].d.macro.macro_locations, x, gt_pch_p_9line_maps,
gt_types_enum_last);
                  }
                }
                break;
              default:
                break;
              }
          }
          gt_pch_note_object ((*x).info_macro.maps, x, gt_pch_p_9line_maps,
gt_types_enum_last);
        }
      }
    }
}

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[stevenb.gcc at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #8 from stevenb.gcc at gmail dot com <stevenb.gcc at gmail dot com> 2012-07-24 08:16:01 UTC ---
(In reply to comment #7)
I don't think it's really called from there. It should be called from
gt_pch_save. gt_pch_nx_line_maps only registers the function (for
pointer rewriting, AFAIU).

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #9 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-24 09:20:14 UTC ---
(In reply to comment #1)
> Boost version is 1.49.
> (When I use --save-temps and use the resulting .ii file instead, the regression
> is no longer reproducible)

That's of course because all preprocessor macros are already expanded and
written out to the .ii file :-)

I've posted this patch for -O0 builds:
http://gcc.gnu.org/ml/gcc-patches/2012-07/msg01174.html

I have no idea if it helps for "normal" optimized builds also. Perhaps you can
give it a try.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-07-24 09:34:21 UTC ---
Err, isn't the GTY annotation in

     as y1.  x0 is the spelling location for the argument token "1",
     and x2 is the spelling location for the argument token "2".  */
  source_location * GTY((length ("2 * %h.n_tokens"))) macro_locations;

simply pointless?

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[stevenb.gcc at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #11 from stevenb.gcc at gmail dot com <stevenb.gcc at gmail dot com> 2012-07-24 09:41:18 UTC ---
> Err, isn't the GTY annotation in
>
>      as y1.  x0 is the spelling location for the argument token "1",
>      and x2 is the spelling location for the argument token "2".  */
>   source_location * GTY((length ("2 * %h.n_tokens"))) macro_locations;
>
> simply pointless?

I don't think so. You still want to pointer-rewrite it if you output a PCH.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #12 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-07-24 09:42:53 UTC ---
(In reply to comment #11)
> > Err, isn't the GTY annotation in
> >
> >      as y1.  x0 is the spelling location for the argument token "1",
> >      and x2 is the spelling location for the argument token "2".  */
> >   source_location * GTY((length ("2 * %h.n_tokens"))) macro_locations;
> >
> > simply pointless?
> 
> I don't think so. You still want to pointer-rewrite it if you output a PCH.

The pointer to the array, but not the array elements.  So it's pointless
to know the length and

   souce_location * macro_locations;

should still rewrite the pointer itself, no?

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[stevenb.gcc at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #13 from stevenb.gcc at gmail dot com <stevenb.gcc at gmail dot com> 2012-07-24 10:03:05 UTC ---
On Tue, Jul 24, 2012 at 11:42 AM, rguenth at gcc dot gnu.org
<gcc-bugzilla@gcc.gnu.org> wrote:
> The pointer to the array, but not the array elements.  So it's pointless
> to know the length and
>
>    souce_location * macro_locations;
>
> should still rewrite the pointer itself, no?

Hmm.  I'm not sure. Without the annotation, how does the PCH machinery
know how long that array is? OTOH there isn't anything else, other
than those dead loops, that looks at h.n_tokens.

Perhaps there should be a warning from gengtype if the length
attribute is applied to a scalar type.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #14 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-07-24 10:13:24 UTC ---
Thanks Steven for looking into this!

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[dodji at seketeli dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #15 from dodji at seketeli dot org <dodji at seketeli dot org> 2012-07-26 16:30:33 UTC ---
"paolo.carlini at oracle dot com" <gcc-bugzilla@gcc.gnu.org> a écrit:

> --- Comment #4 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-07-23 13:46:43 UTC ---
> Dodji, just in case isn't clear already, this is the PR about
> -ftrack-macro-expansion + PCHs which I mentioned in Prague...

Woops, I am looking at this just now, sorry.  Thank you Paolo for the
head-up.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[dodji at seketeli dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #16 from dodji at seketeli dot org <dodji at seketeli dot org> 2012-07-26 17:15:15 UTC ---
"rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org> a écrit:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880
>
> --- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-07-24 09:34:21 UTC ---
> Err, isn't the GTY annotation in
>
>      as y1.  x0 is the spelling location for the argument token "1",
>      and x2 is the spelling location for the argument token "2".  */
>   source_location * GTY((length ("2 * %h.n_tokens"))) macro_locations;
>
> simply pointless?

In theory, I would say yes it should be pointless.

But in practise, it looks like the "gengtype code output system" needs a
length annotation for it because macro_location is a pointer to
something that is not a struct, basically.

Otherwise, calling gengtype on the resulting gtype.state yields:

gcc/fix-master/gcc/../libcpp/include/line-map.h:168: field
`(*x).info_ordinary.maps[i0].d.macro.macro_locations' is pointer to
unimplemented type

This is because with no annotation, the information for
"macro_locations" that we have in the generated gtype.state is:

(!pair  "macro_locations"
 (!type already_seen 6)
 (!srcfileloc  "../libcpp/include/line-map.h" 168)
 nil)

With the gengtype length annotation, we have:

(!pair  "macro_locations"
 (!type already_seen 6)
 (!srcfileloc  "../libcpp/include/line-map.h" 168)
 (!options 
  (!option length string  "2 * %h.n_tokens")))

The important thing to notice there is that in the latest case, there is
a "length" option (or attribute) that is present in the description.

Then looking at the walk_type function in gengtype.c, we see:

    case TYPE_POINTER:
      {
        ...

        if (!length)
      {
        if (!UNION_OR_STRUCT_P (t->u.p)
        && t->u.p->kind != TYPE_PARAM_STRUCT)
          {
        error_at_line (d->line,
                   "field `%s' is pointer to unimplemented type",
                   d->val);
        break;
          }
          ...

The 'length' variable not being set to zero there is a consequence of no
gengtype length annotation being present in the line-map.h source code.

So my question would be, why is gengtype forcing pointers to e.g,
scalars to have a length annotation?

If this wasn't the case, I think the expensive empty loops wouldn't be
generated in the first place, would they?

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[dodji at seketeli dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #16 from dodji at seketeli dot org <dodji at seketeli dot org> 2012-07-26 17:15:15 UTC ---
"rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org> a écrit:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880
>
> --- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-07-24 09:34:21 UTC ---
> Err, isn't the GTY annotation in
>
>      as y1.  x0 is the spelling location for the argument token "1",
>      and x2 is the spelling location for the argument token "2".  */
>   source_location * GTY((length ("2 * %h.n_tokens"))) macro_locations;
>
> simply pointless?

In theory, I would say yes it should be pointless.

But in practise, it looks like the "gengtype code output system" needs a
length annotation for it because macro_location is a pointer to
something that is not a struct, basically.

Otherwise, calling gengtype on the resulting gtype.state yields:

gcc/fix-master/gcc/../libcpp/include/line-map.h:168: field
`(*x).info_ordinary.maps[i0].d.macro.macro_locations' is pointer to
unimplemented type

This is because with no annotation, the information for
"macro_locations" that we have in the generated gtype.state is:

(!pair  "macro_locations"
 (!type already_seen 6)
 (!srcfileloc  "../libcpp/include/line-map.h" 168)
 nil)

With the gengtype length annotation, we have:

(!pair  "macro_locations"
 (!type already_seen 6)
 (!srcfileloc  "../libcpp/include/line-map.h" 168)
 (!options 
  (!option length string  "2 * %h.n_tokens")))

The important thing to notice there is that in the latest case, there is
a "length" option (or attribute) that is present in the description.

Then looking at the walk_type function in gengtype.c, we see:

    case TYPE_POINTER:
      {
        ...

        if (!length)
      {
        if (!UNION_OR_STRUCT_P (t->u.p)
        && t->u.p->kind != TYPE_PARAM_STRUCT)
          {
        error_at_line (d->line,
                   "field `%s' is pointer to unimplemented type",
                   d->val);
        break;
          }
          ...

The 'length' variable not being set to zero there is a consequence of no
gengtype length annotation being present in the line-map.h source code.

So my question would be, why is gengtype forcing pointers to e.g,
scalars to have a length annotation?

If this wasn't the case, I think the expensive empty loops wouldn't be
generated in the first place, would they?

--- Comment #17 from dodji at seketeli dot org <dodji at seketeli dot org> 2012-07-26 17:15:43 UTC ---
> --- Comment #14 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-07-24 10:13:24 UTC ---
> Thanks Steven for looking into this!

Indeed, thank you Steven!

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[dodji at seketeli dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #17 from dodji at seketeli dot org <dodji at seketeli dot org> 2012-07-26 17:15:43 UTC ---
> --- Comment #14 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-07-24 10:13:24 UTC ---
> Thanks Steven for looking into this!

Indeed, thank you Steven!

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[dodji at seketeli dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #18 from dodji at seketeli dot org <dodji at seketeli dot org> 2012-07-26 17:18:34 UTC ---
> --- Comment #13 from stevenb.gcc at gmail dot com <stevenb.gcc at gmail dot com> 2012-07-24 10:03:05 UTC ---
> On Tue, Jul 24, 2012 at 11:42 AM, rguenth at gcc dot gnu.org
> <gcc-bugzilla@gcc.gnu.org> wrote:
>> The pointer to the array, but not the array elements.  So it's pointless
>> to know the length and
>>
>>    souce_location * macro_locations;
>>
>> should still rewrite the pointer itself, no?
>
> Hmm.  I'm not sure. Without the annotation, how does the PCH machinery
> know how long that array is? OTOH there isn't anything else, other
> than those dead loops, that looks at h.n_tokens.
>
> Perhaps there should be a warning from gengtype if the length
> attribute is applied to a scalar type.

As I said in a previous comment, I'd say no length attribute should even
be needed here.  In this particular case, no empty loop would have been
generated in the first place.

I would still apply the patch that you cooked (and mentioned in another
comment) to be sure we don't get bitten by such empty loops being
otherwise generated.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[stevenb.gcc at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #19 from stevenb.gcc at gmail dot com <stevenb.gcc at gmail dot com> 2012-07-26 19:44:59 UTC ---
Dodji, please see http://gcc.gnu.org/ml/gcc-patches/2012-07/msg01204.html

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[dodji at seketeli dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #20 from dodji at seketeli dot org <dodji at seketeli dot org> 2012-07-26 21:11:16 UTC ---
"stevenb.gcc at gmail dot com" <gcc-bugzilla@gcc.gnu.org> a écrit:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880
>
> --- Comment #19 from stevenb.gcc at gmail dot com <stevenb.gcc at gmail dot com> 2012-07-26 19:44:59 UTC ---
> Dodji, please see
> http://gcc.gnu.org/ml/gcc-patches/2012-07/msg01204.html

Ah, right.

I missed that patch.  It all looks good to me.

Thank you.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

Steven Bosscher <steven at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING

--- Comment #21 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-30 09:11:27 UTC ---
Markus, I think that r189951 should have fixed this problem (xf. 
http://gcc.gnu.org/viewcvs?view=revision&revision=189951), although I suspect
there will still be a compile time increase from GCC 4.7 to GCC 4.8 because of
the macro expansion tracking stuff. Could you please try a recent trunk GCC 4.8
and report back how compile times look for you now?

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[markus at trippelsdorf dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #22 from Markus Trippelsdorf <markus at trippelsdorf dot de> 2012-07-30 10:54:35 UTC ---
(In reply to comment #21)
> Markus, I think that r189951 should have fixed this problem (xf. 
> http://gcc.gnu.org/viewcvs?view=revision&revision=189951), although I suspect
> there will still be a compile time increase from GCC 4.7 to GCC 4.8 because of
> the macro expansion tracking stuff. Could you please try a recent trunk GCC 4.8
> and report back how compile times look for you now?

gcc version 4.8.0 20120730 (experimental) (GCC) 
markus@x4 tmp % time c++ -c -o pch.hpp.gch
/usr/include/boost/math/special_functions.hpp
c++ -c -o pch.hpp.gch /usr/include/boost/math/special_functions.hpp  138.60s
user 0.38s system 99% cpu 2:19.87 total

Still a 98% compile time regression on todays trunk.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #23 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-30 11:08:01 UTC ---
(In reply to comment #22)
> Still a 98% compile time regression on todays trunk.

What revision number is this? If it includes r189951, could you please see if
you can get an updated profile (similar to the one in comment #3)?

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[markus at trippelsdorf dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #24 from Markus Trippelsdorf <markus at trippelsdorf dot de> 2012-07-30 11:21:54 UTC ---
(In reply to comment #23)
> (In reply to comment #22)
> > Still a 98% compile time regression on todays trunk.
> 
> What revision number is this? If it includes r189951, could you please see if
> you can get an updated profile (similar to the one in comment #3)?

I'm running revision 189954.

# Samples: 557K of event 'cycles'
# Event count (approx.): 448107030198
#
# Overhead  Command  Shared Object          Symbol        
    97.54%  cc1plus  cc1plus            [.] gt_pch_p_9line_maps
     0.12%  cc1plus  cc1plus            [.] htab_find_slot_with_hash
     0.10%  cc1plus  libc-2.16.90.so    [.] memcpy@@GLIBC_2.14
     0.10%  cc1plus  cc1plus            [.] htab_expand
     0.09%  cc1plus  libc-2.16.90.so    [.] malloc_consolidate
     0.08%  cc1plus  cc1plus            [.] htab_find_with_hash
     0.08%  cc1plus  libc-2.16.90.so    [.] msort_with_tmp.part.0

Zoom into gt_pch_p_9line_maps:

       │         if ((*x).info_macro.maps != NULL) {
       │           size_t i2;
       │           for (i2 = 0; i2 != (size_t)(l2); i2++) {
  4.10 │109:   cmp    %rcx,%rbx
  3.86 │       nop
  3.22 │     ↓ je     188
  3.26 │112:   mov    0x18(%rbp),%rax
  3.21 │       add    $0x1,%rbx
       │           break;
       │         }
       │     }
       │
       │     void
       │     gt_pch_p_9line_maps (ATTRIBUTE_UNUSED void *this_obj,
  2.94 │11a:   lea    (%rbx,%rbx,4),%r15
  3.69 │       shl    $0x3,%r15
       │       {
       │         size_t l2 = (size_t)(((*x).info_macro).used);
       │         if ((*x).info_macro.maps != NULL) {
       │           size_t i2;
       │           for (i2 = 0; i2 != (size_t)(l2); i2++) {
       │             switch (((*x).info_macro.maps[i2]).reason ==
LC_ENTER_MACRO)
  3.58 │       lea    (%rax,%r15,1),%rdi
  3.59 │       cmpb   $0x4,0x4(%rdi)
  4.26 │     ↑ jne    100
       │                   op (&((*x).info_macro.maps[i2].d.ordinary.to_file),
cookie);
       │                 break;
       │               case 1:
       │                 {
       │                   union tree_node * x3 =
       │                     ((*x).info_macro.maps[i2].d.macro.macro) ?
HT_IDENT_TO_GCC_IDENT (HT_NODE (((*x).info_macro.maps[i2].d.macro.macro))) :
  4.18 │       mov    0x8(%rdi),%rsi
  4.07 │       xor    %edx,%edx
  2.28 │       lea    -0x18(%rsi),%r8
  2.28 │       test   %rsi,%rsi
  2.70 │       cmovne %r8,%rdx
       │                   if ((void *)((*x).info_macro.maps) == this_obj)
  2.69 │       cmp    %rax,%r14
       │                 if ((void *)((*x).info_macro.maps) == this_obj)
       │                   op (&((*x).info_macro.maps[i2].d.ordinary.to_file),
cookie);
       │                 break;
       │               case 1:
       │                 {
       │                   union tree_node * x3 =
  2.55 │       mov    %rdx,0x18(%rsp)
       │                     ((*x).info_macro.maps[i2].d.macro.macro) ?
HT_IDENT_TO_GCC_IDENT (HT_NODE (((*x).info_macro.maps[i2].d.macro.macro))) :
       │                   if ((void *)((*x).info_macro.maps) == this_obj)
  2.82 │     ↓ je     1a0
       │                     op (&(x3), cookie);
       │                   (*x).info_macro.maps[i2].d.macro.macro = (x3) ?
CPP_HASHNODE (GCC_IDENT_TO_HT_IDENT ((x3))) : NULL;
  2.80 │147:   lea    0x18(%rdx),%rsi
  3.00 │       xor    %eax,%eax
  3.28 │       test   %rdx,%rdx
  3.11 │       cmovne %rsi,%rax
  3.15 │       mov    %rax,0x8(%rdi)
       │                 }
       │                 if ((*x).info_macro.maps[i2].d.macro.macro_locations
!= NULL) {
  4.51 │       mov    0x18(%rbp),%rax
  4.45 │       add    %rax,%r15
  4.94 │       cmpq   $0x0,0x18(%r15)
  3.74 │     ↑ je     109
       │                   if ((void *)((*x).info_macro.maps) == this_obj)
  3.16 │       cmp    %rax,%r14
  3.03 │     ↑ jne    109
       │                     op
(&((*x).info_macro.maps[i2].d.macro.macro_locations), cookie);
       │       mov    %rcx,0x8(%rsp)
  0.00 │       lea    0x18(%r15),%rdi
       │       mov    %r13,%rsi
       │     → callq  *%r12
  0.00 │       mov    0x8(%rsp),%rcx
       │       }

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

Steven Bosscher <steven at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW
      Known to work|                            |4.7.1
      Known to fail|                            |4.8.0

--- Comment #25 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-30 12:03:25 UTC ---
AFAICT, the only way that gt_pch_p_9line_maps can be called, is from
ggc-common.c:gt_pch_save (via note_ptr_fn). The "op" argument will be
relocate_ptrs, so all calls are for pointer-rewriting and there are so many of
them simply because the linemaps structure is so big if we're tracking macro
expansions on a library as big as Boost.

It looks like this is the only note_ptr_fn callback left (if there were any
others to begin with) so a first simple speed-up could be to rip out this
callback stuff and just call relocate_ptrs directly. That would eliminate
millions of costly indirect calls.

Even better would be to reduce the size of line_maps, but I don't see how.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #26 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-30 13:17:25 UTC ---
FWIW, the resulting PCH for GCC 4.8 is more than twice as large as the PCH for
GCC 4.7.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #27 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-30 13:51:58 UTC ---
The problem is the quadratic behavior invoked by the loop in
gt_pch_nx_line_maps:
      {
        size_t l2 = (size_t)(((*x).info_macro).used);
        if ((*x).info_macro.maps != NULL) {
          size_t i2;
          for (i2 = 0; i2 != (size_t)(l2); i2++) {
            switch (((*x).info_macro.maps[i2]).reason == LC_ENTER_MACRO)
              {
              case 0:
                gt_pch_n_S ((*x).info_macro.maps[i2].d.ordinary.to_file);
                break;
              case 1:
                {
                  union tree_node * const x3 =
                    ((*x).info_macro.maps[i2].d.macro.macro) ?
HT_IDENT_TO_GCC_IDENT (HT_NODE (((*x).info_macro.maps[i2].d.macro.macro))) :
NULL;
                  gt_pch_n_9tree_node (x3);
                }
                if ((*x).info_macro.maps[i2].d.macro.macro_locations != NULL) {
                  gt_pch_note_object
((*x).info_macro.maps[i2].d.macro.macro_locations, x, gt_pch_p_9line_maps,
gt_types_enum_last);
                }
                break;
              default:
                break;
              }
          }

and the loop in gt_pch_p_9line_maps:

    size_t l2 = (size_t)(((*x).info_macro).used);
    if ((*x).info_macro.maps != NULL) {
      size_t i2;
      for (i2 = 0; i2 != (size_t)(l2); i2++) {
        switch (((*x).info_macro.maps[i2]).reason == LC_ENTER_MACRO)
          {
          case 0:
            if ((void *)((*x).info_macro.maps) == this_obj)
              op (&((*x).info_macro.maps[i2].d.ordinary.to_file), cookie);
            break;
          case 1:
            {
              union tree_node * x3 =
                ((*x).info_macro.maps[i2].d.macro.macro) ?
HT_IDENT_TO_GCC_IDENT (HT_NODE (((*x).info_macro.maps[i2].d.macro.macro))) :
NULL;
              if ((void *)((*x).info_macro.maps) == this_obj)
                op (&(x3), cookie);
              (*x).info_macro.maps[i2].d.macro.macro = (x3) ? CPP_HASHNODE
(GCC_IDENT_TO_HT_IDENT ((x3))) : NULL;
            }
            if ((*x).info_macro.maps[i2].d.macro.macro_locations != NULL) {
              if ((void *)((*x).info_macro.maps) == this_obj)
                op (&((*x).info_macro.maps[i2].d.macro.macro_locations),
cookie);
            }
            break;
          default:
            break;
          }
      }


The first loop registers every line map entry to be visited for pointer
rewriting later by gt_pch_p_9line_maps:

gt_pch_note_object ((*x).info_macro.maps[i2].d.macro.macro_locations, x,
gt_pch_p_9line_maps, gt_types_enum_last);

where x==input.c:line_table, and
(*x).info_macro.maps[i2].d.macro.macro_locations is the "i2"'th entry.

The second loop is called from the loop in gt_pch_save via note_ptr_fn, which
is gt_pch_p_9line_maps:

      state.ptrs[i]->note_ptr_fn (state.ptrs[i]->obj,
                                  state.ptrs[i]->note_ptr_cookie,
                                  relocate_ptrs, &state);
==
       gt_pch_p_9line_maps((*x).info_macro.maps[i2].d.macro.macro_locations,
                           &line_table, relocate_ptrs, &state);

and gt_pch_p_9line_maps loops over all map elements until:
  (void *)((*x).info_macro.maps) == this_obj

This causes (((*x).info_macro).used)**2 visits.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #28 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-30 14:22:37 UTC ---
With -ftrack-macro-expansion=2 (the default):

(gdb) call dump_line_table_statistics()
Number of expanded macros:                     237994
Average number of tokens per macro expansion:     12

Line Table allocations during the compilation process
Number of ordinary maps used:         2732
Ordinary map used size:                106k
Number of ordinary maps allocated:    6553
Ordinary maps allocated size:          255k
Number of macro maps used:             183k
Macro maps used size:                 7339k
Macro maps locations size:              24M
Macro maps size:                        31M
Duplicated maps locations size:       5598k
Total allocated maps size:              40M
Total used maps size:                   31M


With -ftrack-macro-expansion=0:
(gdb) call dump_line_table_statistics()
Number of expanded macros:                     233678
Average number of tokens per macro expansion:     13

Line Table allocations during the compilation process
Number of ordinary maps used:         2732
Ordinary map used size:                106k
Number of ordinary maps allocated:    6553
Ordinary maps allocated size:          255k
Number of macro maps used:               0
Macro maps used size:                    0
Macro maps locations size:               0
Macro maps size:                         0
Duplicated maps locations size:          0
Total allocated maps size:             255k
Total used maps size:                  106k

(The count for number of tokens per macro expansion should be the same, looks
like a bug...).

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #29 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-30 15:04:18 UTC ---
Created attachment 27897
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27897
Unswitched gtype-desc.c at r189965

Manually "unswitching" gt_pch_p_9line_maps gives me acceptable compile times.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #30 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-30 16:15:35 UTC ---
Created attachment 27898
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27898
Hack to avoid quadratic loops

This makes use of the assumption that this_obj and its comparison companion are
loop invariant (they have to be for pointer rewriting). Drops compile time to
worse-than-gcc47-but-acceptable levels.

Markus, could you give this a try, please?

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[markus at trippelsdorf dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #31 from Markus Trippelsdorf <markus at trippelsdorf dot de> 2012-07-30 16:26:23 UTC ---
(In reply to comment #30)
> Created attachment 27898 [details]
> Hack to avoid quadratic loops
> 
> This makes use of the assumption that this_obj and its comparison companion are
> loop invariant (they have to be for pointer rewriting). Drops compile time to
> worse-than-gcc47-but-acceptable levels.
> 
> Markus, could you give this a try, please?

Looks very good:

 % time c++ -c -o pch.hpp.gch /usr/include/boost/math/special_functions.hpp
c++ -c -o pch.hpp.gch /usr/include/boost/math/special_functions.hpp  2.67s user
0.20s system 99% cpu 2.888 total

Thanks a lot, Steven.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

Steven Bosscher <steven at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |compile-time-hog
             Status|NEW                         |ASSIGNED
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2012-07/msg01524.htm
                   |                            |l
         AssignedTo|unassigned at gcc dot       |steven at gcc dot gnu.org
                   |gnu.org                     |

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

--- Comment #32 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-31 09:21:03 UTC ---
Author: steven
Date: Tue Jul 31 09:20:56 2012
New Revision: 189999

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189999
Log:
    PR pch/53880
    * gengtype.c (struct walk_type_data): Add have_this_obj field.
    (walk_type): For functions that take a this_obj argument and
    that process fields with a GTY((length)) argument, write the
    test that write_types_local_process_field will write also at the
    head of the loop, effectively unswitching the loop.
    (write_func_for_structure, write_local_func_for_structure): Clear
    have_this_obj before calling walk_type.
    (write_local_func_for_structure): Set have_this_obj before walk_type.
    (write_array): Set have_this_obj for output of local pointer walking
    functions but not for marker functions.
    (write_types_local_process_field): Assert have_this_obj is set.

    * rtl.h (simplify_using_condition): Adjust prototype using bitmap
    from coretypes.h.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gengtype.c
    trunk/gcc/rtl.h

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

Steven Bosscher <steven at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|4.8.0                       |

--- Comment #33 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-31 09:23:52 UTC ---
Fixed for GCC 4.8.0.

It would be nice to know what the effect of the patch is for GCC 4.7, but I
don't have things set up to test that. Markus, if you can do that testing, I'm
interested in the results. Otherwise, this PR can be closed as FIXED.

[Bug pch/53880] [4.8 Regression] compile time regression when generating precompiled headers for boost

[markus at trippelsdorf dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53880

Markus Trippelsdorf <markus at trippelsdorf dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #34 from Markus Trippelsdorf <markus at trippelsdorf dot de> 2012-07-31 10:35:54 UTC ---
(In reply to comment #33)
> Fixed for GCC 4.8.0.
> 
> It would be nice to know what the effect of the patch is for GCC 4.7, but I
> don't have things set up to test that. Markus, if you can do that testing, I'm
> interested in the results. Otherwise, this PR can be closed as FIXED.

Given that -ftrack-macro-expansion is zero by default for GCC-4.7
there is no regression to fix AFAICS. So IMHO this patch shouldn't
be applied on the 4.7 branch. (It would only be relevant if one
explicitly uses the -ftrack-macro-expansion=2 when generating the pch.)