From patchwork Fri Jun 25 16:03:08 2010
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [rfa] Fix PR44592: wrong code
Date: Fri, 25 Jun 2010 06:03:08 -0000
From: Michael Matz <matz@suse.de>
X-Patchwork-Id: 56924
Message-Id: <Pine.LNX.4.64.1006251746400.18619@wotan.suse.de>
To: gcc-patches@gcc.gnu.org

Hello,

For callers of gimplify_and_update_call_from_tree() that possibly traverse 
the VOP chains we have to make the new sequence as a whole have the same 
effects as the original statements (from a VOP perspective), i.e. 
attaching the old VOPs to the new sequence.  There was a latent ommission 
uncovered by my more general use of the above function.  It involved 
transforming a call without LHS but VOPs (e.g. memcpy) into stores.

Hence, we track the last store of the new sequence, and possibly attach 
the VOPs of the original statement to that one.

Fixed the testcase, regstrapping on x86_64-linux in progress.  Okay for 
trunk and 4.5 after some time?


Ciao,
Michael.


Index: gimple-fold.c
===================================================================
--- gimple-fold.c	(revision 161389)
+++ gimple-fold.c	(working copy)
@@ -1065,6 +1065,7 @@ gimplify_and_update_call_from_tree (gimp
   gimple_seq stmts = gimple_seq_alloc();
   struct gimplify_ctx gctx;
   gimple last = NULL;
+  gimple laststore = NULL;
 
   stmt = gsi_stmt (*si_p);
 
@@ -1096,12 +1097,28 @@ gimplify_and_update_call_from_tree (gimp
       find_new_referenced_vars (new_stmt);
       mark_symbols_for_renaming (new_stmt);
       last = new_stmt;
+      if (gimple_assign_single_p (last)
+	  && !is_gimple_reg (gimple_assign_lhs (last)))
+	laststore = last;
     }
 
   if (lhs == NULL_TREE)
     {
-      unlink_stmt_vdef (stmt);
-      release_defs (stmt);
+      /* If we replace a call without LHS that has a VDEF and our new
+         sequence ends with a store we must make that store have the same
+	 vdef in order not to break the sequencing.  This can happen
+	 for instance when folding memcpy calls into assignments.  */
+      if (gimple_vdef (stmt) && laststore)
+	{
+	  gimple_set_vuse (laststore, gimple_vuse (stmt));
+	  gimple_set_vdef (laststore, gimple_vdef (stmt));
+	  move_ssa_defining_stmt_for_defs (laststore, stmt);
+	}
+      else
+	{
+	  unlink_stmt_vdef (stmt);
+	  release_defs (stmt);
+	}
       new_stmt = last;
     }
   else
Index: testsuite/gfortran.dg/pr44592.f90
===================================================================
--- testsuite/gfortran.dg/pr44592.f90	(revision 0)
+++ testsuite/gfortran.dg/pr44592.f90	(revision 0)
@@ -0,0 +1,20 @@
+! { dg-do run }
+! { dg-options "-O3" }
+! From forall_12.f90
+! Fails with loop reversal at -O3
+!
+  character(len=1) :: b(4) = (/"1","2","3","4"/), c(4)
+  c = b
+  i = 1
+  ! This statement must be here for the abort below
+  b(1:3)(i:i) = b(2:4)(i:i)
+
+  b = c
+  b(4:2:-1)(i:i) = b(3:1:-1)(i:i)
+
+  ! This fails.  If the condition is printed, the result is F F F F
+  if (any (b .ne. (/"1","1","2","3"/))) i = 2
+  print *, b
+  print *, b .ne. (/"1","1","2","3"/)
+  if (i == 2) call abort
+end