Re: [SFN] Bootstrap broken

2017-12-20 Thread Jakub Jelinek 
On Wed, Dec 20, 2017 at 04:57:43AM -0200, Alexandre Oliva wrote:
> for  gcc/ChangeLog
> 
>   PR bootstrap/83396
>   * cfgexpand.c (label_rtx_for_bb): Revert SFN changes that
>   allowed debug stmts before labels.
>   (expand_gimple_basic_block): Likewise.
>   * gimple-iterator.c (gimple_find_edge_insert_loc): Likewise.
>   * gimple-iterator.h (gsi_after_labels): Likewise.
>   * tree-cfgcleanup (remove_forwarder_block): Likewise, but
>   rename reused variable, and simplify using gsi_move_before.
>   * tree-ssa-tail-merge.c (find_duplicate): Likewise.
>   * tree-cfg.c (make_edges, cleanup_dead_labels): Likewise.
>   (gimple_can_merge_blocks_p, verify_gimple_in_cfg): Likewise.
>   (gimple_verify_flow_info, gimple_block_label): Likewise.
>   (make_blocks): Move debug markers after adjacent labels.
>   * cfgrtl.c (skip_insns_after_block): Revert SFN changes that
>   allowed debug insns outside blocks.
>   * df-scan.c (df_insn_delete): Likewise.
>   * lra-constraints.c (update_ebb_live_info): Likewise.
>   * var-tracking.c (get_first_insn, vt_emit_notes): Likewise.
>   (vt_initialize, delete_vta_debug_insns): Likewise.
>   (reemit_marker_as_note): Drop BB parm.  Adjust callers.

Ok, thanks.

Jakub

Re: [SFN] Bootstrap broken

2017-12-19 Thread Alexandre Oliva 
On Dec 19, 2017, Jakub Jelinek  wrote:

> Has the patch you've posted passed bootstrap/regtest?

Yeah, here's the latest version, that survived O1, O2 and O3 bootstraps
with bootstrap-debug (-g0 for stage2), bootstrap-debug-lean
(-fcompare-debug for stage3) and bootstrap-debug-lib (-fcompare-debug
for target libs) on all of x86_64-, i686-, ppc64-, ppc64le-, and
aarch64-, and also survived a regular bootstrap on sparc64-linux-gnu (it
had a few -fcompare-debug failures that I'm yet to investigate, though).

Ok to install?


> I'll try to eyeball it in detail tomorrow (or if you have spare cycles,
> just try to verify it against revision before SFNs were introduced)
> that the affected spots are reverted to the old code unless needed for SFNs
> without markers in between bbs.

I reviewed the entire SFN patchset looking for such spots, and reverted
all the bits that seemed relevant; that's how I got to the patch below.
I went too far with the vfork-breaking reversal, but after putting that
back in, testing proceeded smoothly.

BTW, we don't want markers in between BBs either.



[SFN] debug markers before labels no more

Make sure that gimple and RTL IRs don't have debug markers before
labels.  When we build the CFG, we move labels before any markers
appearing before them.  Then, make sure we don't mistakenly
reintroduce them.

This reverts some of the complexity that had been brought about by the
initial SFN patches.

for  gcc/ChangeLog

PR bootstrap/83396
* cfgexpand.c (label_rtx_for_bb): Revert SFN changes that
allowed debug stmts before labels.
(expand_gimple_basic_block): Likewise.
* gimple-iterator.c (gimple_find_edge_insert_loc): Likewise.
* gimple-iterator.h (gsi_after_labels): Likewise.
* tree-cfgcleanup (remove_forwarder_block): Likewise, but
rename reused variable, and simplify using gsi_move_before.
* tree-ssa-tail-merge.c (find_duplicate): Likewise.
* tree-cfg.c (make_edges, cleanup_dead_labels): Likewise.
(gimple_can_merge_blocks_p, verify_gimple_in_cfg): Likewise.
(gimple_verify_flow_info, gimple_block_label): Likewise.
(make_blocks): Move debug markers after adjacent labels.
* cfgrtl.c (skip_insns_after_block): Revert SFN changes that
allowed debug insns outside blocks.
* df-scan.c (df_insn_delete): Likewise.
* lra-constraints.c (update_ebb_live_info): Likewise.
* var-tracking.c (get_first_insn, vt_emit_notes): Likewise.
(vt_initialize, delete_vta_debug_insns): Likewise.
(reemit_marker_as_note): Drop BB parm.  Adjust callers.
---
 gcc/cfgexpand.c   |   13 +-
 gcc/cfgrtl.c  |3 -
 gcc/df-scan.c |2 -
 gcc/gimple-iterator.c |8 +---
 gcc/gimple-iterator.h |   15 ++-
 gcc/lra-constraints.c |7 ---
 gcc/tree-cfg.c|  101 -
 gcc/tree-cfgcleanup.c |   49 +-
 gcc/tree-ssa-tail-merge.c |4 +-
 gcc/var-tracking.c|   65 +++--
 10 files changed, 101 insertions(+), 166 deletions(-)

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index d1616e192cb5..186012b0360f 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -2327,9 +2327,6 @@ label_rtx_for_bb (basic_block bb ATTRIBUTE_UNUSED)
 {
   glabel *lab_stmt;
 
-  if (is_gimple_debug (gsi_stmt (gsi)))
-   continue;
-
   lab_stmt = dyn_cast  (gsi_stmt (gsi));
   if (!lab_stmt)
break;
@@ -5503,16 +5500,14 @@ expand_gimple_basic_block (basic_block bb, bool 
disable_tail_calls)
}
 }
 
-  gsi = gsi_start_nondebug (stmts);
+  gsi = gsi_start (stmts);
   if (!gsi_end_p (gsi))
 {
   stmt = gsi_stmt (gsi);
   if (gimple_code (stmt) != GIMPLE_LABEL)
stmt = NULL;
 }
-  gsi = gsi_start (stmts);
 
-  gimple *label_stmt = stmt;
   rtx_code_label **elt = lab_rtx_for_bb->get (bb);
 
   if (stmt || elt)
@@ -5523,8 +5518,7 @@ expand_gimple_basic_block (basic_block bb, bool 
disable_tail_calls)
   if (stmt)
{
  expand_gimple_stmt (stmt);
- if (gsi_stmt (gsi) == stmt)
-   gsi_next ();
+ gsi_next ();
}
 
   if (elt)
@@ -5550,9 +5544,6 @@ expand_gimple_basic_block (basic_block bb, bool 
disable_tail_calls)
 
   stmt = gsi_stmt (gsi);
 
-  if (stmt == label_stmt)
-   continue;
-
   /* If this statement is a non-debug one, and we generate debug
 insns, then this one might be the last real use of a TERed
 SSA_NAME, but where there are still some debug uses further
diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index 45cccba680c9..e2da7608a91b 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -3390,9 +3390,6 @@ skip_insns_after_block (basic_block bb)
  last_insn = insn;
  continue;
 
-   case DEBUG_INSN:
- continue;
-
case NOTE:
  switch

Re: [SFN] Bootstrap broken

2017-12-19 Thread Jakub Jelinek 
On Tue, Dec 19, 2017 at 04:29:53PM -0200, Alexandre Oliva wrote:
> On Dec 15, 2017, Jakub Jelinek  wrote:
> 
> > We want to verify there are no statements before labels.
> 
> Erhm, actually...
> 
> We already verify that in gimple_verify_flow_info.
> 
> Is there any point in having such redundant (?) verification?

Ah, you're right.  The reason I've added was that you've added
  if (is_gimple_debug (gsi_stmt (gsi)))
continue;
there and so it didn't catch those.  But now you're reverting that
and so it should be effective again.

Has the patch you've posted passed bootstrap/regtest?
I'll try to eyeball it in detail tomorrow (or if you have spare cycles,
just try to verify it against revision before SFNs were introduced)
that the affected spots are reverted to the old code unless needed for SFNs
without markers in between bbs.

Jakub

Re: [SFN] Bootstrap broken

2017-12-19 Thread Alexandre Oliva 
On Dec 15, 2017, Jakub Jelinek  wrote:

> We want to verify there are no statements before labels.

Erhm, actually...

We already verify that in gimple_verify_flow_info.

Is there any point in having such redundant (?) verification?

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [SFN] Bootstrap broken

2017-12-19 Thread Alexandre Oliva 
On Dec 15, 2017, Jakub Jelinek  wrote:

> Please don't revert the above 2 hunks.  [...]  We want to verify there are
> no statements before labels.

Why, sure!  Thanks for catching this.

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [SFN] Bootstrap broken

2017-12-15 Thread Jakub Jelinek 
Hi!

I'll try to read it in more details later today, but one thing I've noticed:

On Thu, Dec 14, 2017 at 11:51:29PM -0200, Alexandre Oliva wrote:
> @@ -5380,7 +5410,6 @@ verify_gimple_in_cfg (struct function *fn, bool 
> verify_nothrow)
> err |= err2;
>   }
>  
> -  bool label_allowed = true;
>for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next ())
>   {
> gimple *stmt = gsi_stmt (gsi);
> @@ -5397,19 +5426,6 @@ verify_gimple_in_cfg (struct function *fn, bool 
> verify_nothrow)
> err2 = true;
>   }
>  
> -   /* Labels may be preceded only by debug markers, not debug bind
> -  or source bind or any other statements.  */
> -   if (gimple_code (stmt) == GIMPLE_LABEL)
> - {
> -   if (!label_allowed)
> - {
> -   error ("gimple label in the middle of a basic block");
> -   err2 = true;
> - }
> - }
> -   else if (!gimple_debug_begin_stmt_p (stmt))
> - label_allowed = false;
> -

Please don't revert the above 2 hunks.  Instead just remove the
> -   else if (!gimple_debug_begin_stmt_p (stmt))
> - label_allowed = false;
lines only from it and adjust the comment.  We want to verify there are
no statements before labels.

Jakub

Re: [SFN] Bootstrap broken

2017-12-14 Thread Alexandre Oliva 
On Dec 14, 2017, Alexandre Oliva  wrote:

> On Dec 14, 2017, Alexandre Oliva  wrote:
>> I'll arrange for markers to be moved past labels, even in gimple

