[Bug middle-end/52756] New: [4.7 Regression] 255.vortex in SPEC CPU 2000 failed to build

[Bug middle-end/52756] New: [4.7 Regression] 255.vortex in SPEC CPU 2000 failed to build

[hjl.tools at gmail dot com]

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

             Bug #: 52756
           Summary: [4.7 Regression] 255.vortex in SPEC CPU 2000 failed to
                    build
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: hjl.tools@gmail.com
                CC: areg.melikadamyan@gmail.com


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.

[Bug middle-end/52756] [4.8 Regression] 255.vortex in SPEC CPU 2000 failed to build

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-03-29
         AssignedTo|unassigned at gcc dot       |rguenth at gcc dot gnu.org
                   |gnu.org                     |
   Target Milestone|---                         |4.8.0
     Ever Confirmed|0                           |1

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-29 09:40:27 UTC ---
Confirmed.  Mine.

[Bug middle-end/52756] [4.8 Regression] 255.vortex in SPEC CPU 2000 failed to build

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 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;
}

[Bug middle-end/52756] [4.8 Regression] 255.vortex in SPEC CPU 2000 failed to build

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 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.

[Bug middle-end/52756] [4.8 Regression] 255.vortex in SPEC CPU 2000 failed to build

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 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));

[Bug middle-end/52756] [4.8 Regression] 255.vortex in SPEC CPU 2000 failed to build

[law at redhat dot com]

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

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at redhat dot com

--- Comment #5 from Jeffrey A. Law <law at redhat dot com> 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.

[Bug middle-end/52756] [4.8 Regression] 255.vortex in SPEC CPU 2000 failed to build

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 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

[Bug middle-end/52756] [4.8 Regression] 255.vortex in SPEC CPU 2000 failed to build

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 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);
     }

[Bug middle-end/52756] [4.8 Regression] 255.vortex in SPEC CPU 2000 failed to build

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 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

[Bug middle-end/52756] [4.8 Regression] 255.vortex in SPEC CPU 2000 failed to build

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-03 09:08:52 UTC ---
Fixed.