[pushed] Fix profiledbootstrap poly_int fallout [PR111642]

[Richard Sandiford]

rtl-tests.cc and simplify-rtx.cc used partial specialisation
to try to restrict the NUM_POLY_INT_COEFFS>1 tests without
resorting to preprocessor tests.  That now triggers an error
in some configurations, since the NUM_POLY_INT_COEFFS>1 tests
used the global poly_int64, whose definition does not depend
on the template parameter.

This patch uses local types that do depend on the template parameter.

Tested using profiledbootstrap and bootstrap4 on x86_64-linux-gnu,
both of which failed for me for unrelated reasons later.  But Sergei
confirms in the PR that the patch does fix the bug.  Pushed as obvious.

Richard


gcc/
PR bootstrap/111642
* rtl-tests.cc (const_poly_int_tests::run): Use a local
poly_int64 typedef.
* simplify-rtx.cc (simplify_const_poly_int_tests::run): Likewise.
---
 gcc/rtl-tests.cc| 1 +
 gcc/simplify-rtx.cc | 1 +
 2 files changed, 2 insertions(+)

diff --git a/gcc/rtl-tests.cc b/gcc/rtl-tests.cc
index ae8669419b6..96656c54a48 100644
--- a/gcc/rtl-tests.cc
+++ b/gcc/rtl-tests.cc
@@ -246,6 +246,7 @@ template
 void
 const_poly_int_tests::run ()
 {
+  using poly_int64 = poly_int;
   rtx x1 = gen_int_mode (poly_int64 (1, 1), QImode);
   rtx x255 = gen_int_mode (poly_int64 (1, 255), QImode);
 
diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index 170406aa28b..bd9443dbcc2 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -8689,6 +8689,7 @@ template
 void
 simplify_const_poly_int_tests::run ()
 {
+  using poly_int64 = poly_int;
   rtx x1 = gen_int_mode (poly_int64 (1, 1), QImode);
   rtx x2 = gen_int_mode (poly_int64 (-80, 127), QImode);
   rtx x3 = gen_int_mode (poly_int64 (-79, -128), QImode);
-- 
2.25.1

Fix profiledbootstrap

[Jan Hubicka via Gcc-patches]

Hi,
Profiledbootstrap fails with ICE in update_loop_exit_probability_scale_dom_bbs
called from loop unroling.
The reason is that under relatively rare situations, we may run into case where
loop has multiple exits and all are considered as likely but then we scale down
the profile and one of the exits becomes unlikely. 

We pass around unadjusted_exit_count to scale exit probability correctly.  In 
this
case we may end up using uninitialized value and profile-count type 
intentionally
bombs on that.

Profiledbootstrapped x86_64-linux, comitted.

gcc/ChangeLog:

PR bootstrap/110857
* cfgloopmanip.cc (scale_loop_profile): (Un)initialize
unadjusted_exit_count.

diff --git a/gcc/cfgloopmanip.cc b/gcc/cfgloopmanip.cc
index 86360b5f380..b237ad4e8ac 100644
--- a/gcc/cfgloopmanip.cc
+++ b/gcc/cfgloopmanip.cc
@@ -742,7 +742,7 @@ scale_loop_profile (class loop *loop, profile_probability p,
   /* In a consistent profile unadjusted_exit_count should be same as count_in,
  however to preserve as much of the original info, avoid recomputing
  it.  */
-  profile_count unadjusted_exit_count;
+  profile_count unadjusted_exit_count = profile_count::uninitialized ();
   if (exit_edge)
 unadjusted_exit_count = exit_edge->count ();
   scale_loop_frequencies (loop, scale_prob);

Re: [PATCH] Fix profiledbootstrap - store-merging aliasing issue (PR bootstrap/82916)

[Richard Biener]

On Fri, 10 Nov 2017, Jakub Jelinek wrote:

> On Fri, Nov 10, 2017 at 08:52:16AM +0100, Richard Biener wrote:
> > > @@ -958,8 +959,10 @@ pass_store_merging::terminate_all_aliasi
> > >unsigned int i;
> > >FOR_EACH_VEC_ELT (cur->m_store_info, i, info)
> > >   {
> > > -   if (ref_maybe_used_by_stmt_p (stmt, gimple_assign_lhs (info->stmt))
> > > -   || stmt_may_clobber_ref_p (stmt, gimple_assign_lhs (info->stmt)))
> > > +   tree lhs = gimple_assign_lhs (info->stmt);
> > > +   if (ref_maybe_used_by_stmt_p (stmt, lhs)
> > > +   || stmt_may_clobber_ref_p (stmt, lhs)
> > > +   || (store_lhs && refs_output_dependent_p (store_lhs, lhs)))
> > 
> > Looks good but may do redundant work for store_lhs?  So rather
> > 
> >|| (! store_lhs && stmt_may_clobber_ref_p (stmt, lhs)
> >|| (store_lhs && refs_output_dependent_p (store_lhs, lhs)
> > 
> > ?  Fails to handle storing calls (in case those can appear in the chains).
> 
> info->stmt is known to be a store, but stmt is not, it can be any other
> stmt, including calls, so the above would miss the calls handling.
> 
> > Looks like we miss some convenient stmt_output/anti_dependent_p (you can
> > follow stmt_may_clobbers_ref_p[_1] for cut and/or add a
> > bool tbaa flag we can pass down to stmt_may_clobber_ref_p_1).
> 
> So perhaps bool tbaa = true argument to both stmt_may_clobber_ref_p_1
> and stmt_may_clobber_ref_p, or just stmt_may_clobber_ref_p_1 and
> add some differently named alternative to stmt_may_clobber_ref_p
> (in that case, any suggestions on a good name?)?

Internally (aka static fn) I'd just add a bool param w/o default.

The external API should be stmt_output/anti_dependent_p and I think
these days with C++ we could make stmt_may_clobber_ref_p_1 taking ao_ref
and stmt_may_clobber_ref_p taking a tree overloads of
stmt_may_clobber_ref_p.  We'd then have _1 being static and having the
extra arg.

Richard.

> > That said - the patch is ok, any improvements can be done as followup.
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Re: [PATCH] Fix profiledbootstrap - store-merging aliasing issue (PR bootstrap/82916)

[Jakub Jelinek]

On Fri, Nov 10, 2017 at 08:52:16AM +0100, Richard Biener wrote:
> > @@ -958,8 +959,10 @@ pass_store_merging::terminate_all_aliasi
> >unsigned int i;
> >FOR_EACH_VEC_ELT (cur->m_store_info, i, info)
> > {
> > - if (ref_maybe_used_by_stmt_p (stmt, gimple_assign_lhs (info->stmt))
> > - || stmt_may_clobber_ref_p (stmt, gimple_assign_lhs (info->stmt)))
> > + tree lhs = gimple_assign_lhs (info->stmt);
> > + if (ref_maybe_used_by_stmt_p (stmt, lhs)
> > + || stmt_may_clobber_ref_p (stmt, lhs)
> > + || (store_lhs && refs_output_dependent_p (store_lhs, lhs)))
> 
> Looks good but may do redundant work for store_lhs?  So rather
> 
>|| (! store_lhs && stmt_may_clobber_ref_p (stmt, lhs)
>|| (store_lhs && refs_output_dependent_p (store_lhs, lhs)
> 
> ?  Fails to handle storing calls (in case those can appear in the chains).

info->stmt is known to be a store, but stmt is not, it can be any other
stmt, including calls, so the above would miss the calls handling.

> Looks like we miss some convenient stmt_output/anti_dependent_p (you can
> follow stmt_may_clobbers_ref_p[_1] for cut and/or add a
> bool tbaa flag we can pass down to stmt_may_clobber_ref_p_1).

So perhaps bool tbaa = true argument to both stmt_may_clobber_ref_p_1
and stmt_may_clobber_ref_p, or just stmt_may_clobber_ref_p_1 and
add some differently named alternative to stmt_may_clobber_ref_p
(in that case, any suggestions on a good name?)?

> That said - the patch is ok, any improvements can be done as followup.

Jakub

Re: [PATCH] Fix profiledbootstrap - store-merging aliasing issue (PR bootstrap/82916)

[Richard Biener]

On Thu, 9 Nov 2017, Jakub Jelinek wrote:

> Hi!
> 
> We want to terminate a chain if a chain with different base (or insn
> outside of any chain) has a store that the current stmt might use, or
> overwrite.  The functions it used didn't cover the store after store
> case which in the middle-end aliasing model needs to avoid tbaa, because
> the latter store might be after a placement new or might otherwise change
> the dynamic object type of the object.
> 
> The following patch does that.  Bootstrapped/regtested on x86_64-linux and
> i686-linux, additionally profiledbootstrapped with the configure options
> Markus mentioned in the PR.  Ok for trunk?
> 
> 2017-11-09  Jakub Jelinek  
> 
>   PR bootstrap/82916
>   * gimple-ssa-store-merging.c
>   (pass_store_merging::terminate_all_aliasing_chains): For
>   gimple_store_p stmts also call refs_output_dependent_p.
> 
>   * gcc.dg/store_merging_2.c: Only expect 2 successful mergings instead
>   of 3.
>   * gcc.dg/pr82916.c: New test.
> 
> --- gcc/gimple-ssa-store-merging.c.jj 2017-11-09 15:51:08.0 +0100
> +++ gcc/gimple-ssa-store-merging.c2017-11-09 18:01:20.789236951 +0100
> @@ -945,6 +945,7 @@ pass_store_merging::terminate_all_aliasi
>if (!gimple_vuse (stmt))
>  return false;
>  
> +  tree store_lhs = gimple_store_p (stmt) ? gimple_get_lhs (stmt) : NULL_TREE;
>for (imm_store_chain_info *next = m_stores_head, *cur = next; cur; cur = 
> next)
>  {
>next = cur->next;
> @@ -958,8 +959,10 @@ pass_store_merging::terminate_all_aliasi
>unsigned int i;
>FOR_EACH_VEC_ELT (cur->m_store_info, i, info)
>   {
> -   if (ref_maybe_used_by_stmt_p (stmt, gimple_assign_lhs (info->stmt))
> -   || stmt_may_clobber_ref_p (stmt, gimple_assign_lhs (info->stmt)))
> +   tree lhs = gimple_assign_lhs (info->stmt);
> +   if (ref_maybe_used_by_stmt_p (stmt, lhs)
> +   || stmt_may_clobber_ref_p (stmt, lhs)
> +   || (store_lhs && refs_output_dependent_p (store_lhs, lhs)))

Looks good but may do redundant work for store_lhs?  So rather

   || (! store_lhs && stmt_may_clobber_ref_p (stmt, lhs)
   || (store_lhs && refs_output_dependent_p (store_lhs, lhs)

?  Fails to handle storing calls (in case those can appear in the chains).
Looks like we miss some convenient stmt_output/anti_dependent_p (you can
follow stmt_may_clobbers_ref_p[_1] for cut and/or add a
bool tbaa flag we can pass down to stmt_may_clobber_ref_p_1).

That said - the patch is ok, any improvements can be done as followup.

Thanks,
Richard.

>   {
> if (dump_file && (dump_flags & TDF_DETAILS))
>   {
> --- gcc/testsuite/gcc.dg/store_merging_2.c.jj 2017-11-08 16:46:19.0 
> +0100
> +++ gcc/testsuite/gcc.dg/store_merging_2.c2017-11-09 18:16:33.482344795 
> +0100
> @@ -77,4 +77,4 @@ main (void)
>return 0;
>  }
>  
> -/* { dg-final { scan-tree-dump-times "Merging successful" 3 "store-merging" 
> } } */
> +/* { dg-final { scan-tree-dump-times "Merging successful" 2 "store-merging" 
> } } */
> --- gcc/testsuite/gcc.dg/pr82916.c.jj 2017-11-09 18:12:40.707128841 +0100
> +++ gcc/testsuite/gcc.dg/pr82916.c2017-11-09 18:12:19.0 +0100
> @@ -0,0 +1,47 @@
> +/* PR bootstrap/82916 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fno-tree-dse" } */
> +
> +struct A { struct A *next; };
> +struct C
> +{
> +  int *of;
> +  struct C *parent, *prev, *next;
> +  int depth;
> +  int min;
> +  struct C *min_occ;
> +};
> +
> +__attribute__((noipa)) struct C *
> +foo (int *node)
> +{
> +  struct A *p = __builtin_malloc (sizeof (struct C));
> +  if (!p)
> +return 0;
> +  p->next = 0;
> +  /* Originally placement new.  */
> +  struct C *nw = (struct C *)(void *)p;
> +  nw->of = node;
> +  nw->parent = 0;
> +  nw->prev = 0;
> +  nw->next = 0;
> +  nw->depth = 0;
> +  nw->min_occ = nw;
> +  nw->min = 0;
> +  return nw;
> +}
> +
> +int
> +main ()
> +{
> +  int o;
> +  struct C *p = foo ();
> +  if (p)
> +{
> +  if (p->of !=  || p->parent || p->prev || p->next || p->depth
> +   || p->min || p->min_occ != p)
> + __builtin_abort ();
> +}
> +  __builtin_free (p);
> +  return 0;
> +}
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

[PATCH] Fix profiledbootstrap - store-merging aliasing issue (PR bootstrap/82916)

[Jakub Jelinek]

Hi!

We want to terminate a chain if a chain with different base (or insn
outside of any chain) has a store that the current stmt might use, or
overwrite.  The functions it used didn't cover the store after store
case which in the middle-end aliasing model needs to avoid tbaa, because
the latter store might be after a placement new or might otherwise change
the dynamic object type of the object.

The following patch does that.  Bootstrapped/regtested on x86_64-linux and
i686-linux, additionally profiledbootstrapped with the configure options
Markus mentioned in the PR.  Ok for trunk?

2017-11-09  Jakub Jelinek  

PR bootstrap/82916
* gimple-ssa-store-merging.c
(pass_store_merging::terminate_all_aliasing_chains): For
gimple_store_p stmts also call refs_output_dependent_p.

* gcc.dg/store_merging_2.c: Only expect 2 successful mergings instead
of 3.
* gcc.dg/pr82916.c: New test.

--- gcc/gimple-ssa-store-merging.c.jj   2017-11-09 15:51:08.0 +0100
+++ gcc/gimple-ssa-store-merging.c  2017-11-09 18:01:20.789236951 +0100
@@ -945,6 +945,7 @@ pass_store_merging::terminate_all_aliasi
   if (!gimple_vuse (stmt))
 return false;
 
+  tree store_lhs = gimple_store_p (stmt) ? gimple_get_lhs (stmt) : NULL_TREE;
   for (imm_store_chain_info *next = m_stores_head, *cur = next; cur; cur = 
next)
 {
   next = cur->next;
@@ -958,8 +959,10 @@ pass_store_merging::terminate_all_aliasi
   unsigned int i;
   FOR_EACH_VEC_ELT (cur->m_store_info, i, info)
{
- if (ref_maybe_used_by_stmt_p (stmt, gimple_assign_lhs (info->stmt))
- || stmt_may_clobber_ref_p (stmt, gimple_assign_lhs (info->stmt)))
+ tree lhs = gimple_assign_lhs (info->stmt);
+ if (ref_maybe_used_by_stmt_p (stmt, lhs)
+ || stmt_may_clobber_ref_p (stmt, lhs)
+ || (store_lhs && refs_output_dependent_p (store_lhs, lhs)))
{
  if (dump_file && (dump_flags & TDF_DETAILS))
{
--- gcc/testsuite/gcc.dg/store_merging_2.c.jj   2017-11-08 16:46:19.0 
+0100
+++ gcc/testsuite/gcc.dg/store_merging_2.c  2017-11-09 18:16:33.482344795 
+0100
@@ -77,4 +77,4 @@ main (void)
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-times "Merging successful" 3 "store-merging" } 
} */
+/* { dg-final { scan-tree-dump-times "Merging successful" 2 "store-merging" } 
} */
--- gcc/testsuite/gcc.dg/pr82916.c.jj   2017-11-09 18:12:40.707128841 +0100
+++ gcc/testsuite/gcc.dg/pr82916.c  2017-11-09 18:12:19.0 +0100
@@ -0,0 +1,47 @@
+/* PR bootstrap/82916 */
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-tree-dse" } */
+
+struct A { struct A *next; };
+struct C
+{
+  int *of;
+  struct C *parent, *prev, *next;
+  int depth;
+  int min;
+  struct C *min_occ;
+};
+
+__attribute__((noipa)) struct C *
+foo (int *node)
+{
+  struct A *p = __builtin_malloc (sizeof (struct C));
+  if (!p)
+return 0;
+  p->next = 0;
+  /* Originally placement new.  */
+  struct C *nw = (struct C *)(void *)p;
+  nw->of = node;
+  nw->parent = 0;
+  nw->prev = 0;
+  nw->next = 0;
+  nw->depth = 0;
+  nw->min_occ = nw;
+  nw->min = 0;
+  return nw;
+}
+
+int
+main ()
+{
+  int o;
+  struct C *p = foo ();
+  if (p)
+{
+  if (p->of !=  || p->parent || p->prev || p->next || p->depth
+ || p->min || p->min_occ != p)
+   __builtin_abort ();
+}
+  __builtin_free (p);
+  return 0;
+}

Jakub

[PATCH][OBVIOUS] Fix profiledbootstrap.

[Martin Liška]

Hello.

So eventually it looks needed fix is much simpler, one just need to rename a 
folder.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
I'm going to install it.

Martin
>From 028b5cc3865bdc77c185172de84627e140030303 Mon Sep 17 00:00:00 2001
From: marxin <mli...@suse.cz>
Date: Fri, 27 Oct 2017 12:21:02 +0200
Subject: [PATCH] Fix profiledbootstrap.

ChangeLog:

2017-10-27  Martin Liska  <mli...@suse.cz>

	* Makefile.tpl: Use proper name of folder as it was renamed
	during transition to 4 stages.
	* Makefile.in: Regenerate.
---
 Makefile.in  | 4 ++--
 Makefile.tpl | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile.in b/Makefile.in
index 78db0982ba2..13d23915349 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -57174,8 +57174,8 @@ stageprofile-end::
 stagefeedback-start::
 	@r=`${PWD_COMMAND}`; export r; \
 	s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
-	for i in prev-*; do \
-	  j=`echo $$i | sed s/^prev-//`; \
+	for i in stageprofile-*; do \
+	  j=`echo $$i | sed s/^stageprofile-//`; \
 	  cd $$r/$$i && \
 	  { find . -type d | sort | sed 's,.*,$(SHELL) '"$$s"'/mkinstalldirs "../'$$j'/&",' | $(SHELL); } && \
 	  { find . -name '*.*da' | sed 's,.*,$(LN) -f "&" "../'$$j'/&",' | $(SHELL); }; \
diff --git a/Makefile.tpl b/Makefile.tpl
index 5fcd7e358d9..1f23b79b4b2 100644
--- a/Makefile.tpl
+++ b/Makefile.tpl
@@ -1718,8 +1718,8 @@ stageprofile-end::
 stagefeedback-start::
 	@r=`${PWD_COMMAND}`; export r; \
 	s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
-	for i in prev-*; do \
-	  j=`echo $$i | sed s/^prev-//`; \
+	for i in stageprofile-*; do \
+	  j=`echo $$i | sed s/^stageprofile-//`; \
 	  cd $$r/$$i && \
 	  { find . -type d | sort | sed 's,.*,$(SHELL) '"$$s"'/mkinstalldirs "../'$$j'/&",' | $(SHELL); } && \
 	  { find . -name '*.*da' | sed 's,.*,$(LN) -f "&" "../'$$j'/&",' | $(SHELL); }; \
-- 
2.14.2

Fix profiledbootstrap

[Jan Hubicka]

Hi,
this patch fixes ICE during profiledbootstrap about hot BB being dominated by
cold.  This is verified by RTL verify_flow_info and was added by Theresa
along with patches to undo mistakes in in sane profile.

The implementation is odd because this is not about dominance but reachability.
It is also bit questionable decision because it is possible for very hot loop
to be reahcable only by cold path (i.e. not executed every time program is
executed). But I guess it makes sense to stay in cold region once it is entered.

This patch reimplements the dominance based decision to reachability and makes
bb-reorder to promote hot bbs that are only reahcable by cold BBs to cold 
section.

Profilebootstrapped/regtested x86_64-linux, will commit it tomorrow.

* bb-reorder.c (find_rarely_executed_basic_blocks_and_crossing_edges):
Put all BBs reachable only via paths crossing cold region to cold
region.
* cfgrtl.c (find_bbs_reachable_by_hot_paths): New function.
Index: bb-reorder.c
===
--- bb-reorder.c(revision 250390)
+++ bb-reorder.c(working copy)
@@ -1665,6 +1665,12 @@ find_rarely_executed_basic_blocks_and_cr
   _in_hot_partition);
   if (cold_bb_count)
 sanitize_hot_paths (false, cold_bb_count, _in_hot_partition);
+
+  hash_set  set;
+  find_bbs_reachable_by_hot_paths ();
+  FOR_EACH_BB_FN (bb, cfun)
+   if (!set.contains (bb))
+ BB_SET_PARTITION (bb, BB_COLD_PARTITION);
 }
 
   /* The format of .gcc_except_table does not allow landing pads to
Index: cfgrtl.c
===
--- cfgrtl.c(revision 250378)
+++ cfgrtl.c(working copy)
@@ -2282,6 +2282,29 @@ get_last_bb_insn (basic_block bb)
   return end;
 }
 
+/* Add all BBs reachable from entry via hot paths into the SET.  */
+
+void
+find_bbs_reachable_by_hot_paths (hash_set *set)
+{
+  auto_vec worklist;
+
+  set->add (ENTRY_BLOCK_PTR_FOR_FN (cfun));
+  worklist.safe_push (ENTRY_BLOCK_PTR_FOR_FN (cfun));
+
+  while (worklist.length () > 0)
+{
+  basic_block bb = worklist.pop ();
+  edge_iterator ei;
+  edge e;
+
+  FOR_EACH_EDGE (e, ei, bb->succs)
+   if (BB_PARTITION (e->dest) != BB_COLD_PARTITION
+   && !set->add (e->dest))
+ worklist.safe_push (e->dest);
+}
+}
+
 /* Sanity check partition hotness to ensure that basic blocks in
    the cold partition don't dominate basic blocks in the hot partition.
If FLAG_ONLY is true, report violations as errors. Otherwise
@@ -2295,49 +2318,25 @@ find_partition_fixes (bool flag_only)
   basic_block bb;
   vec bbs_in_cold_partition = vNULL;
   vec bbs_to_fix = vNULL;
+  hash_set set;
 
   /* Callers check this.  */
   gcc_checking_assert (crtl->has_bb_partition);
 
-  FOR_EACH_BB_FN (bb, cfun)
-if ((BB_PARTITION (bb) == BB_COLD_PARTITION))
-  bbs_in_cold_partition.safe_push (bb);
-
-  if (bbs_in_cold_partition.is_empty ())
-return vNULL;
-
-  bool dom_calculated_here = !dom_info_available_p (CDI_DOMINATORS);
-
-  if (dom_calculated_here)
-calculate_dominance_info (CDI_DOMINATORS);
+  find_bbs_reachable_by_hot_paths ();
 
-  while (! bbs_in_cold_partition.is_empty  ())
-{
-  bb = bbs_in_cold_partition.pop ();
-  /* Any blocks dominated by a block in the cold section
- must also be cold.  */
-  basic_block son;
-  for (son = first_dom_son (CDI_DOMINATORS, bb);
-   son;
-   son = next_dom_son (CDI_DOMINATORS, son))
-{
-  /* If son is not yet cold, then mark it cold here and
- enqueue it for further processing.  */
-  if ((BB_PARTITION (son) != BB_COLD_PARTITION))
-{
-  if (flag_only)
-error ("non-cold basic block %d dominated "
-   "by a block in the cold partition (%d)", son->index, 
bb->index);
-  else
-BB_SET_PARTITION (son, BB_COLD_PARTITION);
-  bbs_to_fix.safe_push (son);
-  bbs_in_cold_partition.safe_push (son);
-}
-}
-}
-
-  if (dom_calculated_here)
-free_dominance_info (CDI_DOMINATORS);
+  FOR_EACH_BB_FN (bb, cfun)
+if (!set.contains (bb)
+   && BB_PARTITION (bb) != BB_COLD_PARTITION)
+  {
+   if (flag_only)
+ error ("non-cold basic block %d reachable only "
+"by paths crossing the cold partition", bb->index);
+   else
+ BB_SET_PARTITION (bb, BB_COLD_PARTITION);
+   bbs_to_fix.safe_push (bb);
+   bbs_in_cold_partition.safe_push (bb);
+  }
 
   return bbs_to_fix;
 }
Index: cfgrtl.h
===
--- cfgrtl.h(revision 250378)
+++ cfgrtl.h(working copy)
@@ -54,5 +54,6 @@ extern void cfg_layout_initialize (int);
 extern void cfg_layout_finalize (void);
 

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Eric Botcazou]

> eric@polaris:~/build/gcc/native/gcc> rm ada/sem_util.o
> eric@polaris:~/build/gcc/native/gcc> make ADAFLAGS="-gnatpgn"
> /home/eric/build/gcc/native/./prev-gcc/xgcc -
> B/home/eric/build/gcc/native/./prev-gcc/
> -B/home/eric/install/gcc/x86_64-suse- linux/bin/
> -B/home/eric/install/gcc/x86_64-suse-linux/bin/ -
> B/home/eric/install/gcc/x86_64-suse-linux/lib/ -isystem
> /home/eric/install/gcc/x86_64-suse-linux/include -isystem
> /home/eric/install/gcc/x86_64-suse-linux/sys-include-c -g -O2  -gnatpgn 
> - W -Wall -nostdinc -I- -I. -Iada/generated -Iada
> -I/home/eric/svn/gcc/gcc/ada - I/home/eric/svn/gcc/gcc/ada/gcc-interface
> /home/eric/svn/gcc/gcc/ada/sem_util.adb -o ada/sem_util.o
> 
> raised STORAGE_ERROR : stack overflow or erroneous memory access
> /home/eric/svn/gcc/gcc/ada/gcc-interface/Make-lang.in:119: recipe for target
> 'ada/sem_util.o' failed

We have parent & child functions that are (partially) inlined into each other:

   ---
   -- Set_Debug_Info_Needed --
   ---

   procedure Set_Debug_Info_Needed (T : Entity_Id) is

  procedure Set_Debug_Info_Needed_If_Not_Set (E : Entity_Id);
  pragma Inline (Set_Debug_Info_Needed_If_Not_Set);
  --  Used to set debug info in a related node if not set already

  --
  -- Set_Debug_Info_Needed_If_Not_Set --
  --

  procedure Set_Debug_Info_Needed_If_Not_Set (E : Entity_Id) is
  begin
 if Present (E) and then not Needs_Debug_Info (E) then
Set_Debug_Info_Needed (E);

--  For a private type, indicate that the full view also needs
--  debug information.

if Is_Type (E)
  and then Is_Private_Type (E)
  and then Present (Full_View (E))
then
   Set_Debug_Info_Needed (Full_View (E));
end if;
 end if;
  end Set_Debug_Info_Needed_If_Not_Set;

   --  Start of processing for Set_Debug_Info_Needed

so -fno-partial-inlining is a workaround.

-- 
Eric Botcazou

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Eric Botcazou]

> We have local modifications in the Ada front-end so I cannot reproduce it
> with the pristine tree either. :-(

I apparently screwed up yesterday, this can be reproduced as such in the 
directory of a boostrapped compiler:

eric@polaris:~/build/gcc/native/gcc> rm ada/sem_util.o 
eric@polaris:~/build/gcc/native/gcc> make ADAFLAGS="-gnatpgn"
/home/eric/build/gcc/native/./prev-gcc/xgcc -
B/home/eric/build/gcc/native/./prev-gcc/ -B/home/eric/install/gcc/x86_64-suse-
linux/bin/ -B/home/eric/install/gcc/x86_64-suse-linux/bin/ -
B/home/eric/install/gcc/x86_64-suse-linux/lib/ -isystem 
/home/eric/install/gcc/x86_64-suse-linux/include -isystem 
/home/eric/install/gcc/x86_64-suse-linux/sys-include-c -g -O2  -gnatpgn  -
W -Wall -nostdinc -I- -I. -Iada/generated -Iada -I/home/eric/svn/gcc/gcc/ada -
I/home/eric/svn/gcc/gcc/ada/gcc-interface 
/home/eric/svn/gcc/gcc/ada/sem_util.adb -o ada/sem_util.o

raised STORAGE_ERROR : stack overflow or erroneous memory access
/home/eric/svn/gcc/gcc/ada/gcc-interface/Make-lang.in:119: recipe for target 
'ada/sem_util.o' failed
make: *** [ada/sem_util.o] Error 1

(gdb) bt
#0  0x00a2d4f8 in ggc_internal_alloc(unsigned long, void (*)(void*), 
unsigned long, unsigned long) ()
#1  0x00c2e069 in ggc_internal_cleared_alloc(unsigned long, void (*)
(void*), unsigned long, unsigned long) ()
#2  0x00b23865 in one_reg_loc_descriptor(unsigned int, 
var_init_status)
()
#3  0x00b5ba98 in loc_descriptor(rtx_def*, machine_mode, 
var_init_status) ()
#4  0x00b5c0b2 in loc_descriptor(rtx_def*, machine_mode, 
var_init_status) ()
#5  0x00b5661a in dw_loc_list_1(tree_node*, rtx_def*, int, 
var_init_status) ()
#6  0x00b56e0d in loc_list_from_tree_1(tree_node*, int, 
loc_descr_context*) ()
#7  0x00b59aeb in loc_list_from_tree(tree_node*, int, 
loc_descr_context*) ()
#8  0x00b5e14f in add_location_or_const_value_attribute(die_struct*, 
tree_node*, bool) ()
#9  0x00b60435 in gen_formal_parameter_die(tree_node*, tree_node*, 
bool, die_struct*) ()
#10 0x00b464c3 in gen_decl_die(tree_node*, tree_node*, vlr_context*, 
die_struct*) ()
#11 0x00b6660d in process_scope_var(tree_node*, tree_node*, 
tree_node*, die_struct*) ()
#12 0x00b40e78 in decls_for_scope(tree_node*, die_struct*) ()
#13 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
#14 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#15 0x00b41b7f in gen_subprogram_die(tree_node*, die_struct*) ()
#16 0x00b45c5a in gen_decl_die(tree_node*, tree_node*, vlr_context*, 
die_struct*) ()
#17 0x00b6660d in process_scope_var(tree_node*, tree_node*, 
tree_node*, die_struct*) ()
#18 0x00b40fad in decls_for_scope(tree_node*, die_struct*) ()
#19 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
---Type  to continue, or q  to quit---
#20 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#21 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
#22 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#23 0x00b41b7f in gen_subprogram_die(tree_node*, die_struct*) ()
#24 0x00b45c5a in gen_decl_die(tree_node*, tree_node*, vlr_context*, 
die_struct*) ()
#25 0x00b6660d in process_scope_var(tree_node*, tree_node*, 
tree_node*, die_struct*) ()
#26 0x00b40fad in decls_for_scope(tree_node*, die_struct*) ()
#27 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
#28 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#29 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
#30 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#31 0x00b41b7f in gen_subprogram_die(tree_node*, die_struct*) ()
#32 0x00b45c5a in gen_decl_die(tree_node*, tree_node*, vlr_context*, 
die_struct*) ()
#33 0x00b6660d in process_scope_var(tree_node*, tree_node*, 
tree_node*, die_struct*) ()
#34 0x00b40fad in decls_for_scope(tree_node*, die_struct*) ()
#35 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
#36 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#37 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
#38 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#39 0x00b41b7f in gen_subprogram_die(tree_node*, die_struct*) ()
#40 0x00b45c5a in gen_decl_die(tree_node*, tree_node*, vlr_context*, 
die_struct*) ()
#41 0x00b6660d in process_scope_var(tree_node*, tree_node*, 
tree_node*, die_struct*) ()