> Here's a patch that implements this, and reverts all the changes I could
> find that had been introduced to support debug markers before labels and
> between BBs.

> I have *not* fully tested it yet

I reverted too much :-(  see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83396#c77 for details, or
just apply this patchlet on top of the earlier one (it reinstates some
of the SFN changes that turned out to be needed for more than accepting
markers before labels)


diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 4ec356d204d1..7fe0a1eec4c8 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -561,14 +561,22 @@ make_blocks_1 (gimple_seq seq, basic_block bb)
 {
   gimple_stmt_iterator i = gsi_start (seq);
   gimple *stmt = NULL;
+  gimple *prev_stmt = NULL;
   bool start_new_block = true;
   bool first_stmt_of_seq = true;
 
   while (!gsi_end_p (i))
 {
-  gimple *prev_stmt;
-
-  prev_stmt = stmt;
+  /* PREV_STMT should only be set to a debug stmt if the debug
+stmt is before nondebug stmts.  Once stmt reaches a nondebug
+nonlabel, prev_stmt will be set to it, so that
+stmt_starts_bb_p will know to start a new block if a label is
+found.  However, if stmt was a label after debug stmts only,
+keep the label in prev_stmt even if we find further debug
+stmts, for there may be other labels after them, and they
+should land in the same block.  */
+  if (!prev_stmt || !stmt || !is_gimple_debug (stmt))
+   prev_stmt = stmt;
   stmt = gsi_stmt (i);
 
   if (stmt && is_gimple_call (stmt))
@@ -583,6 +591,7 @@ make_blocks_1 (gimple_seq seq, basic_block bb)
gsi_split_seq_before (, );
  bb = create_basic_block (seq, bb);
  start_new_block = false;
+ prev_stmt = NULL;
}
 
   /* Now add STMT to BB and create the subgraphs for special statement
@@ -2703,6 +2712,13 @@ stmt_starts_bb_p (gimple *stmt, gimple *prev_stmt)
   if (stmt == NULL)
 return false;
 
+  /* PREV_STMT is only set to a debug stmt if the debug stmt is before
+ any nondebug stmts in the block.  We don't want to start another
+ block in this case: the debug stmt will already have started the
+ one STMT would start if we weren't outputting debug stmts.  */
+  if (prev_stmt && is_gimple_debug (prev_stmt))
+return false;
+
   /* Labels start a new basic block only if the preceding statement
  wasn't a label of the same type.  This prevents the creation of
  consecutive blocks that have nothing but a single label.  */

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [SFN] Bootstrap broken

2017-12-14 Thread Alexandre Oliva 
On Dec 14, 2017, Alexandre Oliva  wrote:

> I'll arrange for markers to be moved past labels, even in gimple

Here's a patch that implements this, and reverts all the changes I could
find that had been introduced to support debug markers before labels and
between BBs.

I have *not* fully tested it yet, nor do I intend to install it shortly;
at the earliest (assuming it's reviewed and approved) Monday or Tuesday
next week.

This will give me time to give it more varied testing (multiple
optimization flags, for one; more targets, if others volunteer testing,
or I can locate available machines in the GCC Compile Farm or elsewhere)

Assuming testing succeeds, is this ok to install?


[SFN] debug markers before labels no more

Make sure that gimple and RTL IRs don't have debug markers before
labels.  When we build the CFG, we move labels before any markers
appearing before them.  Then, make sure we don't mistakenly
reintroduce them.

This reverts some of the complexity that had been brought about by the
initial SFN patches.

for  gcc/ChangeLog

PR bootstrap/83396
* cfgexpand.c (label_rtx_for_bb): Revert SFN changes that
allowed debug stmts before labels.
(expand_gimple_basic_block): Likewise.
* gimple-iterator.c (gimple_find_edge_insert_loc): Likewise.
* gimple-iterator.h (gsi_after_labels): Likewise.
* tree-cfgcleanup (remove_forwarder_block): Likewise, but
rename reused variable, and simplify using gsi_move_before.
* tree-ssa-tail-merge.c (find_duplicate): Likewise.
* tree-cfg.c (make_blocks_1, make_edges): Likewise.
(cleanup_dead_labels, gimple_can_merge_blocks_p): Likewise.
(stmt_starts_bb_p, verify_gimple_in_cfg): Likewise.
(gimple_verify_flow_info, gimple_block_label): Likewise.
(make_blocks): Move debug markers after adjacent labels.
* cfgrtl.c (skip_insns_after_block): Revert SFN changes that
allowed debug insns outside blocks.
* df-scan.c (df_insn_delete): Likewise.
* lra-constraints.c (update_ebb_live_info): Likewise.
* var-tracking.c (get_first_insn, vt_emit_notes): Likewise.
(vt_initialize, delete_vta_debug_insns): Likewise.
(reemit_marker_as_note): Drop BB parm.  Adjust callers.
---
 gcc/cfgexpand.c   |   13 +
 gcc/cfgrtl.c  |3 -
 gcc/df-scan.c |2 -
 gcc/gimple-iterator.c |8 +--
 gcc/gimple-iterator.h |   15 +
 gcc/lra-constraints.c |7 ---
 gcc/tree-cfg.c|  123 -
 gcc/tree-cfgcleanup.c |   49 --
 gcc/tree-ssa-tail-merge.c |4 +
 gcc/var-tracking.c|   65 ++--
 10 files changed, 104 insertions(+), 185 deletions(-)

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 30d9bac1118e..9c97f6e55691 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -2327,9 +2327,6 @@ label_rtx_for_bb (basic_block bb ATTRIBUTE_UNUSED)
 {
   glabel *lab_stmt;
 
-  if (is_gimple_debug (gsi_stmt (gsi)))
-   continue;
-
   lab_stmt = dyn_cast  (gsi_stmt (gsi));
   if (!lab_stmt)
break;
@@ -5498,16 +5495,14 @@ expand_gimple_basic_block (basic_block bb, bool 
disable_tail_calls)
}
 }
 
-  gsi = gsi_start_nondebug (stmts);
+  gsi = gsi_start (stmts);
   if (!gsi_end_p (gsi))
 {
   stmt = gsi_stmt (gsi);
   if (gimple_code (stmt) != GIMPLE_LABEL)
stmt = NULL;
 }
-  gsi = gsi_start (stmts);
 
-  gimple *label_stmt = stmt;
   rtx_code_label **elt = lab_rtx_for_bb->get (bb);
 
   if (stmt || elt)
@@ -5518,8 +5513,7 @@ expand_gimple_basic_block (basic_block bb, bool 
disable_tail_calls)
   if (stmt)
{
  expand_gimple_stmt (stmt);
- if (gsi_stmt (gsi) == stmt)
-   gsi_next ();
+ gsi_next ();
}
 
   if (elt)
@@ -5545,9 +5539,6 @@ expand_gimple_basic_block (basic_block bb, bool 
disable_tail_calls)
 
   stmt = gsi_stmt (gsi);
 
-  if (stmt == label_stmt)
-   continue;
-
   /* If this statement is a non-debug one, and we generate debug
 insns, then this one might be the last real use of a TERed
 SSA_NAME, but where there are still some debug uses further
diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index bc1e3ee7ece8..e4cdab533a49 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -3390,9 +3390,6 @@ skip_insns_after_block (basic_block bb)
  last_insn = insn;
  continue;
 
-   case DEBUG_INSN:
- continue;
-
case NOTE:
  switch (NOTE_KIND (insn))
{
diff --git a/gcc/df-scan.c b/gcc/df-scan.c
index 429dab8a99b4..8ab3d716ea29 100644
--- a/gcc/df-scan.c
+++ b/gcc/df-scan.c
@@ -945,7 +945,7 @@ df_insn_delete (rtx_insn *insn)
  In any case, we expect BB to be non-NULL at least up to register
  allocation, so disallow a non-NULL BB up to there.  Not perfect
  but better than

Re: [SFN] Bootstrap broken

2017-12-14 Thread Jakub Jelinek 
On Thu, Dec 14, 2017 at 04:08:45PM -0200, Alexandre Oliva wrote:
> On Dec 13, 2017, Alexandre Oliva  wrote:
> 
> > On Dec 12, 2017, David Edelsohn  wrote:
> >> Rainer,
> >> PR83396 opened.  you can add Solaris to the list of targets.
> 
> > Andreas,
> 
> > Here's a fix for the ia64 regression you mentioned in that PR.
> 
> And here's a patch that fixes the two other ia64 build failures you'd
> mentioned.
> 
> Regstrapped on x86_64- and i686-linux-gnu by myself; bootstrapped on
> ia64-linux-gnu by yourself IIUC, though with some weird bootstrap
> compare errors I'm very curious to learn more about.
> 
> Ok to install?
> 
> Emitting markers before labels turned out to not be worth the trouble.
> The markers outside BBs confuse the ebb scheduler, and they don't add
> any useful information.  I'll arrange for markers to be moved past
> labels, even in gimple, but for now this will fix the two remaining
> known problems on ia64.
> 
> for  gcc/ChangeLog
> 
>   PR bootstrap/83396
>   * cfgexpand.c (expand_gimple_basic_block): Expand label first,
>   even if there are markers before it.
>   * cfgrtl.c (rtl_verify_bb_layout): Reject DEBUG_INSNs outside BBs.

Ok, but please work on reversion of everything that has been changed
in order to support those (on RTL e.g. the var-tracking.c
  for (insn = get_first_insn (bb);
   insn != BB_HEAD (bb->next_bb)
 ? next = NEXT_INSN (insn), true : false;
   insn = next)
and
  rtx_insn *next;
  bool outside_bb = true;
  for (insn = get_first_insn (bb); insn != BB_HEAD (bb->next_bb);
   insn = next)
{
  if (insn == BB_HEAD (bb))
outside_bb = false;
  else if (insn == NEXT_INSN (BB_END (bb)))
outside_bb = true;
...
  if (outside_bb)
{
  /* Ignore non-debug insns outside of basic blocks.  */
  if (!DEBUG_INSN_P (insn))
continue;
  /* Debug binds shouldn't appear outside of bbs.  */
  gcc_assert (!DEBUG_BIND_INSN_P (insn));
}
and the NULL BLOCK_FOR_INSN stuff, on GIMPLE the gsi_after_labels changes,
the tree-cfg.c verification needs to be changed to disallow even the
markers before labels, ...).  That can be done incrementally, it is better
to unbreak the tree ASAP.

