On Thu, Nov 28, 2013 at 6:22 PM, H.J. Lu <hongjiu...@intel.com> wrote:
> There is a bad interaction between inlined C++ member functions
> and LTO + profiledbootstrap, which leads to
>
> LTO bootstrap to fail with bootstrap-profile:
>
> Existing SSA name for symbol marked for renaming: aloop_37
> In member function \u2018__base_ctor \u2019:
> lto1: internal compiler error: SSA corruption
> 0xcd84eb update_ssa(unsigned int)
> /export/project/git/gcc-regression/gcc/gcc/tree-into-ssa.c:3246
> 0xa5814c input_function
> /export/project/git/gcc-regression/gcc/gcc/lto-streamer-in.c:1006
> 0xa5814c lto_read_body
> /export/project/git/gcc-regression/gcc/gcc/lto-streamer-in.c:1070
> 0xa5814c lto_input_function_body(lto_file_decl_data*, cgraph_node*, char
> const*)
> /export/project/git/gcc-regression/gcc/gcc/lto-streamer-in.c:1112
> 0x66d2bc cgraph_get_body(cgraph_node*)
> /export/project/git/gcc-regression/gcc/gcc/cgraph.c:2981
> 0x99aa58 ipa_merge_profiles(cgraph_node*, cgraph_node*)
> /export/project/git/gcc-regression/gcc/gcc/ipa-utils.c:699
> 0x595a86 lto_cgraph_replace_node
> /export/project/git/gcc-regression/gcc/gcc/lto/lto-symtab.c:82
> 0x596159 lto_symtab_merge_symbols_1
> /export/project/git/gcc-regression/gcc/gcc/lto/lto-symtab.c:561
> 0x596159 lto_symtab_merge_symbols()
> /export/project/git/gcc-regression/gcc/gcc/lto/lto-symtab.c:589
> 0x5850dd read_cgraph_and_symbols
> /export/project/git/gcc-regression/gcc/gcc/lto/lto.c:2946
> 0x5850dd lto_main()
> /export/project/git/gcc-regression/gcc/gcc/lto/lto.c:3255
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <http://gcc.gnu.org/bugs.html> for instructions.
>
> There are only 2 files which don't inline all loop_iterator
> member function and may be miscompiled:
>
> File: ipa-inline-analysis.o
>
> Symbol table '.symtab' contains 454 entries:
> Num: Value Size Type Bind Vis Ndx Name
> ...
> 262: 0000000000000000 0 NOTYPE LOCAL DEFAULT 5
> loop_iterator::loop_iterator(loop**, unsigned int)
> ...
> 352: 0000000000000000 89 FUNC WEAK DEFAULT 27
> loop_iterator::next()
> 353: 0000000000000000 748 FUNC WEAK DEFAULT 30
> loop_iterator::loop_iterator(loop**, unsigned int)
> 354: 0000000000000000 748 FUNC WEAK DEFAULT 30
> loop_iterator::loop_iterator(loop**, unsigned int)
> ...
>
> File: tree-cfg.o
>
> Symbol table '.symtab' contains 783 entries:
> Num: Value Size Type Bind Vis Ndx Name
> ...
> 385: 0000000000000000 0 NOTYPE LOCAL DEFAULT 5
> loop_iterator::loop_iterator(loop**, unsigned int)
> ...
> 536: 0000000000000000 748 FUNC WEAK DEFAULT 34
> loop_iterator::loop_iterator(loop**, unsigned int)
> ...
> 538: 0000000000000000 748 FUNC WEAK DEFAULT 34
> loop_iterator::loop_iterator(loop**, unsigned int)
> ...
>
> When either loop_iterator::next or loop_iterator::loop_iterator
> inlined, bootstrap fails with the similar error. This patch
> works around the problem by not inlining those 2 functions.
> On Nehalem machine using "make -j8", without the patch, I got
>
> 17836.13user 638.12system 55:49.72elapsed
>
> for bootstrap and
>
> 32362.67user 4313.11system 1:29:59elapsed
>
> for running testsuite. With the patch, I got
>
> 7900.41user 640.39system 55:03.14elapsed
>
> for bootstrap and
>
> 31891.96user 4251.23system 1:31:41elapse
>
> for running testsuite. There is very little performance
> difference and the binaries are also a little bit smaller:
>
> 16787252 34920 1098648 17920820 1117334
> build-x86_64-linux/gcc/cc1
> 16809748 34920 1098648 17943316 111cb14
> build-x86_64-linux.old/gcc/cc1
> 19188340 35008 1126552 20349900 13683cc
> build-x86_64-linux/gcc/cc1objplus
> 18865150 35008 1121848 20022006 13182f6
> build-x86_64-linux/gcc/cc1plus
> 19210836 35008 1126552 20372396 136dbac
> build-x86_64-linux.old/gcc/cc1objplus
> 18887646 35008 1121848 20044502 131dad6
> build-x86_64-linux.old/gcc/cc1plus
> 17274027 44056 1104024 18422107 119195b
> build-x86_64-linux/gcc/f951
> 17296523 44056 1104024 18444603 119713b
> build-x86_64-linux.old/gcc/f951
> 17354837 51424 1105752 18512013 11a788d
> build-x86_64-linux/gcc/go1
> 17377333 51424 1105752 18534509 11ad06d
> build-x86_64-linux.old/gcc/go1
> 20815529 43928 6289304 27148761 19e41d9
> build-x86_64-linux/gcc/gnat1
> 20838025 43928 6289304 27171257 19e99b9
> build-x86_64-linux.old/gcc/gnat1
> 15944305 35688 1095064 17075057 1048b71
> build-x86_64-linux/gcc/jc1
> 15966801 35688 1095064 17097553 104e351
> build-x86_64-linux.old/gcc/jc1
>
> Should this patch be applied to restore LTO bootstrap with
> bootstrap-profile?
I'd rather fix the bug than moving those functions out-of-line. Is it enough
to move the constructor and destructor out-of-line?
Thanks,
Richard.
> Thanks.
>
> H.J.
> ---
> 2013-11-28 H.J. Lu <hongjiu...@intel.com>
>
> PR bootstrap/59199
> * cfgloop.h (loop_iterator::next, loop_iterator::loop_iterator):
> Moved to ...
> * cfgloop.c (loop_iterator::next, loop_iterator::loop_iterator):
> Here.
>
> diff --git a/gcc/cfgloop.c b/gcc/cfgloop.c
> index 0b12e73..3e34850 100644
> --- a/gcc/cfgloop.c
> +++ b/gcc/cfgloop.c
> @@ -1919,3 +1919,95 @@ bb_loop_depth (const_basic_block bb)
> {
> return bb->loop_father ? loop_depth (bb->loop_father) : 0;
> }
> +
> +loop_p
> +loop_iterator::next ()
> +{
> + int anum;
> +
> + while (this->to_visit.iterate (this->idx, &anum))
> + {
> + this->idx++;
> + loop_p loop = get_loop (cfun, anum);
> + if (loop)
> + return loop;
> + }
> +
> + return NULL;
> +}
> +
> +loop_iterator::loop_iterator (loop_p *loop, unsigned flags)
> +{
> + struct loop *aloop;
> + unsigned i;
> + int mn;
> +
> + this->idx = 0;
> + if (!current_loops)
> + {
> + this->to_visit.create (0);
> + *loop = NULL;
> + return;
> + }
> +
> + this->to_visit.create (number_of_loops (cfun));
> + mn = (flags & LI_INCLUDE_ROOT) ? 0 : 1;
> +
> + if (flags & LI_ONLY_INNERMOST)
> + {
> + for (i = 0; vec_safe_iterate (current_loops->larray, i, &aloop); i++)
> + if (aloop != NULL
> + && aloop->inner == NULL
> + && aloop->num >= mn)
> + this->to_visit.quick_push (aloop->num);
> + }
> + else if (flags & LI_FROM_INNERMOST)
> + {
> + /* Push the loops to LI->TO_VISIT in postorder. */
> + for (aloop = current_loops->tree_root;
> + aloop->inner != NULL;
> + aloop = aloop->inner)
> + continue;
> +
> + while (1)
> + {
> + if (aloop->num >= mn)
> + this->to_visit.quick_push (aloop->num);
> +
> + if (aloop->next)
> + {
> + for (aloop = aloop->next;
> + aloop->inner != NULL;
> + aloop = aloop->inner)
> + continue;
> + }
> + else if (!loop_outer (aloop))
> + break;
> + else
> + aloop = loop_outer (aloop);
> + }
> + }
> + else
> + {
> + /* Push the loops to LI->TO_VISIT in preorder. */
> + aloop = current_loops->tree_root;
> + while (1)
> + {
> + if (aloop->num >= mn)
> + this->to_visit.quick_push (aloop->num);
> +
> + if (aloop->inner != NULL)
> + aloop = aloop->inner;
> + else
> + {
> + while (aloop != NULL && aloop->next == NULL)
> + aloop = loop_outer (aloop);
> + if (aloop == NULL)
> + break;
> + aloop = aloop->next;
> + }
> + }
> + }
> +
> + *loop = this->next ();
> +}
> diff --git a/gcc/cfgloop.h b/gcc/cfgloop.h
> index 68285a6..a76d243 100644
> --- a/gcc/cfgloop.h
> +++ b/gcc/cfgloop.h
> @@ -547,7 +547,7 @@ struct loop_iterator
> loop_iterator (loop_p *loop, unsigned flags);
> ~loop_iterator ();
>
> - inline loop_p next ();
> + loop_p next ();
>
> /* The list of loops to visit. */
> vec<int> to_visit;
> @@ -556,99 +556,6 @@ struct loop_iterator
> unsigned idx;
> };
>
> -inline loop_p
> -loop_iterator::next ()
> -{
> - int anum;
> -
> - while (this->to_visit.iterate (this->idx, &anum))
> - {
> - this->idx++;
> - loop_p loop = get_loop (cfun, anum);
> - if (loop)
> - return loop;
> - }
> -
> - return NULL;
> -}
> -
> -inline
> -loop_iterator::loop_iterator (loop_p *loop, unsigned flags)
> -{
> - struct loop *aloop;
> - unsigned i;
> - int mn;
> -
> - this->idx = 0;
> - if (!current_loops)
> - {
> - this->to_visit.create (0);
> - *loop = NULL;
> - return;
> - }
> -
> - this->to_visit.create (number_of_loops (cfun));
> - mn = (flags & LI_INCLUDE_ROOT) ? 0 : 1;
> -
> - if (flags & LI_ONLY_INNERMOST)
> - {
> - for (i = 0; vec_safe_iterate (current_loops->larray, i, &aloop); i++)
> - if (aloop != NULL
> - && aloop->inner == NULL
> - && aloop->num >= mn)
> - this->to_visit.quick_push (aloop->num);
> - }
> - else if (flags & LI_FROM_INNERMOST)
> - {
> - /* Push the loops to LI->TO_VISIT in postorder. */
> - for (aloop = current_loops->tree_root;
> - aloop->inner != NULL;
> - aloop = aloop->inner)
> - continue;
> -
> - while (1)
> - {
> - if (aloop->num >= mn)
> - this->to_visit.quick_push (aloop->num);
> -
> - if (aloop->next)
> - {
> - for (aloop = aloop->next;
> - aloop->inner != NULL;
> - aloop = aloop->inner)
> - continue;
> - }
> - else if (!loop_outer (aloop))
> - break;
> - else
> - aloop = loop_outer (aloop);
> - }
> - }
> - else
> - {
> - /* Push the loops to LI->TO_VISIT in preorder. */
> - aloop = current_loops->tree_root;
> - while (1)
> - {
> - if (aloop->num >= mn)
> - this->to_visit.quick_push (aloop->num);
> -
> - if (aloop->inner != NULL)
> - aloop = aloop->inner;
> - else
> - {
> - while (aloop != NULL && aloop->next == NULL)
> - aloop = loop_outer (aloop);
> - if (aloop == NULL)
> - break;
> - aloop = aloop->next;
> - }
> - }
> - }
> -
> - *loop = this->next ();
> -}
> -
> inline
> loop_iterator::~loop_iterator ()
> {