with thousands of similar frames.

-- 
Eric Botcazou

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Eric Botcazou]

> The following C testcase shows how profiledbootstrap fails with checking
> compiler.  We have a (nested) FUNCTION_DECL inside of BLOCK_VARS of an
> inline function, when it gets inlined, it is moved into
> BLOCK_NONLOCALIZED_VARS.  And, decls_for_scope calls process_scope_var
> with NULL decl and non-NULL origin for all BLOCK_NONLOCALIZED_VARS.
> That is fine for variables, but for FUNCTION_DECLs it can actually
> try to dwarf2out_abstract_function that FUNCTION_DECL, which should be
> really done only when it is inlined (i.e. BLOCK_ABSTRACT_ORIGIN of
> some BLOCK).  The effect is that we actually add DW_AT_inline attribute
> to that DW_TAG_subroutine, and then later when processing it again
> we add DW_AT_low_pc etc. and ICE, because those attributes should not
> appear on DW_AT_inline functions.
> 
> Fixed by handling FUNCTION_DECLs always the same, whether in BLOCK_VARS
> or BLOCK_NONLOCALIZED_VARS.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2017-03-23  Jakub Jelinek  
> 
>   PR debug/79255
>   * dwarf2out.c (decls_for_scope): If BLOCK_NONLOCALIZED_VAR is
>   a FUNCTION_DECL, pass it as decl instead of origin to
>   process_scope_var.