Jakub

Re: [SFN] Bootstrap broken

2017-12-14 Thread Alexandre Oliva 
On Dec 13, 2017, Alexandre Oliva  wrote:

> On Dec 12, 2017, David Edelsohn  wrote:
>> Rainer,
>> PR83396 opened.  you can add Solaris to the list of targets.

> Andreas,

> Here's a fix for the ia64 regression you mentioned in that PR.

And here's a patch that fixes the two other ia64 build failures you'd
mentioned.

Regstrapped on x86_64- and i686-linux-gnu by myself; bootstrapped on
ia64-linux-gnu by yourself IIUC, though with some weird bootstrap
compare errors I'm very curious to learn more about.

Ok to install?

Emitting markers before labels turned out to not be worth the trouble.
The markers outside BBs confuse the ebb scheduler, and they don't add
any useful information.  I'll arrange for markers to be moved past
labels, even in gimple, but for now this will fix the two remaining
known problems on ia64.

for  gcc/ChangeLog

PR bootstrap/83396
* cfgexpand.c (expand_gimple_basic_block): Expand label first,
even if there are markers before it.
* cfgrtl.c (rtl_verify_bb_layout): Reject DEBUG_INSNs outside BBs.
---
 gcc/cfgexpand.c |   12 
 gcc/cfgrtl.c|1 -
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index ce98264214ae..30d9bac1118e 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -5510,20 +5510,16 @@ expand_gimple_basic_block (basic_block bb, bool 
disable_tail_calls)
   gimple *label_stmt = stmt;
   rtx_code_label **elt = lab_rtx_for_bb->get (bb);
 
