Bug 52756 - [4.8 Regression] 255.vortex in SPEC CPU 2000 failed to build
Summary: [4.8 Regression] 255.vortex in SPEC CPU 2000 failed to build
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.8.0
: P3 normal
Target Milestone: 4.8.0
Assignee: Richard Biener
URL:
Keywords:
Depends on:
Blocks: 52808
  Show dependency treegraph
 
Reported: 2012-03-28 17:31 UTC by H.J. Lu
Modified: 2012-04-03 09:08 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-03-29 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2012-03-28 17:31:50 UTC
On Linux/x86-64, revision 185913 gave:

gcc -c -o env1.o  -fno-strict-aliasing -DSPEC_CPU2000_LP64         -O2 -ffast-math   env1.c
...
env1.c: In function 'Env_FetchObj0AttrOffset':
env1.c:1094:12: internal compiler error: Segmentation fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
specmake[3]: *** [env1.o] Error 1

Revision 185825. is OK.
Comment 1 Richard Biener 2012-03-29 09:40:27 UTC
Confirmed.  Mine.
Comment 2 Richard Biener 2012-03-29 09:56:12 UTC
Reduced testcase, but a different ICE than the original one.

void Env_FetchObj0AttrOffset (unsigned int NumFields,  int *Status)
{
  int Found = 0;
  if (NumFields)      
    while ((*Status == 0) && NumFields-- > 0 && Found == 0)       
      Found = 1;
}

Reduced testcase for the original ICE:

void Env_FetchObj0AttrOffset (unsigned int NumFields, int AttrNum, 
                              int *Status, int *Vfields)
{
  int Found = 0;
  if (NumFields)  
    while ((*Status == 0) && NumFields-- > 0 && Found == 0)
      if (*Vfields == AttrNum)
        Found = 1;
}
Comment 3 Richard Biener 2012-03-29 12:42:37 UTC
DOM jump threading threads the loop latch edge:

  if (latch->aux)
    {
      /* First handle the case latch edge is redirected.  */
      loop->latch = thread_single_edge (latch);
      gcc_assert (single_succ (loop->latch) == tgt_bb);
      loop->header = tgt_bb;

      /* Thread the remaining edges through the former header.  */
      thread_block (header, false);

but the updated loop->latch, loop->header pair is certainly not the
proper one.  That's the real bug.  Jump threading rotates the loop
here but fails to realize that - the new header needs to dominate
all of the loops remaining BBs, and determine_bb_domination_status
does not verify that (either by design or because it is buggy).

The symptom is then caused by cfgcleaup merging latch and header and
simple latch creation being confused.
Comment 4 Richard Biener 2012-03-30 13:26:57 UTC
(In reply to comment #3)
> DOM jump threading threads the loop latch edge:
> 
>   if (latch->aux)
>     {
>       /* First handle the case latch edge is redirected.  */
>       loop->latch = thread_single_edge (latch);
>       gcc_assert (single_succ (loop->latch) == tgt_bb);
>       loop->header = tgt_bb;

At this point everything is still ok, but

>       /* Thread the remaining edges through the former header.  */
>       thread_block (header, false);

this creates a multiple entry loop (with the old entry being an
unreachable block after threading - sth cfgcleaup exposes, which
in turn would turn that into a valid loop, albeit with bogus header/latch,
again).

Now, thread_through_loop_header ensures (fingers crossing) that we never
create a multiple entry loop.  Thus the following fix would work.

Index: gcc/tree-ssa-threadupdate.c
===================================================================
--- gcc/tree-ssa-threadupdate.c (revision 186007)
+++ gcc/tree-ssa-threadupdate.c (working copy)
@@ -649,6 +649,17 @@ thread_block (basic_block bb, bool noloo
                  || THREAD_TARGET2 (e))))
        continue;
 
+      /* thread_through_loop_header made sure we are not creating
+        a multiple entry loop.  If we are creating a new loop
+        entry the destination will become the new header of the
+        loop and the old entry will become unreachable.  */
+      if (e->src->loop_father != e2->dest->loop_father)
+       {
+         e2->dest->loop_father->header = e2->dest;
+         /* Discard knowledge about the latch.  */
+         e2->dest->loop_father->latch = NULL;
+       }
+
       if (e->dest == e2->src)
        update_bb_profile_for_threading (e->dest, EDGE_FREQUENCY (e),
                                         e->count, THREAD_TARGET (e));
Comment 5 Jeffrey A. Law 2012-03-30 16:24:25 UTC
Zdenek certainly knows this part of the code better than I and may comment, but it looks reasonable to me.
Comment 6 Richard Biener 2012-04-02 11:38:59 UTC
It doesn't work, as we expect loop_latch_edge () to work during further threading.
It also does not work because we miss some threadings and thus hit the assert
that e->aux is NULL after threading is done.

The issue is that we thread both the latch and the loop entry edge.  But the
code special casing each case does not consider that both may happen at
the same time.  If we'd re-start at thread_through_loop_header for the case
in this bug we'd fail, as the entry edges THREAD_TARGET2 is set.

Thus, I'll try again with

Index: gcc/tree-ssa-threadupdate.c
===================================================================
--- gcc/tree-ssa-threadupdate.c (revision 186066)
+++ gcc/tree-ssa-threadupdate.c (working copy)
@@ -838,6 +838,7 @@ thread_through_loop_header (struct loop
   edge_iterator ei;
   basic_block tgt_bb, atgt_bb;
   enum bb_dom_status domst;
+  bool threaded_latch = false;
 
   /* We have already threaded through headers to exits, so all the threading
      requests now are to the inside of the loop.  We need to avoid creating
@@ -908,6 +909,7 @@ thread_through_loop_header (struct loop
   if (single_succ_p (header))
     goto fail;
 
+thread_rest:
   if (latch->aux)
     {
       if (THREAD_TARGET2 (latch))
@@ -916,7 +918,7 @@ thread_through_loop_header (struct loop
       tgt_bb = tgt_edge->dest;
     }
   else if (!may_peel_loop_headers
-          && !redirection_block_p (loop->header))
+          && !redirection_block_p (header))
     goto fail;
   else
     {
@@ -950,7 +952,7 @@ thread_through_loop_header (struct loop
       if (!tgt_bb)
        {
          /* There are no threading requests.  */
-         return false;
+         return threaded_latch;
        }
 
       /* Redirecting to empty loop latch is useless.  */
@@ -971,7 +973,7 @@ thread_through_loop_header (struct loop
       loop->header = NULL;
       loop->latch = NULL;
       loops_state_set (LOOPS_NEED_FIXUP);
-      return thread_block (header, false);
+      return threaded_latch | thread_block (header, false);
     }
 
   if (tgt_bb->loop_father->header == tgt_bb)
@@ -994,9 +996,10 @@ thread_through_loop_header (struct loop
       loop->latch = thread_single_edge (latch);
       gcc_assert (single_succ (loop->latch) == tgt_bb);
       loop->header = tgt_bb;
+      threaded_latch = true;
 
       /* Thread the remaining edges through the former header.  */
-      thread_block (header, false);
+      goto thread_rest;
     }
   else
     {
@@ -1039,7 +1042,7 @@ fail:
       free (e->aux);
       e->aux = NULL;
     }
-  return false;
+  return threaded_latch;
 }
 
 /* Walk through the registered jump threads and convert them into a
Comment 7 Richard Biener 2012-04-02 14:05:31 UTC
With that patch gcc.dg/tree-ssa/ssa-dom-thread-2.c regresses as we do not
perform the threading through the header edge anymore.  Note that using
thread_block (header, true); for the rest of the header edges does not
work either, as it still happily creates the multiple entry loop with
the bogus header.

The following is another possibility - remove the remaining threadings
that would be harmful but allow those that would be safe.

Index: gcc/tree-ssa-threadupdate.c
===================================================================
--- gcc/tree-ssa-threadupdate.c (revision 186082)
+++ gcc/tree-ssa-threadupdate.c (working copy)
@@ -826,6 +826,17 @@ determine_bb_domination_status (struct l
   return (bb_reachable ? DOMST_DOMINATING : DOMST_LOOP_BROKEN);
 }
 
+/* Return true if BB is part of the new pre-header that is created
+   when threading the latch to DATA.  */
+
+static bool
+def_split_header_continue_p (const_basic_block bb, const void *data)
+{
+  const_basic_block new_header = (const_basic_block) data;
+  return (bb->loop_father == new_header->loop_father
+         && bb != new_header);
+}
+
 /* Thread jumps through the header of LOOP.  Returns true if cfg changes.
    If MAY_PEEL_LOOP_HEADERS is false, we avoid threading from entry edges
    to the inside of the loop.  */