Thanks for working on this, however the patch breaks bootstrap for us in stage 
#2 using a different set of bootstrap options (-gnatpgn -g).  There appears to 
be an endless recursion :

[...]

#27 0x00b7c929 in process_scope_var (stmt=0x742ebf60, 
decl=0x73c01700, origin=0x0, context_die=0x7fffe2ddd870)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24833
#28 0x00b7ca6f in decls_for_scope (stmt=0x742ebf60, 
context_die=0x7fffe2ddd870)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24867
#29 0x00b77e78 in gen_lexical_block_die (stmt=0x742ebf60, 
context_die=0x7fffe2ddd6e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:23207
#30 0x00b7c638 in gen_block_die (stmt=0x742ebf60, 
context_die=0x7fffe2ddd6e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24795
#31 0x00b7cb07 in decls_for_scope (stmt=0x742ebde0, 
context_die=0x7fffe2ddd6e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24881
#32 0x00b78017 in gen_inlined_subroutine_die (stmt=0x742ebde0, 
context_die=0x7fffe2ddd1e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:23245
#33 0x00b7c623 in gen_block_die (stmt=0x742ebde0, 
context_die=0x7fffe2ddd1e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24792
#34 0x00b7cb07 in decls_for_scope (stmt=0x7449e000, 
---Type  to continue, or q  to quit---
context_die=0x7fffe2ddd1e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24881
#35 0x00b7c64d in gen_block_die (stmt=0x7449e000, 
context_die=0x7fffe2ddd1e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24798
#36 0x00b7cb07 in decls_for_scope (stmt=0x73f7bf00, 
context_die=0x7fffe2ddd1e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24881
#37 0x00b77e78 in gen_lexical_block_die (stmt=0x73f7bf00, 
context_die=0x7fffe2ddaeb0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:23207
#38 0x00b7c638 in gen_block_die (stmt=0x73f7bf00, 
context_die=0x7fffe2ddaeb0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24795
#39 0x00b7cb07 in decls_for_scope (stmt=0x73bf98a0, 
context_die=0x7fffe2ddaeb0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24881
#40 0x00b72766 in gen_subprogram_die (decl=0x73c01700, 
context_die=0x7fffe2ddae60)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:22441
#41 0x00b7e034 in gen_decl_die (decl=0x73c01700, origin=0x0, 
ctx=0x0, context_die=0x7fffe2ddae60)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:25289
#42 0x00b7c929 in process_scope_var (stmt=0x742ebf60, 
decl=0x73c01700, origin=0x0, context_die=0x7fffe2ddae60)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24833