-  if (stmt)
-/* We'll get to it in the loop below, and get back to
-   emit_label_and_note then.  */
-;
-  else if (stmt || elt)
+  if (stmt || elt)
 {
-emit_label_and_note:
   gcc_checking_assert (!note);
   last = get_last_insn ();
 
   if (stmt)
{
  expand_gimple_stmt (stmt);
- gsi_next ();
+ if (gsi_stmt (gsi) == stmt)
+   gsi_next ();
}
 
   if (elt)
@@ -5550,7 +5546,7 @@ expand_gimple_basic_block (basic_block bb, bool 
disable_tail_calls)
   stmt = gsi_stmt (gsi);
 
   if (stmt == label_stmt)
-   goto emit_label_and_note;
+   continue;
 
   /* If this statement is a non-debug one, and we generate debug
 insns, then this one might be the last real use of a TERed
diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index b127ea1a0b38..bc1e3ee7ece8 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -2954,7 +2954,6 @@ rtl_verify_bb_layout (void)
{
case BARRIER:
case NOTE:
-   case DEBUG_INSN:
  break;
 
case CODE_LABEL:


-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [SFN] Bootstrap broken

2017-12-14 Thread Jakub Jelinek 
On Thu, Dec 14, 2017 at 03:41:24PM +0100, Andreas Schwab wrote:
> This fixes the m68k ICE.
> 
> Andreas.
> 
>   PR bootstrap/83396
>   * reload1.c (emit_input_reload_insns): Skip debug markers.

This is ok for trunk, thanks.

> --- a/gcc/reload1.c
> +++ b/gcc/reload1.c
> @@ -7345,12 +7345,12 @@ emit_input_reload_insns (struct insn_chain *chain, 
> struct reload *rl,
>  
> /* Adjust any debug insns between temp and insn.  */
> while ((temp = NEXT_INSN (temp)) != insn)
> - if (DEBUG_INSN_P (temp))
> + if (DEBUG_BIND_INSN_P (temp))
> INSN_VAR_LOCATION_LOC (temp)
>   = simplify_replace_rtx (INSN_VAR_LOCATION_LOC (temp),
>   old, reloadreg);
>   else
> -   gcc_assert (NOTE_P (temp));
> +   gcc_assert (DEBUG_INSN_P (temp) || NOTE_P (temp));
>   }
> else
>   {

Jakub

Re: [SFN] Bootstrap broken

2017-12-14 Thread Andreas Schwab 
This fixes the m68k ICE.

Andreas.

PR bootstrap/83396
* reload1.c (emit_input_reload_insns): Skip debug markers.

---
 gcc/reload1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/reload1.c b/gcc/reload1.c
index fe1ec0d011..baedc43b75 100644
--- a/gcc/reload1.c
+++ b/gcc/reload1.c
@@ -7345,12 +7345,12 @@ emit_input_reload_insns (struct insn_chain *chain, 
struct reload *rl,
 
  /* Adjust any debug insns between temp and insn.  */
  while ((temp = NEXT_INSN (temp)) != insn)
-   if (DEBUG_INSN_P (temp))
+   if (DEBUG_BIND_INSN_P (temp))
  INSN_VAR_LOCATION_LOC (temp)
= simplify_replace_rtx (INSN_VAR_LOCATION_LOC (temp),
old, reloadreg);
else
- gcc_assert (NOTE_P (temp));
+ gcc_assert (DEBUG_INSN_P (temp) || NOTE_P (temp));
}
  else
{
-- 
2.15.1

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [SFN] Bootstrap broken

2017-12-14 Thread Alexandre Oliva 
On Dec 13, 2017, Jakub Jelinek  wrote:

> In particular, this testcase is using selective scheduling, therefore
> we turn off -fvar-tracking-assignments, but the debug stmt markers are
> emitted anyway.

*nod*, that much was intended (though I could be convinced to change it ;-)

> -fvar-tracking is still true, so the var-tracking pass does everything it
> normally does (successfully), then the free_cfg pass removes all
> BLOCK_FOR_INSN notes, then some targets in their machine reorg recompute
> those, but sparc apparently doesn't, and finally in final.c:

>   /* Turn debug markers into notes.  */
>   if (!MAY_HAVE_DEBUG_BIND_INSNS && MAY_HAVE_DEBUG_MARKER_INSNS)
> variable_tracking_main ();

> Eeek, this runs all of the var tracking again,

Eeek, indeed ;-)  /me takes a mental note that flag_var_tracking !=
> MAY_HAVE_DEBUG_BIND_INSNS, and underlines it several times ;-D

Thanks for spotting the deeper problem and for fixing it!

I'm glad the patch I posted to fix the shallower one still serves as a
basis for the ongoing attempts to fix one of the remaining ia64 issues.
I'm on it.

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [SFN] Bootstrap broken

2017-12-14 Thread Alexandre Oliva 
Jakub, I summed up to you yesterday on IRC what I expand below; this is
just for the public record.

On Dec 13, 2017, Jakub Jelinek  wrote:

> Furthermore, I must say I don't understand why
> can_move_early_debug_stmts should care whether there are any labels in
> dest bb or not.

An earlier attempt tried to move all labels and debug stmts in a single
loop, and started with this early debug setting and inserting at the
block start stmt, and later switched to the preexisting debug setting
and inserting at the after-label stmt.  In the end, I decided this was
trickier and riskier than two separate loops, but failed to simplify the
logic back all the way.

> Though, if
> gsi_stmt (gsi) == gsi_stmt (gsie), then the loop right below it will not
> do anything and nothing cares about can_move_early_debug_stmts afterwards.
> So, in short, can_move_early_debug_stmts is used only if
> gsi_stmt (gsi) != gsi_stmt (gsie), and therefore
> can_move_early_debug_stmts if it is used is can_move_debug_stmts && (1 || 
> ...);

*nod*

> So, can we get rid of can_move_early_debug_stmts altogether and just use
> can_move_debug_stmts in there instead?

Yes.

> Another thing I find risky is that you compute gsie_to so early and don't
> update it.

The point of computing it early was *precisely* to preserve the
insertion point should we have both before-label and after-label debug
stmts to move.  But in the end it doesn't matter: we'll either find the
right spot after moving labels, or we'll be at it if there weren't any
labels to move.


Thanks for the improvements!

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [SFN] Bootstrap broken

2017-12-13 Thread Rainer Orth 
Hi Jakub,

>> On Wed, Dec 13, 2017 at 02:31:07PM +0100, Rainer Orth wrote:
>>> Hi Jakub,
>>> 
>>> > Here it is everything in patch form, in case some volunteers are willing 
>>> > to
>>> > test it on their targets, because we need faster turn-arounds for this.
>>> 
>>> thanks for that: it's easy to loose track in this maze ;-)
>>
>> True.  What I'm regtesting (bootstraps already done) on
>> {x86_64,i686,powerpc64{,le}}-linux now is:
>> https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00811.html
>> https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00808.html
>> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42861
>> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42866
>> https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00794.html
>> set.  Does pr69102.c FAIL with that set?
>
> thanks for the list.  A sparc-sun-solaris2.11 bootstrap with the whole
> set is now running; expect results in about two hours.

completed now and the testsuite regression is gone, too.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [SFN] Bootstrap broken

2017-12-13 Thread Christophe Lyon 
On 13 December 2017 at 11:45, Jakub Jelinek  wrote:
> On Wed, Dec 13, 2017 at 10:28:22AM +0100, Jakub Jelinek wrote:
>> Formatting, this should be
>>   bool can_move_early_debug_stmts
>> = ...
>> and the line is too long, so needs to be wrapped.
>>
>> Furthermore, I must say I don't understand why
>> can_move_early_debug_stmts should care whether there are any labels in
>> dest bb or not.  That sounds very risky for introducing non-# DEBUG 
>> BEGIN_STMT
>> debug insns before labels if it could happen.  Though, if
>> gsi_stmt (gsi) == gsi_stmt (gsie), then the loop right below it will not
>> do anything and nothing cares about can_move_early_debug_stmts afterwards.
>> So, in short, can_move_early_debug_stmts is used only if
>> gsi_stmt (gsi) != gsi_stmt (gsie), and therefore
>> can_move_early_debug_stmts if it is used is can_move_debug_stmts && (1 || 
>> ...);
>>
>> So, can we get rid of can_move_early_debug_stmts altogether and just use
>> can_move_debug_stmts in there instead?
>>
>> Another thing I find risky is that you compute gsie_to so early and don't
>> update it.  If you don't need it above for can_move_early_debug_stmts, can
>> you just do it back where it used to be done,
>
> Here it is everything in patch form, in case some volunteers are willing to
> test it on their targets, because we need faster turn-arounds for this.
>

Thanks for that, it certainly helps.

So, this version does restore a successful build on arm --with-mode=thumb,
but pr69102 still fails in arm mode.

As I mentioned in PR83396, the 4 patches attached there fix all the
problems I noticed.

HTH.

Christophe

> 2017-12-13  Alexandre Oliva 
> Jakub Jelinek  
>
> PR bootstrap/83396
> PR debug/83391
> * tree-cfgcleanup.c (remove_forwarder_block): Keep after
> labels debug stmts that can only appear after labels.
>
> * gcc.dg/torture/pr83396.c: New test.
> * g++.dg/torture/pr83391.C: New test.
>
> --- gcc/tree-cfgcleanup.c.jj2017-12-12 09:48:26.813393301 +0100
> +++ gcc/tree-cfgcleanup.c   2017-12-13 11:39:03.373065381 +0100
> @@ -536,9 +536,14 @@ remove_forwarder_block (basic_block bb)
>   defined labels and labels with an EH landing pad number to the
>   new block, so that the redirection of the abnormal edges works,
>   jump targets end up in a sane place and debug information for
> - labels is retained.  */
> + labels is retained.
> +
> + While at that, move any debug stmts that appear before or in between
> + labels, but not those that can only appear after labels.  */
>gsi_to = gsi_start_bb (dest);
> -  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
> +  gsi = gsi_start_bb (bb);
> +  gimple_stmt_iterator gsie = gsi_after_labels (bb);
> +  while (gsi_stmt (gsi) != gsi_stmt (gsie))
>  {
>tree decl;
>label = gsi_stmt (gsi);
> @@ -557,6 +562,21 @@ remove_forwarder_block (basic_block bb)
> gsi_next ();
>  }
>
> +  /* Move debug statements if the destination has a single predecessor.  */
> +  if (can_move_debug_stmts && !gsi_end_p (gsi))
> +{
> +  gcc_assert (gsi_stmt (gsi) == gsi_stmt (gsie));
> +  gimple_stmt_iterator gsie_to = gsi_after_labels (dest);
> +  do
> +   {
> + gimple *debug = gsi_stmt (gsi);
> + gcc_assert (is_gimple_debug (debug));
> + gsi_remove (, false);
> + gsi_insert_before (_to, debug, GSI_SAME_STMT);
> +   }
> +  while (!gsi_end_p (gsi));
> +}
> +
>bitmap_set_bit (cfgcleanup_altered_bbs, dest->index);
>
>/* Update the dominators.  */
> --- gcc/testsuite/g++.dg/torture/pr83391.C.jj
> +++ gcc/testsuite/g++.dg/torture/pr83391.C
> @@ -0,0 +1,36 @@
> +// PR debug/83391
> +// { dg-do compile }
> +// { dg-options "-g" }
> +// { dg-additional-options "-mbranch-cost=1" { target { i?86-*-* x86_64-*-* 
> mips*-*-* s390*-*-* avr*-*-* } } }
> +
> +unsigned char a;
> +enum E { F, G, H } b;
> +int c, d;
> +
> +void
> +foo ()
> +{
> +  int e;
> +  bool f;
> +  E g = b;
> +  while (1)
> +{
> +  unsigned char h = a ? d : 0;
> +  switch (g)
> +   {
> +   case 0:
> + f = h <= 'Z' || h >= 'a' && h <= 'z';
> + break;
> +   case 1:
> + {
> +   unsigned char i = h;
> +   e = 0;
> + }
> + if (e || h)
> +   g = H;
> + /* FALLTHRU */
> +   default:
> + c = 0;
> +   }
> +}
> +}
> --- gcc/testsuite/gcc.dg/torture/pr83396.c.jj
> +++ gcc/testsuite/gcc.dg/torture/pr83396.c
> @@ -0,0 +1,38 @@
> +/* PR bootstrap/83396 */
> +/* { dg-do compile } */
> +/* { dg-options "-g" } */
> +
> +int fn1 (void);
> +void fn2 (void *, const char *);
> +void fn3 (void);
> +
> +void
> +fn4 (long long x)
> +{
> +  fn3 ();
> +}
> +
> +void
> +fn5 (long long x)
> +{
> +  if (x)
> +fn3();
> +}
> +
> +void
> +fn6 (long long x)
> +{
> +  switch (fn1 ())
> +{
> +case 0:
> +

Re: [SFN] Bootstrap broken

2017-12-13 Thread Richard Biener 
On Wed, 13 Dec 2017, Jakub Jelinek wrote:

> On Wed, Dec 13, 2017 at 11:45:51AM +0100, Jakub Jelinek wrote:
> > On Wed, Dec 13, 2017 at 10:28:22AM +0100, Jakub Jelinek wrote:
> > > Formatting, this should be
> > >   bool can_move_early_debug_stmts
> > > = ...
> > > and the line is too long, so needs to be wrapped.
> > > 
> > > Furthermore, I must say I don't understand why
> > > can_move_early_debug_stmts should care whether there are any labels in
> > > dest bb or not.  That sounds very risky for introducing non-# DEBUG 
> > > BEGIN_STMT
> > > debug insns before labels if it could happen.  Though, if
> > > gsi_stmt (gsi) == gsi_stmt (gsie), then the loop right below it will not
> > > do anything and nothing cares about can_move_early_debug_stmts afterwards.
> > > So, in short, can_move_early_debug_stmts is used only if
> > > gsi_stmt (gsi) != gsi_stmt (gsie), and therefore
> > > can_move_early_debug_stmts if it is used is can_move_debug_stmts && (1 || 
> > > ...);
> > > 
> > > So, can we get rid of can_move_early_debug_stmts altogether and just use
> > > can_move_debug_stmts in there instead?
> > > 
> > > Another thing I find risky is that you compute gsie_to so early and don't
> > > update it.  If you don't need it above for can_move_early_debug_stmts, can
> > > you just do it back where it used to be done,
> > 
> > Here it is everything in patch form, in case some volunteers are willing to
> > test it on their targets, because we need faster turn-arounds for this.
> > 
> > 2017-12-13  Alexandre Oliva 
> > Jakub Jelinek  
> > 
> > PR bootstrap/83396
> > PR debug/83391
> > * tree-cfgcleanup.c (remove_forwarder_block): Keep after
> > labels debug stmts that can only appear after labels.
> > 
> > * gcc.dg/torture/pr83396.c: New test.
> > * g++.dg/torture/pr83391.C: New test.
> 
> Bootstrapped/regtested on x86_64-linux, i686-linux and powerpc64le-linux,
> powerpc64-linux regtest still pending, ok for trunk?

Ok.

Richard.

> > --- gcc/tree-cfgcleanup.c.jj2017-12-12 09:48:26.813393301 +0100
> > +++ gcc/tree-cfgcleanup.c   2017-12-13 11:39:03.373065381 +0100
> > @@ -536,9 +536,14 @@ remove_forwarder_block (basic_block bb)
> >   defined labels and labels with an EH landing pad number to the
> >   new block, so that the redirection of the abnormal edges works,
> >   jump targets end up in a sane place and debug information for
> > - labels is retained.  */
> > + labels is retained.
> > +
> > + While at that, move any debug stmts that appear before or in between
> > + labels, but not those that can only appear after labels.  */
> >gsi_to = gsi_start_bb (dest);
> > -  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
> > +  gsi = gsi_start_bb (bb);
> > +  gimple_stmt_iterator gsie = gsi_after_labels (bb);
> > +  while (gsi_stmt (gsi) != gsi_stmt (gsie))
> >  {
> >tree decl;
> >label = gsi_stmt (gsi);
> > @@ -557,6 +562,21 @@ remove_forwarder_block (basic_block bb)
> > gsi_next ();
> >  }
> >  
> > +  /* Move debug statements if the destination has a single predecessor.  */
> > +  if (can_move_debug_stmts && !gsi_end_p (gsi))
> > +{
> > +  gcc_assert (gsi_stmt (gsi) == gsi_stmt (gsie));
> > +  gimple_stmt_iterator gsie_to = gsi_after_labels (dest);
> > +  do
> > +   {
> > + gimple *debug = gsi_stmt (gsi);
> > + gcc_assert (is_gimple_debug (debug));
> > + gsi_remove (, false);
> > + gsi_insert_before (_to, debug, GSI_SAME_STMT);
> > +   }
> > +  while (!gsi_end_p (gsi));
> > +}
> > +
> >bitmap_set_bit (cfgcleanup_altered_bbs, dest->index);
> >  
> >/* Update the dominators.  */
> > --- gcc/testsuite/g++.dg/torture/pr83391.C.jj
> > +++ gcc/testsuite/g++.dg/torture/pr83391.C
> > @@ -0,0 +1,36 @@
> > +// PR debug/83391
> > +// { dg-do compile }
> > +// { dg-options "-g" }
> > +// { dg-additional-options "-mbranch-cost=1" { target { i?86-*-* 
> > x86_64-*-* mips*-*-* s390*-*-* avr*-*-* } } }
> > +
> > +unsigned char a;
> > +enum E { F, G, H } b;
> > +int c, d;
> > +
> > +void
> > +foo ()
> > +{
> > +  int e;
> > +  bool f;
> > +  E g = b;
> > +  while (1)
> > +{
> > +  unsigned char h = a ? d : 0;
> > +  switch (g)
> > +   {
> > +   case 0:
> > + f = h <= 'Z' || h >= 'a' && h <= 'z';
> > + break;
> > +   case 1:
> > + {
> > +   unsigned char i = h;
> > +   e = 0;
> > + }
> > + if (e || h)
> > +   g = H;
> > + /* FALLTHRU */
> > +   default:
> > + c = 0;
> > +   }
> > +}
> > +}
> > --- gcc/testsuite/gcc.dg/torture/pr83396.c.jj
> > +++ gcc/testsuite/gcc.dg/torture/pr83396.c
> > @@ -0,0 +1,38 @@
> > +/* PR bootstrap/83396 */
> > +/* { dg-do compile } */
> > +/* { dg-options "-g" } */
> > +
> > +int fn1 (void);
> > +void fn2 (void *, const char *);
> > +void fn3 (void);
> > +
> > +void
> > +fn4 (long long x)
> > +{
> > +  fn3 ();
> > +}
> > +
> > +void
> > +fn5 (long long x)
> > +{

Re: [SFN] Bootstrap broken

2017-12-13 Thread Richard Biener 
On Wed, 13 Dec 2017, Jakub Jelinek wrote:

> On Wed, Dec 13, 2017 at 11:34:04AM +0100, Jakub Jelinek wrote:
> > 2017-12-13  Jakub Jelinek  
> > 
> > PR bootstrap/83396
> > * final.c (rest_of_handle_final): Call variable_tracking_main only
> > if !flag_var_tracking.
> 
> Bootstrapped/regtested on x86_64-linux, i686-linux and powerpc64le-linux,
> powerpc64-linux regtest still pending, ok for trunk?

Ok.

Richard.

> > --- gcc/final.c.jj  2017-12-12 09:48:15.0 +0100
> > +++ gcc/final.c 2017-12-13 11:29:12.284676265 +0100
> > @@ -4541,8 +4541,9 @@ rest_of_handle_final (void)
> >  {
> >const char *fnname = get_fnname_from_decl (current_function_decl);
> >  
> > -  /* Turn debug markers into notes.  */
> > -  if (!MAY_HAVE_DEBUG_BIND_INSNS && MAY_HAVE_DEBUG_MARKER_INSNS)
> > +  /* Turn debug markers into notes if the var-tracking pass has not
> > + been invoked.  */
> > +  if (!flag_var_tracking && MAY_HAVE_DEBUG_MARKER_INSNS)
> >  variable_tracking_main ();
> >  
> >assemble_start_function (current_function_decl, fnname);
> > 
> 
>   Jakub
> 
> 

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

Re: [SFN] Bootstrap broken

2017-12-13 Thread Rainer Orth 
Hi Jakub,

> On Wed, Dec 13, 2017 at 02:31:07PM +0100, Rainer Orth wrote:
>> Hi Jakub,
>> 
>> > Here it is everything in patch form, in case some volunteers are willing to
>> > test it on their targets, because we need faster turn-arounds for this.
>> 
>> thanks for that: it's easy to loose track in this maze ;-)
>
> True.  What I'm regtesting (bootstraps already done) on
> {x86_64,i686,powerpc64{,le}}-linux now is:
> https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00811.html
> https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00808.html
> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42861
> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42866
> https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00794.html
> set.  Does pr69102.c FAIL with that set?

thanks for the list.  A sparc-sun-solaris2.11 bootstrap with the whole
set is now running; expect results in about two hours.

>> I've just bootstrapped sparc-sun-solaris2.11 with your patch and this one:
>> 
>>  https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00794.html
>> 
>> The bootstrap succeeds, but the gcc.c-torture/compile/pr69102.c
>> regression persists.  Besides, I see
>> 
>> +FAIL: libgomp.graphite/force-parallel-4.c scan-tree-dump-times graphite
>> "2 loops carried no dependency" 1 (found 0 times)
>> +FAIL: libgomp.graphite/force-parallel-4.c scan-tree-dump-times optimized
>> "loopfn.1" 4 (found 0 times)
>> +FAIL: libgomp.graphite/force-parallel-8.c scan-tree-dump-times graphite
>> "5 loops carried no dependency" 1 (found 0 times)
>> 
>> which is most likely unrelated (I upgraded the tree from r255584 to
>> r255603).
>
> Yeah, these are almost certainly unrelated.

It certainly is: I've filed PR tree-optimization/83410.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [SFN] Bootstrap broken

2017-12-13 Thread Jakub Jelinek 
On Wed, Dec 13, 2017 at 11:34:04AM +0100, Jakub Jelinek wrote:
> 2017-12-13  Jakub Jelinek  
> 
>   PR bootstrap/83396
>   * final.c (rest_of_handle_final): Call variable_tracking_main only
>   if !flag_var_tracking.

Bootstrapped/regtested on x86_64-linux, i686-linux and powerpc64le-linux,
powerpc64-linux regtest still pending, ok for trunk?

> --- gcc/final.c.jj2017-12-12 09:48:15.0 +0100
> +++ gcc/final.c   2017-12-13 11:29:12.284676265 +0100
> @@ -4541,8 +4541,9 @@ rest_of_handle_final (void)
>  {
>const char *fnname = get_fnname_from_decl (current_function_decl);
>  
> -  /* Turn debug markers into notes.  */
> -  if (!MAY_HAVE_DEBUG_BIND_INSNS && MAY_HAVE_DEBUG_MARKER_INSNS)
> +  /* Turn debug markers into notes if the var-tracking pass has not
> + been invoked.  */
> +  if (!flag_var_tracking && MAY_HAVE_DEBUG_MARKER_INSNS)
>  variable_tracking_main ();
>  
>assemble_start_function (current_function_decl, fnname);
> 

Jakub

Re: [SFN] Bootstrap broken

2017-12-13 Thread Jakub Jelinek 
On Wed, Dec 13, 2017 at 11:45:51AM +0100, Jakub Jelinek wrote:
> On Wed, Dec 13, 2017 at 10:28:22AM +0100, Jakub Jelinek wrote:
> > Formatting, this should be
> >   bool can_move_early_debug_stmts
> > = ...
> > and the line is too long, so needs to be wrapped.
> > 
> > Furthermore, I must say I don't understand why
> > can_move_early_debug_stmts should care whether there are any labels in
> > dest bb or not.  That sounds very risky for introducing non-# DEBUG 
> > BEGIN_STMT
> > debug insns before labels if it could happen.  Though, if
> > gsi_stmt (gsi) == gsi_stmt (gsie), then the loop right below it will not
> > do anything and nothing cares about can_move_early_debug_stmts afterwards.
> > So, in short, can_move_early_debug_stmts is used only if
> > gsi_stmt (gsi) != gsi_stmt (gsie), and therefore
> > can_move_early_debug_stmts if it is used is can_move_debug_stmts && (1 || 
> > ...);
> > 
> > So, can we get rid of can_move_early_debug_stmts altogether and just use
> > can_move_debug_stmts in there instead?
> > 
> > Another thing I find risky is that you compute gsie_to so early and don't
> > update it.  If you don't need it above for can_move_early_debug_stmts, can
> > you just do it back where it used to be done,
> 
> Here it is everything in patch form, in case some volunteers are willing to
> test it on their targets, because we need faster turn-arounds for this.
> 
> 2017-12-13  Alexandre Oliva 
>   Jakub Jelinek  
> 
>   PR bootstrap/83396
>   PR debug/83391
>   * tree-cfgcleanup.c (remove_forwarder_block): Keep after
>   labels debug stmts that can only appear after labels.
> 
>   * gcc.dg/torture/pr83396.c: New test.
>   * g++.dg/torture/pr83391.C: New test.

Bootstrapped/regtested on x86_64-linux, i686-linux and powerpc64le-linux,
powerpc64-linux regtest still pending, ok for trunk?

> --- gcc/tree-cfgcleanup.c.jj  2017-12-12 09:48:26.813393301 +0100
> +++ gcc/tree-cfgcleanup.c 2017-12-13 11:39:03.373065381 +0100
> @@ -536,9 +536,14 @@ remove_forwarder_block (basic_block bb)
>   defined labels and labels with an EH landing pad number to the
>   new block, so that the redirection of the abnormal edges works,
>   jump targets end up in a sane place and debug information for
> - labels is retained.  */
> + labels is retained.
> +
> + While at that, move any debug stmts that appear before or in between
> + labels, but not those that can only appear after labels.  */
>gsi_to = gsi_start_bb (dest);
> -  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
> +  gsi = gsi_start_bb (bb);
> +  gimple_stmt_iterator gsie = gsi_after_labels (bb);
> +  while (gsi_stmt (gsi) != gsi_stmt (gsie))
>  {
>tree decl;
>label = gsi_stmt (gsi);
> @@ -557,6 +562,21 @@ remove_forwarder_block (basic_block bb)
>   gsi_next ();
>  }
>  
> +  /* Move debug statements if the destination has a single predecessor.  */
> +  if (can_move_debug_stmts && !gsi_end_p (gsi))
> +{
> +  gcc_assert (gsi_stmt (gsi) == gsi_stmt (gsie));
> +  gimple_stmt_iterator gsie_to = gsi_after_labels (dest);
> +  do
> + {
> +   gimple *debug = gsi_stmt (gsi);
> +   gcc_assert (is_gimple_debug (debug));
> +   gsi_remove (, false);
> +   gsi_insert_before (_to, debug, GSI_SAME_STMT);
> + }
> +  while (!gsi_end_p (gsi));
> +}
> +
>bitmap_set_bit (cfgcleanup_altered_bbs, dest->index);
>  
>/* Update the dominators.  */
> --- gcc/testsuite/g++.dg/torture/pr83391.C.jj
> +++ gcc/testsuite/g++.dg/torture/pr83391.C
> @@ -0,0 +1,36 @@
> +// PR debug/83391
> +// { dg-do compile }
> +// { dg-options "-g" }
> +// { dg-additional-options "-mbranch-cost=1" { target { i?86-*-* x86_64-*-* 
> mips*-*-* s390*-*-* avr*-*-* } } }
> +
> +unsigned char a;
> +enum E { F, G, H } b;
> +int c, d;
> +
> +void
> +foo ()
> +{
> +  int e;
> +  bool f;
> +  E g = b;
> +  while (1)
> +{
> +  unsigned char h = a ? d : 0;
> +  switch (g)
> + {
> + case 0:
> +   f = h <= 'Z' || h >= 'a' && h <= 'z';
> +   break;
> + case 1:
> +   {
> + unsigned char i = h;
> + e = 0;
> +   }
> +   if (e || h)
> + g = H;
> +   /* FALLTHRU */
> + default:
> +   c = 0;
> + }
> +}
> +}
> --- gcc/testsuite/gcc.dg/torture/pr83396.c.jj
> +++ gcc/testsuite/gcc.dg/torture/pr83396.c
> @@ -0,0 +1,38 @@
> +/* PR bootstrap/83396 */
> +/* { dg-do compile } */
> +/* { dg-options "-g" } */
> +
> +int fn1 (void);
> +void fn2 (void *, const char *);
> +void fn3 (void);
> +
> +void
> +fn4 (long long x)
> +{
> +  fn3 ();
> +}
> +
> +void
> +fn5 (long long x)
> +{
> +  if (x)
> +fn3();
> +}
> +
> +void
> +fn6 (long long x)
> +{
> +  switch (fn1 ())
> +{
> +case 0:
> +  fn5 (x);
> +case 2:
> +  fn2 (0, "");
> +  break;
> +case 1:
> +case 3:
> +  fn4(x);
> +case 5:
> +  fn2 (0, "");
> +}
> +}
> 
> 
>