@@ -990,11 +1001,46 @@ thread_through_loop_header (struct loop
 
   if (latch->aux)
     {
+      basic_block *bblocks;
+      unsigned nblocks, i;
+
       /* First handle the case latch edge is redirected.  */
       loop->latch = thread_single_edge (latch);
       gcc_assert (single_succ (loop->latch) == tgt_bb);
       loop->header = tgt_bb;
 
+      /* Remove the new pre-header blocks from our loop.  */
+      bblocks = XCNEWVEC (basic_block, loop->num_nodes);
+      nblocks = dfs_enumerate_from (header, 0, def_split_header_continue_p,
+                                   bblocks, loop->num_nodes, tgt_bb);
+      for (i = 0; i < nblocks; i++)
+       {
+         remove_bb_from_loops (bblocks[i]);
+         add_bb_to_loop (bblocks[i], loop_outer (loop));
+       }
+      free (bblocks);
+
+      /* Cancel remaining threading requests that would make the
+        loop a multiple entry loop.  */
+      FOR_EACH_EDGE (e, ei, header->preds)
+       {
+         edge e2;
+         if (e->aux == NULL)
+           continue;
+
+         if (THREAD_TARGET2 (e))
+           e2 = THREAD_TARGET2 (e);
+         else
+           e2 = THREAD_TARGET (e);
+
+         if (e->src->loop_father != e2->dest->loop_father
+             && e2->dest != loop->header)
+           {
+             free (e->aux);
+             e->aux = NULL;
+           }
+       }
+
       /* Thread the remaining edges through the former header.  */
       thread_block (header, false);
     }
Comment 8 Richard Biener 2012-04-02 15:13:50 UTC
Author: rguenth
Date: Mon Apr  2 15:13:45 2012
New Revision: 186085

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186085
Log:
2012-04-02  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/52756
	* tree-ssa-threadupdate.c (def_split_header_continue_p): New function.
	(thread_through_loop_header): After threading through the loop latch
	remove the split part from the loop and clear further threading
	opportunities that would create a multiple entry loop.

	* gcc.dg/torture/pr52756.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr52756.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-threadupdate.c
Comment 9 Richard Biener 2012-04-03 09:08:52 UTC
Fixed.