[...]

We have local modifications in the Ada front-end so I cannot reproduce it with 
the pristine tree either. :-(  I'll try harder tomorrow.

-- 
Eric Botcazou

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jakub Jelinek]

On Fri, Mar 24, 2017 at 09:07:54AM -0400, Jason Merrill wrote:
> >> And when it's cloned.
> >>
> >> But does it make sense for gen_decl_die to call
> >> dwarf2out_abstract_function when decl is null?  That seems wrong.
> >
> > Before r144529 we had just:
> >   if (DECL_ORIGIN (decl) != decl)
> > dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
> > and that is indeed to handle clones etc.
> >
> > r144529 changed that to:
> >   if (origin || DECL_ORIGIN (decl) != decl)
> > dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
> > All of the decl is NULL introduced in r144529 implies decl_or_origin
> > is abstract.
> >
> > Removing that origin || wouldn't really work, we'd have to rewrite most of
> > gen_decl_die FUNCTION_DECL handling to use decl_or_origin instead of
> > decl etc.
> 
> I was thinking to change it to
> 
>   if (decl && (origin || DECL_ORIGIN (decl) != decl))

But then you segfault immediately on the next:
  else if (cgraph_function_possibly_inlined_p (decl)
   && ! DECL_ABSTRACT_P (decl)
because decl is NULL.  So, it would be far easier to do:
  if (decl == NULL_TREE)
{
  decl = origin;
  origin = NULL_TREE;
}
before the
  if (origin || DECL_ORIGIN (decl) != decl)
with a comment, rather than to rewrite a couple of dozen decl_or_origin,
and change assumptions that non-NULL origin actually means anything for
FUNCTION_DECL.  That is IMO still uglier than the decls_for_scope change
though, but if you prefer that, I can test that.

Jakub

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jason Merrill]

On Fri, Mar 24, 2017 at 3:46 AM, Jakub Jelinek  wrote:
> On Thu, Mar 23, 2017 at 05:24:31PM -0400, Jason Merrill wrote:
>> On Thu, Mar 23, 2017 at 4:44 PM, Jakub Jelinek  wrote:
>> > The following C testcase shows how profiledbootstrap fails with checking
>> > compiler.  We have a (nested) FUNCTION_DECL inside of BLOCK_VARS of an
>> > inline function, when it gets inlined, it is moved into
>> > BLOCK_NONLOCALIZED_VARS.  And, decls_for_scope calls process_scope_var
>> > with NULL decl and non-NULL origin for all BLOCK_NONLOCALIZED_VARS.
>> > That is fine for variables, but for FUNCTION_DECLs it can actually
>> > try to dwarf2out_abstract_function that FUNCTION_DECL, which should be
>> > really done only when it is inlined (i.e. BLOCK_ABSTRACT_ORIGIN of
>> > some BLOCK).
>>
>> And when it's cloned.
>>
>> But does it make sense for gen_decl_die to call
>> dwarf2out_abstract_function when decl is null?  That seems wrong.
>
> Before r144529 we had just:
>   if (DECL_ORIGIN (decl) != decl)
> dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
> and that is indeed to handle clones etc.
>
> r144529 changed that to:
>   if (origin || DECL_ORIGIN (decl) != decl)
> dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
> All of the decl is NULL introduced in r144529 implies decl_or_origin
> is abstract.
>
> Removing that origin || wouldn't really work, we'd have to rewrite most of
> gen_decl_die FUNCTION_DECL handling to use decl_or_origin instead of
> decl etc.

I was thinking to change it to

  if (decl && (origin || DECL_ORIGIN (decl) != decl))

Jason

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jakub Jelinek]

On Fri, Mar 24, 2017 at 12:45:28PM +0100, Richard Biener wrote:
> On Fri, Mar 24, 2017 at 9:43 AM, Jakub Jelinek  wrote:
> > On Fri, Mar 24, 2017 at 09:29:00AM +0100, Richard Biener wrote:
> >> Yeah, the thing BLOCK_NONLOCALIZED_VARS wants to do is optimize generated
> >> dwarf by adding a DW_AT_abstract_origin (just to refer to the
> >> subprogram DIE) but
> >
> > Well, for FUNCTION_DECLs in BLOCK_VARS/BLOCK_NONLOCALIZED_VARS we actually 
> > don't
> > emit any further DIE and so there is no DW_AT_abstract_origin.
> > E.g. gen_subprogram_die has:
> >   /* Detect and ignore this case, where we are trying to output
> >  something we have already output.  */
> >   if (get_AT (old_die, DW_AT_low_pc)
> >   || get_AT (old_die, DW_AT_ranges))
> > return;
> 
> Hmm, but we do want to put the function in scope?  THus
> 
> void foo () {}
> void bar ()
> {
>   int foo;
>   {
>  void foo();
>  foo();
>   }
> }
> 
> should have a DIE for foo in bar (possibly refering to the concrete instance
> for optimization).

We actually do that.  If I change your testcase so that it actually triggers
the changed part of the code (so there is inlining etc.), so:
volatile int v;
__attribute__((noinline)) void foo () { v++; }
static inline void bar ()
{
  int foo;
  {
 void foo();
 foo();
  }
}
void
baz (void)
{
  bar ();
  bar ();
}
then at -gdwarf-3 -dA -O2 the difference between vanilla GCC and my patched GCC 
is
following (used -gdwarf-3 so that there are no DW_FORM_flag_present that
result in DIE offset changes):
.uleb128 0xd# (DIE (0x117) DW_TAG_subprogram)
.ascii "bar\0"  # DW_AT_name
.byte   0x1 # DW_AT_decl_file (prQQ.c)
.byte   0x3 # DW_AT_decl_line
.byte   0x3 # DW_AT_inline
.long   0x13c   # DW_AT_sibling
.uleb128 0xe# (DIE (0x123) DW_TAG_variable)
.ascii "foo\0"  # DW_AT_name
.byte   0x1 # DW_AT_decl_file (prQQ.c)
.byte   0x5 # DW_AT_decl_line
.long   0x41# DW_AT_type
.uleb128 0xf# (DIE (0x12e) DW_TAG_lexical_block)
.uleb128 0x10   # (DIE (0x12f) DW_TAG_subprogram)
.byte   0x1 # DW_AT_external
.ascii "foo\0"  # DW_AT_name
.byte   0x1 # DW_AT_decl_file (prQQ.c)
.byte   0x7 # DW_AT_decl_line
-   .byte   0   # DW_AT_inline
+   .byte   0x1 # DW_AT_declaration
.uleb128 0x11   # (DIE (0x138) DW_TAG_unspecified_parameters)
.byte   0   # end of children of DIE 0x12f
.byte   0   # end of children of DIE 0x12e
.byte   0   # end of children of DIE 0x117
.uleb128 0x12   # (DIE (0x13c) DW_TAG_subprogram)
.byte   0x1 # DW_AT_external
.ascii "foo\0"  # DW_AT_name
.byte   0x1 # DW_AT_decl_file (prQQ.c)
.byte   0x2 # DW_AT_decl_line
.quad   .LFB0   # DW_AT_low_pc
.quad   .LFE0   # DW_AT_high_pc
.byte   0x1 # DW_AT_frame_base
.byte   0x9c# DW_OP_call_frame_cfa
.byte   0x1 # DW_AT_GNU_all_call_sites
.byte   0   # end of children of DIE 0xb
and corresponding change in .debug_abbrev.
That seems to be the right change to me, inside of bar (the DW_AT_inline
one) we have a declaration of foo, and we properly use DW_AT_declaration for
it, then at the toplevel we actually have the full definition die for foo,
and then we have in two spots inside baz
.uleb128 0x6# (DIE (0x6d) DW_TAG_inlined_subroutine)
.long   0x117   # DW_AT_abstract_origin
without the need to duplicate the foo declaration in there,
that is inherited through DW_AT_abstract_origin.

Jakub

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Richard Biener]

On Fri, Mar 24, 2017 at 9:43 AM, Jakub Jelinek  wrote:
> On Fri, Mar 24, 2017 at 09:29:00AM +0100, Richard Biener wrote:
>> Yeah, the thing BLOCK_NONLOCALIZED_VARS wants to do is optimize generated
>> dwarf by adding a DW_AT_abstract_origin (just to refer to the
>> subprogram DIE) but
>
> Well, for FUNCTION_DECLs in BLOCK_VARS/BLOCK_NONLOCALIZED_VARS we actually 
> don't
> emit any further DIE and so there is no DW_AT_abstract_origin.
> E.g. gen_subprogram_die has:
>   /* Detect and ignore this case, where we are trying to output
>  something we have already output.  */
>   if (get_AT (old_die, DW_AT_low_pc)
>   || get_AT (old_die, DW_AT_ranges))
> return;

Hmm, but we do want to put the function in scope?  THus

void foo () {}
void bar ()
{
  int foo;
  {
 void foo();
 foo();
  }
}

should have a DIE for foo in bar (possibly refering to the concrete instance
for optimization).

Richard.

> That is why the posted testcase doesn't ICE without -fno-toplevel-reorder,
> normally the body is emitted earlier and so we don't do anything at all.
> Otherwise we just want to make sure we have a DIE and, if it is
> inline/clone, have also DW_AT_inline set, and if the DIE is without parent
> that we put it into proper place in the DIE tree.  And when we actually
> see the body of the function we fill locations and all other details.
>
> Jakub

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jakub Jelinek]