Re: [SFN] Bootstrap broken

2017-12-13 Thread Jakub Jelinek 
On Wed, Dec 13, 2017 at 02:31:07PM +0100, Rainer Orth wrote:
> Hi Jakub,
> 
> > Here it is everything in patch form, in case some volunteers are willing to
> > test it on their targets, because we need faster turn-arounds for this.
> 
> thanks for that: it's easy to loose track in this maze ;-)

True.  What I'm regtesting (bootstraps already done) on
{x86_64,i686,powerpc64{,le}}-linux now is:
https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00811.html
https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00808.html
https://gcc.gnu.org/bugzilla/attachment.cgi?id=42861
https://gcc.gnu.org/bugzilla/attachment.cgi?id=42866
https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00794.html
set.  Does pr69102.c FAIL with that set?

> I've just bootstrapped sparc-sun-solaris2.11 with your patch and this one:
> 
>   https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00794.html
> 
> The bootstrap succeeds, but the gcc.c-torture/compile/pr69102.c
> regression persists.  Besides, I see
> 
> +FAIL: libgomp.graphite/force-parallel-4.c scan-tree-dump-times graphite "2 
> loops carried no dependency" 1 (found 0 times)
> +FAIL: libgomp.graphite/force-parallel-4.c scan-tree-dump-times optimized 
> "loopfn.1" 4 (found 0 times)
> +FAIL: libgomp.graphite/force-parallel-8.c scan-tree-dump-times graphite "5 
> loops carried no dependency" 1 (found 0 times)
> 
> which is most likely unrelated (I upgraded the tree from r255584 to
> r255603).

Yeah, these are almost certainly unrelated.

Jakub

Re: [SFN] Bootstrap broken

2017-12-13 Thread Rainer Orth 
Hi Jakub,

> Here it is everything in patch form, in case some volunteers are willing to
> test it on their targets, because we need faster turn-arounds for this.

thanks for that: it's easy to loose track in this maze ;-)

I've just bootstrapped sparc-sun-solaris2.11 with your patch and this one:

https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00794.html

The bootstrap succeeds, but the gcc.c-torture/compile/pr69102.c
regression persists.  Besides, I see

+FAIL: libgomp.graphite/force-parallel-4.c scan-tree-dump-times graphite "2 
loops carried no dependency" 1 (found 0 times)
+FAIL: libgomp.graphite/force-parallel-4.c scan-tree-dump-times optimized 
"loopfn.1" 4 (found 0 times)
+FAIL: libgomp.graphite/force-parallel-8.c scan-tree-dump-times graphite "5 
loops carried no dependency" 1 (found 0 times)

which is most likely unrelated (I upgraded the tree from r255584 to
r255603).

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [SFN] Bootstrap broken

2017-12-13 Thread Jakub Jelinek 
On Wed, Dec 13, 2017 at 10:28:22AM +0100, Jakub Jelinek wrote:
> Formatting, this should be
>   bool can_move_early_debug_stmts
> = ...
> and the line is too long, so needs to be wrapped.
> 
> Furthermore, I must say I don't understand why
> can_move_early_debug_stmts should care whether there are any labels in
> dest bb or not.  That sounds very risky for introducing non-# DEBUG BEGIN_STMT
> debug insns before labels if it could happen.  Though, if
> gsi_stmt (gsi) == gsi_stmt (gsie), then the loop right below it will not
> do anything and nothing cares about can_move_early_debug_stmts afterwards.
> So, in short, can_move_early_debug_stmts is used only if
> gsi_stmt (gsi) != gsi_stmt (gsie), and therefore
> can_move_early_debug_stmts if it is used is can_move_debug_stmts && (1 || 
> ...);
> 
> So, can we get rid of can_move_early_debug_stmts altogether and just use
> can_move_debug_stmts in there instead?
> 
> Another thing I find risky is that you compute gsie_to so early and don't
> update it.  If you don't need it above for can_move_early_debug_stmts, can
> you just do it back where it used to be done,

Here it is everything in patch form, in case some volunteers are willing to
test it on their targets, because we need faster turn-arounds for this.

2017-12-13  Alexandre Oliva 
Jakub Jelinek  

PR bootstrap/83396
PR debug/83391
* tree-cfgcleanup.c (remove_forwarder_block): Keep after
labels debug stmts that can only appear after labels.

* gcc.dg/torture/pr83396.c: New test.
* g++.dg/torture/pr83391.C: New test.

--- gcc/tree-cfgcleanup.c.jj2017-12-12 09:48:26.813393301 +0100
+++ gcc/tree-cfgcleanup.c   2017-12-13 11:39:03.373065381 +0100
@@ -536,9 +536,14 @@ remove_forwarder_block (basic_block bb)
  defined labels and labels with an EH landing pad number to the
  new block, so that the redirection of the abnormal edges works,
  jump targets end up in a sane place and debug information for
- labels is retained.  */
+ labels is retained.
+
+ While at that, move any debug stmts that appear before or in between
+ labels, but not those that can only appear after labels.  */
   gsi_to = gsi_start_bb (dest);
-  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
+  gsi = gsi_start_bb (bb);
+  gimple_stmt_iterator gsie = gsi_after_labels (bb);
+  while (gsi_stmt (gsi) != gsi_stmt (gsie))
 {
   tree decl;
   label = gsi_stmt (gsi);
@@ -557,6 +562,21 @@ remove_forwarder_block (basic_block bb)
gsi_next ();
 }
 
+  /* Move debug statements if the destination has a single predecessor.  */
+  if (can_move_debug_stmts && !gsi_end_p (gsi))
+{
+  gcc_assert (gsi_stmt (gsi) == gsi_stmt (gsie));
+  gimple_stmt_iterator gsie_to = gsi_after_labels (dest);
+  do
+   {
+ gimple *debug = gsi_stmt (gsi);
+ gcc_assert (is_gimple_debug (debug));
+ gsi_remove (, false);
+ gsi_insert_before (_to, debug, GSI_SAME_STMT);
+   }
+  while (!gsi_end_p (gsi));
+}
+
   bitmap_set_bit (cfgcleanup_altered_bbs, dest->index);
 
   /* Update the dominators.  */
--- gcc/testsuite/g++.dg/torture/pr83391.C.jj
+++ gcc/testsuite/g++.dg/torture/pr83391.C
@@ -0,0 +1,36 @@
+// PR debug/83391
+// { dg-do compile }
+// { dg-options "-g" }
+// { dg-additional-options "-mbranch-cost=1" { target { i?86-*-* x86_64-*-* 
mips*-*-* s390*-*-* avr*-*-* } } }
+
+unsigned char a;
+enum E { F, G, H } b;
+int c, d;
+
+void
+foo ()
+{
+  int e;
+  bool f;
+  E g = b;
+  while (1)
+{
+  unsigned char h = a ? d : 0;
+  switch (g)
+   {
+   case 0:
+ f = h <= 'Z' || h >= 'a' && h <= 'z';
+ break;
+   case 1:
+ {
+   unsigned char i = h;
+   e = 0;
+ }
+ if (e || h)
+   g = H;
+ /* FALLTHRU */
+   default:
+ c = 0;
+   }
+}
+}
--- gcc/testsuite/gcc.dg/torture/pr83396.c.jj
+++ gcc/testsuite/gcc.dg/torture/pr83396.c
@@ -0,0 +1,38 @@
+/* PR bootstrap/83396 */
+/* { dg-do compile } */
+/* { dg-options "-g" } */
+
+int fn1 (void);
+void fn2 (void *, const char *);
+void fn3 (void);
+
+void
+fn4 (long long x)
+{
+  fn3 ();
+}
+
+void
+fn5 (long long x)
+{
+  if (x)
+fn3();
+}
+
+void
+fn6 (long long x)
+{
+  switch (fn1 ())
+{
+case 0:
+  fn5 (x);
+case 2:
+  fn2 (0, "");
+  break;
+case 1:
+case 3:
+  fn4(x);
+case 5:
+  fn2 (0, "");
+}
+}


Jakub

Re: [SFN] Bootstrap broken

2017-12-13 Thread Jakub Jelinek 
On Wed, Dec 13, 2017 at 05:22:32AM -0200, Alexandre Oliva wrote:
> On Dec 12, 2017, Rainer Orth  wrote:
> 
> > Hi David,
> >> Something in this series broke bootstrap on AIX, probably Power in general.
> 
> > I'm seeing the same in a sparc-sun-solaris2.11 bootstrap.
> 
> The AIX patch, that I've just emailed out in this thread, should fix
> that as well.  As for the regression you reported, here's a fix.
> Regstrapping; ok to install?
> 
> 
> [SFN] don't assume BLOCK_FOR_INSN is set in var-tracking
> 
> There's no guarantee that BLOCK_FOR_INSN will be set before var-tracking.
> So, keep track of whether we're in the first block header or inside a BB
> explicitly, and apply the logic we meant to apply outside BBs only when
> we are indeed outside a BB.
> 
> for  gcc/ChangeLog
> 
>   PR bootstrap/83396
>   * var-tracking.c (vt_initialize): Keep track of BB boundaries.