On Fri, Mar 24, 2017 at 09:29:00AM +0100, Richard Biener wrote:
> Yeah, the thing BLOCK_NONLOCALIZED_VARS wants to do is optimize generated
> dwarf by adding a DW_AT_abstract_origin (just to refer to the
> subprogram DIE) but

Well, for FUNCTION_DECLs in BLOCK_VARS/BLOCK_NONLOCALIZED_VARS we actually don't
emit any further DIE and so there is no DW_AT_abstract_origin.
E.g. gen_subprogram_die has:
  /* Detect and ignore this case, where we are trying to output
 something we have already output.  */
  if (get_AT (old_die, DW_AT_low_pc)
  || get_AT (old_die, DW_AT_ranges))
return;
That is why the posted testcase doesn't ICE without -fno-toplevel-reorder,
normally the body is emitted earlier and so we don't do anything at all.
Otherwise we just want to make sure we have a DIE and, if it is
inline/clone, have also DW_AT_inline set, and if the DIE is without parent
that we put it into proper place in the DIE tree.  And when we actually
see the body of the function we fill locations and all other details.

Jakub

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Richard Biener]

On Fri, Mar 24, 2017 at 8:46 AM, Jakub Jelinek  wrote:
> On Thu, Mar 23, 2017 at 05:24:31PM -0400, Jason Merrill wrote:
>> On Thu, Mar 23, 2017 at 4:44 PM, Jakub Jelinek  wrote:
>> > The following C testcase shows how profiledbootstrap fails with checking
>> > compiler.  We have a (nested) FUNCTION_DECL inside of BLOCK_VARS of an
>> > inline function, when it gets inlined, it is moved into
>> > BLOCK_NONLOCALIZED_VARS.  And, decls_for_scope calls process_scope_var
>> > with NULL decl and non-NULL origin for all BLOCK_NONLOCALIZED_VARS.
>> > That is fine for variables, but for FUNCTION_DECLs it can actually
>> > try to dwarf2out_abstract_function that FUNCTION_DECL, which should be
>> > really done only when it is inlined (i.e. BLOCK_ABSTRACT_ORIGIN of
>> > some BLOCK).
>>
>> And when it's cloned.
>>
>> But does it make sense for gen_decl_die to call
>> dwarf2out_abstract_function when decl is null?  That seems wrong.
>
> Before r144529 we had just:
>   if (DECL_ORIGIN (decl) != decl)
> dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
> and that is indeed to handle clones etc.
>
> r144529 changed that to:
>   if (origin || DECL_ORIGIN (decl) != decl)
> dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
> All of the decl is NULL introduced in r144529 implies decl_or_origin
> is abstract.
>
> Removing that origin || wouldn't really work, we'd have to rewrite most of
> gen_decl_die FUNCTION_DECL handling to use decl_or_origin instead of
> decl etc.
>
> But it doesn't look right to me, conceptually a FUNCTION_DECL in
> BLOCK_NONLOCALIZED_VARS is nothing that represents a clone or something
> abstract in itself.  We can't keep it in BLOCK_VARS just because those
> are chained through DECL_CHAIN and a single FUNCTION_DECL can't be
> put into multiple BLOCK_VARS chains.  It is still the same FUNCTION_DECL,
> not a sign that we want to have in each inline function a separate
> function declaration with abstract origin of the original FUNCTION_DECL.

Yeah, the thing BLOCK_NONLOCALIZED_VARS wants to do is optimize generated
dwarf by adding a DW_AT_abstract_origin (just to refer to the
subprogram DIE) but
it doesn't actually care about refering to an abstract instance (a
concrete one would
work just fine).

So I think in the context of scope vars calling
dwarf2out_abstract_function is _always_
wrong.  But it would require quite some refactoring to do this in a clean way.

Richard.

> Jakub

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jakub Jelinek]

On Thu, Mar 23, 2017 at 05:24:31PM -0400, Jason Merrill wrote:
> On Thu, Mar 23, 2017 at 4:44 PM, Jakub Jelinek  wrote:
> > The following C testcase shows how profiledbootstrap fails with checking
> > compiler.  We have a (nested) FUNCTION_DECL inside of BLOCK_VARS of an
> > inline function, when it gets inlined, it is moved into
> > BLOCK_NONLOCALIZED_VARS.  And, decls_for_scope calls process_scope_var
> > with NULL decl and non-NULL origin for all BLOCK_NONLOCALIZED_VARS.
> > That is fine for variables, but for FUNCTION_DECLs it can actually
> > try to dwarf2out_abstract_function that FUNCTION_DECL, which should be
> > really done only when it is inlined (i.e. BLOCK_ABSTRACT_ORIGIN of
> > some BLOCK).
> 
> And when it's cloned.
> 
> But does it make sense for gen_decl_die to call
> dwarf2out_abstract_function when decl is null?  That seems wrong.

Before r144529 we had just:
  if (DECL_ORIGIN (decl) != decl)
dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
and that is indeed to handle clones etc.

r144529 changed that to:
  if (origin || DECL_ORIGIN (decl) != decl)
dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
All of the decl is NULL introduced in r144529 implies decl_or_origin
is abstract.

Removing that origin || wouldn't really work, we'd have to rewrite most of
gen_decl_die FUNCTION_DECL handling to use decl_or_origin instead of
decl etc.

But it doesn't look right to me, conceptually a FUNCTION_DECL in
BLOCK_NONLOCALIZED_VARS is nothing that represents a clone or something
abstract in itself.  We can't keep it in BLOCK_VARS just because those
are chained through DECL_CHAIN and a single FUNCTION_DECL can't be
put into multiple BLOCK_VARS chains.  It is still the same FUNCTION_DECL,
not a sign that we want to have in each inline function a separate
function declaration with abstract origin of the original FUNCTION_DECL.

Jakub

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jason Merrill]

On Thu, Mar 23, 2017 at 4:44 PM, Jakub Jelinek  wrote:
> The following C testcase shows how profiledbootstrap fails with checking
> compiler.  We have a (nested) FUNCTION_DECL inside of BLOCK_VARS of an
> inline function, when it gets inlined, it is moved into
> BLOCK_NONLOCALIZED_VARS.  And, decls_for_scope calls process_scope_var
> with NULL decl and non-NULL origin for all BLOCK_NONLOCALIZED_VARS.
> That is fine for variables, but for FUNCTION_DECLs it can actually
> try to dwarf2out_abstract_function that FUNCTION_DECL, which should be
> really done only when it is inlined (i.e. BLOCK_ABSTRACT_ORIGIN of
> some BLOCK).

And when it's cloned.

But does it make sense for gen_decl_die to call
dwarf2out_abstract_function when decl is null?  That seems wrong.

Jason

[PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jakub Jelinek]

Hi!

The following C testcase shows how profiledbootstrap fails with checking
compiler.  We have a (nested) FUNCTION_DECL inside of BLOCK_VARS of an
inline function, when it gets inlined, it is moved into
BLOCK_NONLOCALIZED_VARS.  And, decls_for_scope calls process_scope_var
with NULL decl and non-NULL origin for all BLOCK_NONLOCALIZED_VARS.
That is fine for variables, but for FUNCTION_DECLs it can actually
try to dwarf2out_abstract_function that FUNCTION_DECL, which should be
really done only when it is inlined (i.e. BLOCK_ABSTRACT_ORIGIN of
some BLOCK).  The effect is that we actually add DW_AT_inline attribute
to that DW_TAG_subroutine, and then later when processing it again
we add DW_AT_low_pc etc. and ICE, because those attributes should not
appear on DW_AT_inline functions.

Fixed by handling FUNCTION_DECLs always the same, whether in BLOCK_VARS
or BLOCK_NONLOCALIZED_VARS.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-03-23  Jakub Jelinek  

PR debug/79255
* dwarf2out.c (decls_for_scope): If BLOCK_NONLOCALIZED_VAR is
a FUNCTION_DECL, pass it as decl instead of origin to
process_scope_var.

* gcc.dg/pr79255.c: New test.

--- gcc/dwarf2out.c.jj  2017-03-22 19:31:41.525055795 +0100
+++ gcc/dwarf2out.c 2017-03-23 17:57:09.419362739 +0100
@@ -24861,8 +24861,13 @@ decls_for_scope (tree stmt, dw_die_ref c
 if we've done it once already.  */
   if (! early_dwarf)
for (i = 0; i < BLOCK_NUM_NONLOCALIZED_VARS (stmt); i++)
- process_scope_var (stmt, NULL, BLOCK_NONLOCALIZED_VAR (stmt, i),
-context_die);
+ {
+   decl = BLOCK_NONLOCALIZED_VAR (stmt, i);
+   if (TREE_CODE (decl) == FUNCTION_DECL)
+ process_scope_var (stmt, decl, NULL_TREE, context_die);
+   else
+ process_scope_var (stmt, NULL_TREE, decl, context_die);
+ }
 }
 
   /* Even if we're at -g1, we need to process the subblocks in order to get
--- gcc/testsuite/gcc.dg/pr79255.c.jj   2017-03-23 17:57:44.711911298 +0100
+++ gcc/testsuite/gcc.dg/pr79255.c  2017-03-23 17:56:24.0 +0100
@@ -0,0 +1,21 @@
+/* PR bootstrap/79255 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -g -fno-toplevel-reorder -Wno-attributes" } */
+
+static inline __attribute__((always_inline)) int foo (int x);
+
+int
+baz (void)
+{
+  return foo (3) + foo (6) + foo (9);
+}
+
+static inline __attribute__((always_inline)) int
+foo (int x)
+{
+  auto inline int __attribute__((noinline)) bar (int x)
+  {
+return x + 3;
+  }
+  return bar (x) + bar (x + 2);
+}

Jakub

Re: [PATCH] fix profiledbootstrap with -Werror=format-length (77753)

[Martin Sebor]

On 09/27/2016 10:38 PM, Jeff Law wrote:

On 09/27/2016 01:30 PM, Martin Sebor wrote:

The attached one line patch increases a local buffer size to
avoid an apparently justified (though in reality likely a false
positive) -Wformat-length warning in varasm.c.  The warning has
been reported to break profiledbootstrap on powerp64le (though
not ordinary bootstrap).

An arguably better solution would be to explicitly constrain
the range of the integer variable to that of valid priority values.
Unfortunately, as pointed out in bug 77721, due to the limitation
of the current VRP implementation, this isn't always reliable.

Martin

gcc-77753.diff


PR bootstrap/77753 - [7 Regression] broken profiledbootstrap due to
-Werror=format-length in varasm.c:1573

gcc/ChangeLog:
2016-09-27  Martin Sebor  

PR bootstrap/77753
* varasm.c (assemble_addr_to_section): Increase local buffer size.

OK.

WRT 77721, could you add that test to the testsuite, but xfailed?


Sure.  Done in r240595.

Martin

Re: [PATCH] fix profiledbootstrap with -Werror=format-length (77753)

[Jeff Law]

On 09/27/2016 01:30 PM, Martin Sebor wrote:

The attached one line patch increases a local buffer size to
avoid an apparently justified (though in reality likely a false
positive) -Wformat-length warning in varasm.c.  The warning has
been reported to break profiledbootstrap on powerp64le (though
not ordinary bootstrap).

An arguably better solution would be to explicitly constrain
the range of the integer variable to that of valid priority values.
Unfortunately, as pointed out in bug 77721, due to the limitation
of the current VRP implementation, this isn't always reliable.

Martin

gcc-77753.diff


PR bootstrap/77753 - [7 Regression] broken profiledbootstrap due to
-Werror=format-length in varasm.c:1573

gcc/ChangeLog:
2016-09-27  Martin Sebor  