This looks like a workaround for a bigger problem.

In particular, this testcase is using selective scheduling, therefore
we turn off -fvar-tracking-assignments, but the debug stmt markers are
emitted anyway.

-fvar-tracking is still true, so the var-tracking pass does everything it
normally does (successfully), then the free_cfg pass removes all
BLOCK_FOR_INSN notes, then some targets in their machine reorg recompute
those, but sparc apparently doesn't, and finally in final.c:

  /* Turn debug markers into notes.  */
  if (!MAY_HAVE_DEBUG_BIND_INSNS && MAY_HAVE_DEBUG_MARKER_INSNS)
variable_tracking_main ();

Eeek, this runs all of the var tracking again, even when it has been done
earlier, but this time without BLOCK_FOR_INSN.

This is just wrong.  So, I think the right fix here is instead (so far
tested just with sparc cross-compiler on the single testcase).

Or export delete_vta_debug_insns function and call that under that condition
instead of variable_tracking_main, which will do the same thing for
!flag_var_tracking.

2017-12-13  Jakub Jelinek  

PR bootstrap/83396
* final.c (rest_of_handle_final): Call variable_tracking_main only
if !flag_var_tracking.

--- gcc/final.c.jj  2017-12-12 09:48:15.0 +0100
+++ gcc/final.c 2017-12-13 11:29:12.284676265 +0100
@@ -4541,8 +4541,9 @@ rest_of_handle_final (void)
 {
   const char *fnname = get_fnname_from_decl (current_function_decl);
 
-  /* Turn debug markers into notes.  */
-  if (!MAY_HAVE_DEBUG_BIND_INSNS && MAY_HAVE_DEBUG_MARKER_INSNS)
+  /* Turn debug markers into notes if the var-tracking pass has not
+ been invoked.  */
+  if (!flag_var_tracking && MAY_HAVE_DEBUG_MARKER_INSNS)
 variable_tracking_main ();
 
   assemble_start_function (current_function_decl, fnname);


Jakub

Re: [SFN] Bootstrap broken

2017-12-13 Thread Jakub Jelinek 
On Wed, Dec 13, 2017 at 05:21:01AM -0200, Alexandre Oliva wrote:
> index ..098a1101a3f4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/torture/pr83391.C
> @@ -0,0 +1,34 @@
> +/* PR debug/83391 */
> +/* { dg-do compile } */

If you put this into dg-torture.exp, please add:
/* { dg-options "-g" } */
and readd the needed:
/* { dg-additional-options "-mbranch-cost=1" { target { i?86-*-* x86_64-*-* 
mips*-*-* s390*-*-* avr*-*-* } } } */

> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr83396.c
> @@ -0,0 +1,37 @@
> +/* PR bootstrap/83396 */
> +/* { dg-do compile } */

Please add -g.

> --- a/gcc/tree-cfgcleanup.c
> +++ b/gcc/tree-cfgcleanup.c
> @@ -536,14 +536,23 @@ remove_forwarder_block (basic_block bb)
>   defined labels and labels with an EH landing pad number to the
>   new block, so that the redirection of the abnormal edges works,
>   jump targets end up in a sane place and debug information for
> - labels is retained.  */
> + labels is retained.
> +
> + While at that, move any debug stmts that appear before or among
> + labels, but not those that can only appear after labels, unless
> + the destination block didn't have labels of its own.  */
>gsi_to = gsi_start_bb (dest);
> -  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
> +  gsi = gsi_start_bb (bb);
> +  gimple_stmt_iterator gsie = gsi_after_labels (bb);
> +  gimple_stmt_iterator gsie_to = gsi_after_labels (dest);
> +  bool can_move_early_debug_stmts = can_move_debug_stmts
> +&& (gsi_stmt (gsi) != gsi_stmt (gsie) || gsi_stmt (gsi_to) != gsi_stmt 
> (gsie_to));

Formatting, this should be
  bool can_move_early_debug_stmts
= ...
and the line is too long, so needs to be wrapped.

Furthermore, I must say I don't understand why
can_move_early_debug_stmts should care whether there are any labels in
dest bb or not.  That sounds very risky for introducing non-# DEBUG BEGIN_STMT
debug insns before labels if it could happen.  Though, if
gsi_stmt (gsi) == gsi_stmt (gsie), then the loop right below it will not
do anything and nothing cares about can_move_early_debug_stmts afterwards.
So, in short, can_move_early_debug_stmts is used only if
gsi_stmt (gsi) != gsi_stmt (gsie), and therefore
can_move_early_debug_stmts if it is used is can_move_debug_stmts && (1 || ...);

So, can we get rid of can_move_early_debug_stmts altogether and just use
can_move_debug_stmts in there instead?

Another thing I find risky is that you compute gsie_to so early and don't
update it.  If you don't need it above for can_move_early_debug_stmts, can
you just do it back where it used to be done,

> +  while (gsi_stmt (gsi) != gsi_stmt (gsie))
>  {
>tree decl;
>label = gsi_stmt (gsi);
>if (is_gimple_debug (label)
> -   ? can_move_debug_stmts
> +   ? can_move_early_debug_stmts
> : ((decl = gimple_label_label (as_a  (label))),
>EH_LANDING_PAD_NR (decl) != 0
>|| DECL_NONLOCAL (decl)
> @@ -557,6 +566,20 @@ remove_forwarder_block (basic_block bb)
>   gsi_next ();
>  }
>  
> +  /* Move debug statements if the destination has a single predecessor.  */
> +  if (can_move_debug_stmts && !gsi_end_p (gsi))
> +{
> +  gcc_assert (gsi_stmt (gsi) == gsi_stmt (gsie));

i.e. here?

> +  do
> + {
> +   gimple *debug = gsi_stmt (gsi);
> +   gcc_assert (is_gimple_debug (debug));
> +   gsi_remove (, false);
> +   gsi_insert_before (_to, debug, GSI_SAME_STMT);
> + }
> +  while (!gsi_end_p (gsi));
> +}
> +
>bitmap_set_bit (cfgcleanup_altered_bbs, dest->index);
>  
>/* Update the dominators.  */

Jakub

Re: [SFN] Bootstrap broken

2017-12-13 Thread Rainer Orth 
Hi Alexandre Oliva  writes:

> On Dec 12, 2017, Rainer Orth  wrote:
>
>> Hi David,
>>> Something in this series broke bootstrap on AIX, probably Power in general.
>
>> I'm seeing the same in a sparc-sun-solaris2.11 bootstrap.
>
> The AIX patch, that I've just emailed out in this thread, should fix
> that as well.  As for the regression you reported, here's a fix.

it did indeed, and this one fixed the testsuite regression, as just
confirmed in a sparc-sun-solaris2.11 bootstrap.

Thanks.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [SFN] Bootstrap broken

2017-12-13 Thread Jakub Jelinek 
On Wed, Dec 13, 2017 at 05:29:28AM -0200, Alexandre Oliva wrote:
> Regstrapping; I suppose I could install it as obvious, but...  Ok to install?
> 
> 
> [SFN] don't eliminate regs in markers
> 
> Eliminate regs in debug bind insns, but not in markers.
> 
> for  gcc/ChangeLog
> 
>   PR bootstrap/83396
>   * reload1.c (eliminate_regs_in_insn): Skip debug markers.

Ok.

> --- a/gcc/reload1.c
> +++ b/gcc/reload1.c
> @@ -3202,7 +3202,7 @@ eliminate_regs_in_insn (rtx_insn *insn, int replace)
> || GET_CODE (PATTERN (insn)) == USE
> || GET_CODE (PATTERN (insn)) == CLOBBER
> || GET_CODE (PATTERN (insn)) == ASM_INPUT);
> -  if (DEBUG_INSN_P (insn))
> +  if (DEBUG_BIND_INSN_P (insn))
>   INSN_VAR_LOCATION_LOC (insn)
> = eliminate_regs (INSN_VAR_LOCATION_LOC (insn), VOIDmode, insn);
>return 0;

Jakub

Re: [SFN] Bootstrap broken

2017-12-12 Thread Alexandre Oliva 
On Dec 12, 2017, David Edelsohn  wrote:

> Rainer,
> PR83396 opened.  you can add Solaris to the list of targets.

Andreas,

Here's a fix for the ia64 regression you mentioned in that PR.

I don't include the 'int main(){return 0;}' testcase, because it would
hardly be useful; any test would trigger this error on the right
platform.

Regstrapping; I suppose I could install it as obvious, but...  Ok to install?


[SFN] don't eliminate regs in markers

Eliminate regs in debug bind insns, but not in markers.

for  gcc/ChangeLog

PR bootstrap/83396
* reload1.c (eliminate_regs_in_insn): Skip debug markers.
---
 gcc/reload1.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/reload1.c b/gcc/reload1.c
index 322696a25f3e..fe1ec0d011fb 100644
--- a/gcc/reload1.c
+++ b/gcc/reload1.c
@@ -3202,7 +3202,7 @@ eliminate_regs_in_insn (rtx_insn *insn, int replace)
  || GET_CODE (PATTERN (insn)) == USE
  || GET_CODE (PATTERN (insn)) == CLOBBER
  || GET_CODE (PATTERN (insn)) == ASM_INPUT);
-  if (DEBUG_INSN_P (insn))
+  if (DEBUG_BIND_INSN_P (insn))
INSN_VAR_LOCATION_LOC (insn)
  = eliminate_regs (INSN_VAR_LOCATION_LOC (insn), VOIDmode, insn);
   return 0;


-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [SFN] Bootstrap broken

2017-12-12 Thread Alexandre Oliva 
On Dec 12, 2017, Rainer Orth  wrote:

> Hi David,
>> Something in this series broke bootstrap on AIX, probably Power in general.

> I'm seeing the same in a sparc-sun-solaris2.11 bootstrap.

The AIX patch, that I've just emailed out in this thread, should fix
that as well.  As for the regression you reported, here's a fix.
Regstrapping; ok to install?


[SFN] don't assume BLOCK_FOR_INSN is set in var-tracking

There's no guarantee that BLOCK_FOR_INSN will be set before var-tracking.
So, keep track of whether we're in the first block header or inside a BB
explicitly, and apply the logic we meant to apply outside BBs only when
we are indeed outside a BB.

for  gcc/ChangeLog

PR bootstrap/83396
* var-tracking.c (vt_initialize): Keep track of BB boundaries.
---
 gcc/var-tracking.c |   12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c
index 8e500b144712..12158dbb1e0d 100644
--- a/gcc/var-tracking.c
+++ b/gcc/var-tracking.c
@@ -10156,12 +10156,16 @@ vt_initialize (void)
  /* If we are walking the first basic block, walk any HEADER
 insns that might be before it too.  Unfortunately,
 BB_HEADER and BB_FOOTER are not set while we run this
-pass.  */
+pass.  Unfortunately, BLOCK_FOR_INSN may not be set, so
+we can't assume that its being NULL implies we're outside
+the basic block.  */
  insn = get_first_insn (bb);
+ bool outside_bb = insn != BB_HEAD (bb);
  for (rtx_insn *next;
   insn != BB_HEAD (bb->next_bb)
 ? next = NEXT_INSN (insn), true : false;
-  insn = next)
+  insn = next,
+outside_bb = outside_bb && next != BB_HEAD (bb))
{
  if (INSN_P (insn))
{
@@ -10169,11 +10173,11 @@ vt_initialize (void)
  if (!BLOCK_FOR_INSN (insn))
{
  BLOCK_FOR_INSN (insn) = bb;
- gcc_assert (DEBUG_INSN_P (insn));
+ gcc_assert (!outside_bb || DEBUG_INSN_P (insn));
  /* Reset debug insns between basic blocks.
 Their location is not reliable, because they
 were probably not maintained up to date.  */
- if (DEBUG_BIND_INSN_P (insn))
+ if (outside_bb && DEBUG_BIND_INSN_P (insn))
INSN_VAR_LOCATION_LOC (insn)
  = gen_rtx_UNKNOWN_VAR_LOC ();
}


-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [SFN] Bootstrap broken

2017-12-12 Thread Alexandre Oliva 
On Dec 12, 2017, David Edelsohn  wrote:

> Something in this series broke bootstrap on AIX, probably Power in general.

And probably a number of other platforms as well.

Here's a patch that seems to have fixed lots of problems.  Without it,
we might move debug (bind) stmts from a forwarder block about to be
removed, inserting them before the label of a successor block.  debug
bind stmts before labels are not welcome (they might become insns
outside any blocks, and not be properly adjusted; in this specific
testcase, a reg lowered to a concatn remained shared because the
out-of-block debug insn was not adjusted), and I'm reconsidering even
debug markers, but for now, I'm leaving markers before and among labels.

To fix the problem, I've rearranged the forwarder block remover to
insert stmts that were before or among labels before labels, but those
that are after labels (or in a block without labels) are inserted after
any labels in the destination block.  This should keep debug insns in
order, except when the forwarder block has no labels, whereas the
successor block had markers before the labels.  I'm undecided between
moving all markers in the successor after any labels in it, before
making other changes, or to rule out markers before or among labels
altogether.

For now, to restore bootstrap and builds on most platforms, this will
do.  There are still unrelated -fcompare-debug issues in the supplied
AIX testcase, but Jakub's reduced testcase passes even -fcompare-debug.

Regstrapping on x86_64-linux-gnu and i686-linux-gnu.  Hopefully I'll
have feedback about its bootstrap on powerpc-aix and sparc-solaris as
well before it goes in.  Ok to install?


[SFN] don't move after-label debug stmts before labels

When removing a forwarder block (that contains debug stmts), be careful
to not insert before labels debug stmts that appear after labels.

for  gcc/ChangeLog

PR bootstrap/83396
PR debug/83391
* tree-cfgcleanup.c (remove_forwarder_block): Keep after
labels debug stmts appearing after labels.

for  gcc/testsuite/ChangeLog

PR bootstrap/83396
PR debug/83391
* gcc.dg/torture/pr83396.c: New.
* g++.dg/torture/pr83391.C: New.
---
 gcc/testsuite/g++.dg/torture/pr83391.C |   34 +
 gcc/testsuite/gcc.dg/torture/pr83396.c |   37 
 gcc/tree-cfgcleanup.c  |   29 ++---
 3 files changed, 97 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/torture/pr83391.C
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr83396.c

diff --git a/gcc/testsuite/g++.dg/torture/pr83391.C 
b/gcc/testsuite/g++.dg/torture/pr83391.C
new file mode 100644
index ..098a1101a3f4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr83391.C
@@ -0,0 +1,34 @@
+/* PR debug/83391 */
+/* { dg-do compile } */
+
+unsigned char a;
+enum E { F, G, H } b;
+int c, d;
+
+void
+foo ()
+{
+  int e;
+  bool f;
+  E g = b;
+  while (1)
+{
+  unsigned char h = a ? d : 0;
+  switch (g)
+   {
+   case 0:
+ f = h <= 'Z' || h >= 'a' && h <= 'z';
+ break;
+   case 1:
+ {
+   unsigned char i = h;
+   e = 0;
+ }
+ if (e || h)
+   g = H;
+ /* FALLTHRU */
+   default:
+ c = 0;
+   }
+}
+}
diff --git a/gcc/testsuite/gcc.dg/torture/pr83396.c 
b/gcc/testsuite/gcc.dg/torture/pr83396.c
new file mode 100644
index ..4c0c30ceaab3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr83396.c
@@ -0,0 +1,37 @@
+/* PR bootstrap/83396 */
+/* { dg-do compile } */
+
+int fn1 (void);
+void fn2 (void *, const char *);
+void fn3 (void);
+
+void
+fn4 (long long x)
+{
+  fn3 ();
+}
+
+void
+fn5 (long long x)
+{
+  if (x)
+fn3();
+}
+
+void
+fn6 (long long x)
+{
+  switch (fn1 ())
+{
+case 0:
+  fn5 (x);
+case 2:
+  fn2 (0, "");
+  break;
+case 1:
+case 3:
+  fn4(x);
+case 5:
+  fn2 (0, "");
+}
+}
diff --git a/gcc/tree-cfgcleanup.c b/gcc/tree-cfgcleanup.c
index 0bee21756f2b..e30a93d504b6 100644
--- a/gcc/tree-cfgcleanup.c
+++ b/gcc/tree-cfgcleanup.c
@@ -536,14 +536,23 @@ remove_forwarder_block (basic_block bb)
  defined labels and labels with an EH landing pad number to the
  new block, so that the redirection of the abnormal edges works,
  jump targets end up in a sane place and debug information for
- labels is retained.  */
+ labels is retained.
+
+ While at that, move any debug stmts that appear before or among
+ labels, but not those that can only appear after labels, unless
+ the destination block didn't have labels of its own.  */
   gsi_to = gsi_start_bb (dest);
-  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
+  gsi = gsi_start_bb (bb);
+  gimple_stmt_iterator gsie = gsi_after_labels (bb);
+  gimple_stmt_iterator gsie_to = gsi_after_labels (dest);
+  bool

Re: [SFN] Bootstrap broken

2017-12-12 Thread David Edelsohn 
Rainer,

PR83396 opened.  you can add Solaris to the list of targets.

- David

On Tue, Dec 12, 2017 at 1:49 PM, Rainer Orth
 wrote:
> Hi David,
>
>> Something in this series broke bootstrap on AIX, probably Power in general.
>
> I'm seeing the same in a sparc-sun-solaris2.11 bootstrap.
>
> Rainer
>
>
>> /nasfarm/edelsohn/src/src/gcc/gimple-pretty-print.c: In function 'void
>> pp_gimple_stmt_1(pretty_printer*, gimple*, int, dump_flags_t)':
>> /nasfarm/edelsohn/src/src/gcc/gimple-pretty-print.c:2645:1: error:
>> invalid rtl sharing found in the insn
>>  }
>>  ^
>> (debug_insn 5082 5081 5083 (var_location:DI flags (concatn/v:DI [
>> (reg:SI 1564 [ flags ])
>> (reg:SI 1565 [ flags+4 ])
>> ])) "/nasfarm/edelsohn/src/src/gcc/gimple-pretty-print.c":2583 -1
>>  (nil))
>> /nasfarm/edelsohn/src/src/gcc/gimple-pretty-print.c:2645:1: error: shared rtx
>> (concatn/v:DI [
>> (reg:SI 1564 [ flags ])
>> (reg:SI 1565 [ flags+4 ])
>> ])
>> during RTL pass: outof_cfglayout
>> /nasfarm/edelsohn/src/src/gcc/gimple-pretty-print.c:2645:1: internal
>> compiler error: internal consistency failure
>
> --
> -
> Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [SFN] Bootstrap broken

2017-12-12 Thread Rainer Orth 
Hi David,

> Something in this series broke bootstrap on AIX, probably Power in general.

I'm seeing the same in a sparc-sun-solaris2.11 bootstrap.

Rainer


> /nasfarm/edelsohn/src/src/gcc/gimple-pretty-print.c: In function 'void
> pp_gimple_stmt_1(pretty_printer*, gimple*, int, dump_flags_t)':
> /nasfarm/edelsohn/src/src/gcc/gimple-pretty-print.c:2645:1: error:
> invalid rtl sharing found in the insn
>  }
>  ^
> (debug_insn 5082 5081 5083 (var_location:DI flags (concatn/v:DI [
> (reg:SI 1564 [ flags ])
> (reg:SI 1565 [ flags+4 ])
> ])) "/nasfarm/edelsohn/src/src/gcc/gimple-pretty-print.c":2583 -1
>  (nil))
> /nasfarm/edelsohn/src/src/gcc/gimple-pretty-print.c:2645:1: error: shared rtx
> (concatn/v:DI [
> (reg:SI 1564 [ flags ])
> (reg:SI 1565 [ flags+4 ])
> ])
> during RTL pass: outof_cfglayout
> /nasfarm/edelsohn/src/src/gcc/gimple-pretty-print.c:2645:1: internal
> compiler error: internal consistency failure

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University