PR bootstrap/77753
* varasm.c (assemble_addr_to_section): Increase local buffer size.

OK.

WRT 77721, could you add that test to the testsuite, but xfailed?

Jeff

[PATCH] fix profiledbootstrap with -Werror=format-length (77753)

[Martin Sebor]

The attached one line patch increases a local buffer size to
avoid an apparently justified (though in reality likely a false
positive) -Wformat-length warning in varasm.c.  The warning has
been reported to break profiledbootstrap on powerp64le (though
not ordinary bootstrap).

An arguably better solution would be to explicitly constrain
the range of the integer variable to that of valid priority values.
Unfortunately, as pointed out in bug 77721, due to the limitation
of the current VRP implementation, this isn't always reliable.

Martin
PR bootstrap/77753 - [7 Regression] broken profiledbootstrap due to
	-Werror=format-length in varasm.c:1573

gcc/ChangeLog:
2016-09-27  Martin Sebor  

	PR bootstrap/77753
	* varasm.c (assemble_addr_to_section): Increase local buffer size.

Index: gcc/varasm.c
===
--- gcc/varasm.c	(revision 240556)
+++ gcc/varasm.c	(working copy)
@@ -1556,7 +1556,9 @@ assemble_addr_to_section (rtx symbol, se
 section *
 get_cdtor_priority_section (int priority, bool constructor_p)
 {
-  char buf[16];
+  /* Buffer conservatively large enough for the full range of a 32-bit
+ int plus the text below.  */
+  char buf[18];
 
   /* ??? This only works reliably with the GNU linker.  */
   sprintf (buf, "%s.%.5u",

Re: Fix profiledbootstrap with release checking

[Markus Trippelsdorf]

On 2015.01.16 at 00:15 +0100, Jan Hubicka wrote:
 Hi,
 can_remove_node_now_p assumes that the node in question has no direct calls, 
 this
 however is not checked in inline_call that leads to alias to be removed from
 comdat group while it should not.
 
 Bootstrapped/regtested x86_64-linux, comitted.

Here's a testcase for this issue. OK for trunk?

2015-01-16  Markus Trippelsdorf  mar...@trippelsdorf.de

PR ipa/64163 
PR ipa/64612
* g++.dg/ipa/pr64612.C: New test.

/* { dg-do compile } */
/* { dg-options -fPIC -O3 -std=c++11 } */
/* { dg-final { scan-assembler _ZN5QListI7QStringED1Ev } } */

class A
{
public:
  bool deref ();
};
class QString;
struct B
{
  A ref;
};
template typename class QList
{
  B d;
public:
  ~QList ();
  class const_iterator
  {
  };
  const_iterator constBegin ();
  void clear ();
  void dealloc ();
};
template typename T QListT::~QList ()
{
  if (d.ref.deref ())
dealloc ();
}
template typename T
void
QListT::clear ()
{
  QList ();
}
class A1 : public QListQString
{
};
class B1
{
public:
  B1 (A1);
};
struct F
{
  void addMatch (const QString );
  A1 m_matchingMimeTypes;
};
class G
{
  A1 matchingGlobs (const QString ) const;
};
void
F::addMatch (const QString )
{
  m_matchingMimeTypes.clear ();
}
A1
G::matchingGlobs (const QString ) const
{
  A1 a;
  for (B1 b (a);;)
;
}

-- 
Markus

Re: Fix profiledbootstrap with release checking

[Richard Biener]

On Fri, Jan 16, 2015 at 9:34 AM, Markus Trippelsdorf
mar...@trippelsdorf.de wrote:
 On 2015.01.16 at 00:15 +0100, Jan Hubicka wrote:
 Hi,
 can_remove_node_now_p assumes that the node in question has no direct calls, 
 this
 however is not checked in inline_call that leads to alias to be removed from
 comdat group while it should not.

 Bootstrapped/regtested x86_64-linux, comitted.

 Here's a testcase for this issue. OK for trunk?

Ok.

Thanks,
Richard.

 2015-01-16  Markus Trippelsdorf  mar...@trippelsdorf.de

 PR ipa/64163
 PR ipa/64612
 * g++.dg/ipa/pr64612.C: New test.

 /* { dg-do compile } */
 /* { dg-options -fPIC -O3 -std=c++11 } */
 /* { dg-final { scan-assembler _ZN5QListI7QStringED1Ev } } */

 class A
 {
 public:
   bool deref ();
 };
 class QString;
 struct B
 {
   A ref;
 };
 template typename class QList
 {
   B d;
 public:
   ~QList ();
   class const_iterator
   {
   };
   const_iterator constBegin ();
   void clear ();
   void dealloc ();
 };
 template typename T QListT::~QList ()
 {
   if (d.ref.deref ())
 dealloc ();
 }
 template typename T
 void
 QListT::clear ()
 {
   QList ();
 }
 class A1 : public QListQString
 {
 };
 class B1
 {
 public:
   B1 (A1);
 };
 struct F
 {
   void addMatch (const QString );
   A1 m_matchingMimeTypes;
 };
 class G
 {
   A1 matchingGlobs (const QString ) const;
 };
 void
 F::addMatch (const QString )
 {
   m_matchingMimeTypes.clear ();
 }
 A1
 G::matchingGlobs (const QString ) const
 {
   A1 a;
   for (B1 b (a);;)
 ;
 }

 --
 Markus

Re: Fix profiledbootstrap with release checking

[Markus Trippelsdorf]

On 2015.01.16 at 12:03 +0100, Richard Biener wrote:
 On Fri, Jan 16, 2015 at 9:34 AM, Markus Trippelsdorf
 mar...@trippelsdorf.de wrote:
  On 2015.01.16 at 00:15 +0100, Jan Hubicka wrote:
  Hi,
  can_remove_node_now_p assumes that the node in question has no direct 
  calls, this
  however is not checked in inline_call that leads to alias to be removed 
  from
  comdat group while it should not.
 
  Bootstrapped/regtested x86_64-linux, comitted.
 
  Here's a testcase for this issue. OK for trunk?
 
 Ok.

Thanks, committed. I dropped the -fPIC flag, because it isn't necessary.

-- 
Markus

Fix profiledbootstrap with release checking

[Jan Hubicka]

Hi,
can_remove_node_now_p assumes that the node in question has no direct calls, 
this
however is not checked in inline_call that leads to alias to be removed from
comdat group while it should not.

Bootstrapped/regtested x86_64-linux, comitted.

Honza

PR ipa/64612
* ipa-inline-transform.c (can_remove_node_now_p): Fix handling
of comdat locals.
(inline_call): Fix removal of aliases.
Index: ipa-inline-transform.c
===
--- ipa-inline-transform.c  (revision 219674)
+++ ipa-inline-transform.c  (working copy)
@@ -139,7 +139,7 @@ can_remove_node_now_p (struct cgraph_nod
 
   /* When we see same comdat group, we need to be sure that all
  items can be removed.  */
-  if (!node-same_comdat_group)
+  if (!node-same_comdat_group || !node-externally_visible)
 return true;
   for (next = dyn_castcgraph_node * (node-same_comdat_group);
next != node; next = dyn_castcgraph_node * (next-same_comdat_group))
@@ -310,7 +310,8 @@ inline_call (struct cgraph_edge *e, bool
   while (alias  alias != callee)
{
  if (!alias-callers
-  can_remove_node_now_p (alias, e))
+  can_remove_node_now_p (alias,
+   !e-next_caller  !e-prev_caller ? e 
: NULL))
{
  next_alias = alias-get_alias_target ();
  alias-remove ();

[google gcc-4_8] fix profiledbootstrap

[Rong Xu]

Hi,

The attached patch fixes the duplicated definition error in
gcov-tool.c in profiledbootstrap.

Google branch only and tested with profiledbootstrap.

Ok for checking in?

-Rong
Index: gcov-tool.c
===
--- gcov-tool.c (revision 207435)
+++ gcov-tool.c (working copy)
@@ -50,16 +50,23 @@
 
 static int verbose;
 
-gcov_unsigned_t __gcov_lipo_grouping_algorithm;
-gcov_unsigned_t __gcov_lipo_merge_modu_edges;
-gcov_unsigned_t __gcov_lipo_weak_inclusion;
-gcov_unsigned_t __gcov_lipo_max_mem;
-gcov_unsigned_t __gcov_lipo_random_group_size;
-gcov_unsigned_t __gcov_lipo_cutoff;
-gcov_unsigned_t __gcov_lipo_random_seed;
-gcov_unsigned_t __gcov_lipo_dump_cgraph;
-gcov_unsigned_t __gcov_lipo_propagate_scale;
+/* The following defines are needed by dyn-ipa.c.
+   They will also be emitted by the compiler with -fprofile-generate,
+   which means this file cannot be compiled with -fprofile-generate
+   -- otherwise we get duplicated defintions.
+   Make the defines weak to link with other objects/libraries
+   that potentially compiled with -fprofile-generate.  */
 
+__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_grouping_algorithm;
+__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_merge_modu_edges;
+__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_weak_inclusion;
+__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_max_mem;
+__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_random_group_size;
+__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_cutoff;
+__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_random_seed;
+__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_dump_cgraph;
+__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_propagate_scale;
+
 /* Remove file NAME if it has a gcda suffix. */
 
 static int
Index: Makefile.in
===
--- Makefile.in (revision 207435)
+++ Makefile.in (working copy)
@@ -4090,6 +4090,9 @@
 GCOV_TOOL_OBJS = gcov-tool.o libgcov-util.o dyn-ipa.o gcov-params.o
 gcov-tool.o: gcov-tool.c $(GCOV_IO_H) intl.h $(SYSTEM_H) coretypes.h $(TM_H) \
$(CONFIG_H) version.h $(DIAGNOSTIC_H)
+   +$(COMPILER) -c $(ALL_CPPFLAGS) \
+ $(filter-out -fprofile-generate -fprofile-use,$(ALL_COMPILERFLAGS)) \
+ $(INCLUDES) -o $@ $
 gcov-params.o: params.c $(PARAMS_H)
+$(COMPILER) -DIN_GCOV_TOOL -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) \
  $(INCLUDES) -o $@ $

Re: [google gcc-4_8] fix profiledbootstrap

[Xinliang David Li]

ok.

David

On Tue, Feb 4, 2014 at 11:28 AM, Rong Xu x...@google.com wrote:
 Hi,

 The attached patch fixes the duplicated definition error in
 gcov-tool.c in profiledbootstrap.

 Google branch only and tested with profiledbootstrap.

 Ok for checking in?

 -Rong

Re: [PATCH, bootstrap]: Initialize deref_align in ipa_modify_call_arguments to fix profiledbootstrap

[Richard Biener]

On Sat, Sep 7, 2013 at 10:15 AM, Uros Bizjak ubiz...@gmail.com wrote:
 Hello!

 It looks that it is too hard for the compiler to track deref_align
 initialization through dependent deref_base boolean. The patch bellow
 fixes may be used uninitialized warning that breaks
 profiledbootstrap.

 2013-09-07  Uros Bizjak  ubiz...@gmail.com

 * ipa-prop.c (ipa_modify_call_arguments): Initialize deref_align.

 Tested on x86_64-pc-linux-gnu with LTO profiledbootstrap.

 OK for mainline?

Ok.

Thanks,
Richard.

 Uros.

[PATCH, bootstrap]: Initialize deref_align in ipa_modify_call_arguments to fix profiledbootstrap

[Uros Bizjak]

Hello!

It looks that it is too hard for the compiler to track deref_align
initialization through dependent deref_base boolean. The patch bellow
fixes may be used uninitialized warning that breaks
profiledbootstrap.

2013-09-07  Uros Bizjak  ubiz...@gmail.com

* ipa-prop.c (ipa_modify_call_arguments): Initialize deref_align.

Tested on x86_64-pc-linux-gnu with LTO profiledbootstrap.

OK for mainline?

Uros.
Index: ipa-prop.c
===
--- ipa-prop.c  (revision 202352)
+++ ipa-prop.c  (working copy)
@@ -3526,7 +3526,7 @@ ipa_modify_call_arguments (struct cgraph_edge *cs,
{
  tree expr, base, off;
  location_t loc;
- unsigned int deref_align;
+ unsigned int deref_align = 0;
  bool deref_base = false;
 
  /* We create a new parameter out of the value of the old one, we can

[Ping] [Google] Fix profiledbootstrap failure

[Dinar Temirbulatov]

Ping?
Hi,
Here is the patch, Tested by profiledbootstrap. Ok for google gcc-4.8?
thanks, Dinar.


profiledbootstrap-fix1.patch
Description: Binary data

Re: [Ping] [Google] Fix profiledbootstrap failure

[Xinliang David Li]

Sorry for the delay.  The patch is ok and I have committed it to the
google branch.

thanks,

David

On Thu, Aug 1, 2013 at 4:51 PM, Dinar Temirbulatov di...@kugelworks.com wrote:
 Ping?
 Hi,
 Here is the patch, Tested by profiledbootstrap. Ok for google gcc-4.8?
 thanks, Dinar.

Re: [Google] Fix profiledbootstrap failure

[Dinar Temirbulatov]

Hi,
Here is the patch, Tested by profiledbootstrap. Ok for google gcc-4.8?
thanks, Dinar.

On Wed, Jul 31, 2013 at 12:01 AM, Rong Xu x...@google.com wrote:
 Will do.

 The patch was in gcc-4_7 by Dehao.

 r194713 | dehao | 2012-12-24 16:49:06 -0800 (Mon, 24 Dec 2012) | 5 lines

 Fix the profiled bootstrap:

 1. Set the default value of gcov-debug to be 0.
 2. Merge profile summaries from different instrumented binaries.

 On Tue, Jul 30, 2013 at 12:58 PM, Xinliang David Li davi...@google.com 
 wrote:
 Ok. Rong, can you help commit the parameter default setting patch?

 thanks,

 David

 On Tue, Jul 30, 2013 at 12:48 PM, Rong Xu x...@google.com wrote:
 We have seen the issue before. It does fail the profile boostrap as it
 reads a wrong gcda file.
 I thought it had been fixed. (The fix was as David mentioned, setting
 the default value of the parameter to 0).

 -Rong

 On Tue, Jul 30, 2013 at 12:02 PM, Xinliang David Li davi...@google.com 
 wrote:
 I need to understand why this affects profile bootstrap -- is this due
 to file name conflict?

 The fix is wrong -- please do not remove the parameter. If it is a
 problem, a better fix is to change the default parameter value to 0.

 David


 On Tue, Jul 30, 2013 at 11:56 AM, Teresa Johnson tejohn...@google.com 
 wrote:
 cc'ing Rong and David since this came from LIPO support.

 The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on
 by default) without removing the parameter itself. What is the failure
 mode you see from this code?

 Thanks,
 Teresa

 On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov
 di...@kugelworks.com wrote:
 Hello

 This change allows to complete profiledbootstrap on the google gcc-4.8
 branch, tested with make bootstrap with no new regressions.  OK for
 google 4.8?
thanks, Dinar.



 --
 Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413


profiledbootstrap-fix1.patch
Description: Binary data

[Google] Fix profiledbootstrap failure

[Dinar Temirbulatov]

Hello

This change allows to complete profiledbootstrap on the google gcc-4.8
branch, tested with make bootstrap with no new regressions.  OK for
google 4.8?
   thanks, Dinar.


profiledbootstrap-fix.patch
Description: Binary data

Re: [Google] Fix profiledbootstrap failure

[Teresa Johnson]

cc'ing Rong and David since this came from LIPO support.

The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on
by default) without removing the parameter itself. What is the failure
mode you see from this code?

Thanks,
Teresa

On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov
di...@kugelworks.com wrote:
 Hello

 This change allows to complete profiledbootstrap on the google gcc-4.8
 branch, tested with make bootstrap with no new regressions.  OK for
 google 4.8?
thanks, Dinar.



-- 
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413

Re: [Google] Fix profiledbootstrap failure

[Xinliang David Li]

I need to understand why this affects profile bootstrap -- is this due
to file name conflict?

The fix is wrong -- please do not remove the parameter. If it is a
problem, a better fix is to change the default parameter value to 0.

David


On Tue, Jul 30, 2013 at 11:56 AM, Teresa Johnson tejohn...@google.com wrote:
 cc'ing Rong and David since this came from LIPO support.

 The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on
 by default) without removing the parameter itself. What is the failure
 mode you see from this code?

 Thanks,
 Teresa

 On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov
 di...@kugelworks.com wrote:
 Hello

 This change allows to complete profiledbootstrap on the google gcc-4.8
 branch, tested with make bootstrap with no new regressions.  OK for
 google 4.8?
thanks, Dinar.



 --
 Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413

Re: [Google] Fix profiledbootstrap failure

[Rong Xu]

We have seen the issue before. It does fail the profile boostrap as it
reads a wrong gcda file.
I thought it had been fixed. (The fix was as David mentioned, setting
the default value of the parameter to 0).

-Rong

On Tue, Jul 30, 2013 at 12:02 PM, Xinliang David Li davi...@google.com wrote:
 I need to understand why this affects profile bootstrap -- is this due
 to file name conflict?

 The fix is wrong -- please do not remove the parameter. If it is a
 problem, a better fix is to change the default parameter value to 0.

 David


 On Tue, Jul 30, 2013 at 11:56 AM, Teresa Johnson tejohn...@google.com wrote:
 cc'ing Rong and David since this came from LIPO support.

 The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on
 by default) without removing the parameter itself. What is the failure
 mode you see from this code?

 Thanks,
 Teresa

 On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov
 di...@kugelworks.com wrote:
 Hello

 This change allows to complete profiledbootstrap on the google gcc-4.8
 branch, tested with make bootstrap with no new regressions.  OK for
 google 4.8?
thanks, Dinar.



 --
 Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413

Re: [Google] Fix profiledbootstrap failure

[Dinar Temirbulatov]

I need to understand why this affects profile bootstrap -- is this due
to file name conflict?
Yes, It is simple. During the profiledbootstrap on x86_64 platform for
libiberty the compiler picks an incorrect profile from the current
directory(non-pic version), while compiling pic version, and
profiledbootstrap fails.
Here is log:
if [ x-fpic != x ]; then \
  /home/dinar/bugz/x86_64/build-gcc-4_8-svn-orig/./prev-gcc/xgcc
-B/home/dinar/bugz/x86_64/bisect/build-gcc-4_8-svn-orig/./prev-gcc/
-B/tmp/x86_64-unknown-linux-gnu/bin/
-B/tmp/x86_64-unknown-linux-gnu/bin/
-B/tmp/x86_64-unknown-linux-gnu/lib/ -isystem
/tmp/x86_64-unknown-linux-gnu/include -isystem
/tmp/x86_64-unknown-linux-gnu/sys-include-c -DHAVE_CONFIG_H -g -O2
-fprofile-use  -I. -I../../gcc-4_8-svn-orig/libiberty/../include  -W
-Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic
-fpic ../../gcc-4_8-svn-orig/libiberty/dwarfnames.c -o
pic/dwarfnames.o; \
else true; fi
yes
../../gcc-4_8-svn-orig/libiberty/dwarfnames.c:75:0: error: the control
flow of function ‘get_DW_CFA_name’ does not match its profile data
(counter ‘arcs’) [-Werror=coverage-mismatch]

 ^
../../gcc-4_8-svn-orig/libiberty/dwarfnames.c:75:0: note: use
-Wno-error=coverage-mismatch to tolerate the mismatch but performance
may drop if the function is hot
../../gcc-4_8-svn-orig/libiberty/dwarfnames.c: In function ‘get_DW_ATE_name’:
../../gcc-4_8-svn-orig/libiberty/dwarfnames.c:75:0: error: the control
flow of function ‘get_DW_ATE_name’ does not match its profile data
(counter ‘arcs’) [-Werror=coverage-mismatch]
../../gcc-4_8-svn-orig/libiberty/dwarfnames.c:75:0: note: use
-Wno-error=coverage-mismatch to tolerate the mismatch but performance
may drop if the function is hot
../../gcc-4_8-svn-orig/libiberty/dwarfnames.c: In function ‘get_DW_OP_name’:
../../gcc-4_8-svn-orig/libiberty/dwarfnames.c:75:0: error: the control
flow of function ‘get_DW_OP_name’ does not match its profile data
(counter ‘arcs’) [-Werror=coverage-mismatch]
../../gcc-4_8-svn-orig/libiberty/dwarfnames.c:75:0: note: use
-Wno-error=coverage-mismatch to tolerate the mismatch but performance
may drop if the function is hot
thanks, Dinar.

On Tue, Jul 30, 2013 at 11:02 PM, Xinliang David Li davi...@google.com wrote:
 I need to understand why this affects profile bootstrap -- is this due
 to file name conflict?

 The fix is wrong -- please do not remove the parameter. If it is a
 problem, a better fix is to change the default parameter value to 0.

 David


 On Tue, Jul 30, 2013 at 11:56 AM, Teresa Johnson tejohn...@google.com wrote:
 cc'ing Rong and David since this came from LIPO support.

 The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on
 by default) without removing the parameter itself. What is the failure
 mode you see from this code?

 Thanks,
 Teresa

 On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov
 di...@kugelworks.com wrote:
 Hello

 This change allows to complete profiledbootstrap on the google gcc-4.8
 branch, tested with make bootstrap with no new regressions.  OK for
 google 4.8?
thanks, Dinar.



 --
 Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413

Re: [Google] Fix profiledbootstrap failure

[Xinliang David Li]

Ok. Rong, can you help commit the parameter default setting patch?

thanks,

David

On Tue, Jul 30, 2013 at 12:48 PM, Rong Xu x...@google.com wrote:
 We have seen the issue before. It does fail the profile boostrap as it
 reads a wrong gcda file.
 I thought it had been fixed. (The fix was as David mentioned, setting
 the default value of the parameter to 0).

 -Rong

 On Tue, Jul 30, 2013 at 12:02 PM, Xinliang David Li davi...@google.com 
 wrote:
 I need to understand why this affects profile bootstrap -- is this due
 to file name conflict?

 The fix is wrong -- please do not remove the parameter. If it is a
 problem, a better fix is to change the default parameter value to 0.

 David


 On Tue, Jul 30, 2013 at 11:56 AM, Teresa Johnson tejohn...@google.com 
 wrote:
 cc'ing Rong and David since this came from LIPO support.

 The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on
 by default) without removing the parameter itself. What is the failure
 mode you see from this code?

 Thanks,
 Teresa

 On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov
 di...@kugelworks.com wrote:
 Hello

 This change allows to complete profiledbootstrap on the google gcc-4.8
 branch, tested with make bootstrap with no new regressions.  OK for
 google 4.8?
thanks, Dinar.



 --
 Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413

Re: [Google] Fix profiledbootstrap failure

[Rong Xu]

Will do.

The patch was in gcc-4_7 by Dehao.

r194713 | dehao | 2012-12-24 16:49:06 -0800 (Mon, 24 Dec 2012) | 5 lines

Fix the profiled bootstrap:

1. Set the default value of gcov-debug to be 0.
2. Merge profile summaries from different instrumented binaries.

On Tue, Jul 30, 2013 at 12:58 PM, Xinliang David Li davi...@google.com wrote:
 Ok. Rong, can you help commit the parameter default setting patch?

 thanks,

 David

 On Tue, Jul 30, 2013 at 12:48 PM, Rong Xu x...@google.com wrote:
 We have seen the issue before. It does fail the profile boostrap as it
 reads a wrong gcda file.
 I thought it had been fixed. (The fix was as David mentioned, setting
 the default value of the parameter to 0).

 -Rong

 On Tue, Jul 30, 2013 at 12:02 PM, Xinliang David Li davi...@google.com 
 wrote:
 I need to understand why this affects profile bootstrap -- is this due
 to file name conflict?

 The fix is wrong -- please do not remove the parameter. If it is a
 problem, a better fix is to change the default parameter value to 0.

 David


 On Tue, Jul 30, 2013 at 11:56 AM, Teresa Johnson tejohn...@google.com 
 wrote:
 cc'ing Rong and David since this came from LIPO support.

 The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on
 by default) without removing the parameter itself. What is the failure
 mode you see from this code?

 Thanks,
 Teresa

 On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov
 di...@kugelworks.com wrote:
 Hello

 This change allows to complete profiledbootstrap on the google gcc-4.8
 branch, tested with make bootstrap with no new regressions.  OK for
 google 4.8?
thanks, Dinar.



 --
 Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413

Re: [PATCH] Fix profiledbootstrap with -fexceptions (PR bootstrap/51648)

[Richard Guenther]

On Thu, Jan 5, 2012 at 1:13 AM, Jakub Jelinek ja...@redhat.com wrote:
 Hi!

 profiledbootstrap with --disable-poststage-build-with-cxx and -fexceptions
 currently fails on x86_64-linux.  The problem is that no EDGE_FAKE edge is
 added from noreturn fatal_error call to the exit block, eventhough
 fatal_error calls exit.
 The problem is that we have several places which add EDGE_FAKE edges.
 One is add_noreturn_fake_exit_edges, which adds EDGE_FAKE to bbs with zero
 successor (not this case, because after the call there is EDGE_EH (to a
 ={v} {CLOBBER} bb, which is why this is a regression)).
 And another one is gimple_flow_call_edges_add, but that one skips
 all ECF_NORETURN calls, based on the assumption (apparently wrong) that
 all noreturn calls have zero successor edges.
 And the third place that adds EDGE_FAKE edges is branch_prob
 itself, but it does so only for abnormal edges.

 I think this bug can be fixed in any of the 3 places, the following patch
 changes the second one.

 Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok (I suppose we could trigger this even w/o the CLOBBER stuff by
having a noreturn function that throws).

Thanks,
Richard.

 2012-01-04  Jakub Jelinek  ja...@redhat.com

        PR bootstrap/51648
        * tree-cfg.c (need_fake_edge_p): Return true also for noreturn
        calls that have any non-fake successor edges.

 --- gcc/tree-cfg.c.jj   2011-12-27 11:39:49.0 +0100
 +++ gcc/tree-cfg.c      2012-01-04 22:30:22.072838130 +0100
 @@ -6882,9 +6882,20 @@ need_fake_edge_p (gimple t)
            DECL_FUNCTION_CODE (fndecl) == BUILT_IN_FORK))
     return false;

 -  if (is_gimple_call (t)
 -       !(call_flags  ECF_NORETURN))
 -    return true;
 +  if (is_gimple_call (t))
 +    {
 +      edge_iterator ei;
 +      edge e;
 +      basic_block bb;
 +
 +      if (!(call_flags  ECF_NORETURN))
 +       return true;
 +
 +      bb = gimple_bb (t);
 +      FOR_EACH_EDGE (e, ei, bb-succs)
 +       if ((e-flags  EDGE_FAKE) == 0)
 +         return true;
 +    }

   if (gimple_code (t) == GIMPLE_ASM
         (gimple_asm_volatile_p (t) || gimple_asm_input_p (t)))
 @@ -6895,9 +6906,10 @@ need_fake_edge_p (gimple t)


  /* Add fake edges to the function exit for any non constant and non
 -   noreturn calls, volatile inline assembly in the bitmap of blocks
 -   specified by BLOCKS or to the whole CFG if BLOCKS is zero.  Return
 -   the number of blocks that were split.
 +   noreturn calls (or noreturn calls with EH/abnormal edges),
 +   volatile inline assembly in the bitmap of blocks specified by BLOCKS
 +   or to the whole CFG if BLOCKS is zero.  Return the number of blocks
 +   that were split.

    The goal is to expose cases in which entering a basic block does
    not imply that all subsequent instructions must be executed.  */

        Jakub

[PATCH] Fix profiledbootstrap with -fexceptions (PR bootstrap/51648)

[Jakub Jelinek]

Hi!

profiledbootstrap with --disable-poststage-build-with-cxx and -fexceptions
currently fails on x86_64-linux.  The problem is that no EDGE_FAKE edge is
added from noreturn fatal_error call to the exit block, eventhough
fatal_error calls exit.
The problem is that we have several places which add EDGE_FAKE edges.
One is add_noreturn_fake_exit_edges, which adds EDGE_FAKE to bbs with zero
successor (not this case, because after the call there is EDGE_EH (to a
={v} {CLOBBER} bb, which is why this is a regression)).
And another one is gimple_flow_call_edges_add, but that one skips
all ECF_NORETURN calls, based on the assumption (apparently wrong) that
all noreturn calls have zero successor edges.
And the third place that adds EDGE_FAKE edges is branch_prob
itself, but it does so only for abnormal edges.

I think this bug can be fixed in any of the 3 places, the following patch
changes the second one.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2012-01-04  Jakub Jelinek  ja...@redhat.com

PR bootstrap/51648
* tree-cfg.c (need_fake_edge_p): Return true also for noreturn
calls that have any non-fake successor edges.

--- gcc/tree-cfg.c.jj   2011-12-27 11:39:49.0 +0100
+++ gcc/tree-cfg.c  2012-01-04 22:30:22.072838130 +0100
@@ -6882,9 +6882,20 @@ need_fake_edge_p (gimple t)
DECL_FUNCTION_CODE (fndecl) == BUILT_IN_FORK))
 return false;
 
-  if (is_gimple_call (t)
-   !(call_flags  ECF_NORETURN))
-return true;
+  if (is_gimple_call (t))
+{
+  edge_iterator ei;
+  edge e;
+  basic_block bb;
+
+  if (!(call_flags  ECF_NORETURN))
+   return true;
+
+  bb = gimple_bb (t);
+  FOR_EACH_EDGE (e, ei, bb-succs)
+   if ((e-flags  EDGE_FAKE) == 0)
+ return true;
+}
 
   if (gimple_code (t) == GIMPLE_ASM
 (gimple_asm_volatile_p (t) || gimple_asm_input_p (t)))
@@ -6895,9 +6906,10 @@ need_fake_edge_p (gimple t)
 
 
 /* Add fake edges to the function exit for any non constant and non
-   noreturn calls, volatile inline assembly in the bitmap of blocks
-   specified by BLOCKS or to the whole CFG if BLOCKS is zero.  Return
-   the number of blocks that were split.
+   noreturn calls (or noreturn calls with EH/abnormal edges),
+   volatile inline assembly in the bitmap of blocks specified by BLOCKS
+   or to the whole CFG if BLOCKS is zero.  Return the number of blocks
+   that were split.
 
The goal is to expose cases in which entering a basic block does
not imply that all subsequent instructions must be executed.  */

Jakub

[PATCH] Fix profiledbootstrap failures

[Jakub Jelinek]

Hi!

My profiledbootstrap --enable-checking=release (and lots of other configury
options) failed this morning, I got warnings (promoted to errors due to
-Werror*) because our uninitialized warning code couldn't figure these out.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk?

2011-12-21  Jakub Jelinek  ja...@redhat.com

* tree-vect-patterns.c (vect_operation_fits_smaller_type): Initialize
*op0 and *op1 to NULL_TREE first to avoid warnings.
* calls.c (initialize_argument_information): Initialize base to avoid
warnings.

--- gcc/tree-vect-patterns.c.jj 2011-12-21 08:43:48.0 +0100
+++ gcc/tree-vect-patterns.c2011-12-21 09:47:50.908126257 +0100
@@ -900,6 +900,8 @@ vect_operation_fits_smaller_type (gimple
   loop_vec_info loop_info = STMT_VINFO_LOOP_VINFO (vinfo_for_stmt (stmt));
   struct loop *loop = LOOP_VINFO_LOOP (loop_info);
 
+  *op0 = NULL_TREE;
+  *op1 = NULL_TREE;
   *new_def_stmt = NULL;
 
   if (!is_gimple_assign (stmt))
--- gcc/calls.c.jj  2011-12-12 22:10:26.0 +0100
+++ gcc/calls.c 2011-12-21 09:03:44.721030991 +0100
@@ -1157,7 +1157,7 @@ initialize_argument_information (int num
 type, argpos  n_named_args))
{
  bool callee_copies;
- tree base;
+ tree base = NULL_TREE;
 
  callee_copies
= reference_callee_copied (args_so_far_pnt, TYPE_MODE (type),

Jakub

Re: [PATCH] Fix profiledbootstrap failures

[Richard Guenther]

On Wed, Dec 21, 2011 at 3:21 PM, Jakub Jelinek ja...@redhat.com wrote:
 Hi!

 My profiledbootstrap --enable-checking=release (and lots of other configury
 options) failed this morning, I got warnings (promoted to errors due to
 -Werror*) because our uninitialized warning code couldn't figure these out.

 Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
 ok for trunk?

Ok.

Richard.

 2011-12-21  Jakub Jelinek  ja...@redhat.com

        * tree-vect-patterns.c (vect_operation_fits_smaller_type): Initialize
        *op0 and *op1 to NULL_TREE first to avoid warnings.
        * calls.c (initialize_argument_information): Initialize base to avoid
        warnings.

 --- gcc/tree-vect-patterns.c.jj 2011-12-21 08:43:48.0 +0100
 +++ gcc/tree-vect-patterns.c    2011-12-21 09:47:50.908126257 +0100
 @@ -900,6 +900,8 @@ vect_operation_fits_smaller_type (gimple
   loop_vec_info loop_info = STMT_VINFO_LOOP_VINFO (vinfo_for_stmt (stmt));
   struct loop *loop = LOOP_VINFO_LOOP (loop_info);

 +  *op0 = NULL_TREE;
 +  *op1 = NULL_TREE;
   *new_def_stmt = NULL;

   if (!is_gimple_assign (stmt))
 --- gcc/calls.c.jj      2011-12-12 22:10:26.0 +0100
 +++ gcc/calls.c 2011-12-21 09:03:44.721030991 +0100
 @@ -1157,7 +1157,7 @@ initialize_argument_information (int num
                             type, argpos  n_named_args))
        {
          bool callee_copies;
 -         tree base;
 +         tree base = NULL_TREE;

          callee_copies
            = reference_callee_copied (args_so_far_pnt, TYPE_MODE (type),

        Jakub