This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Unset used bit in simplify_replace_* on newly copied rtxs (PR target/78614)


Hi!

Instead of a simple approach to fix PR78614 (a rs6000 backend bug) by adding:
pat = copy_rtx (pat);
before
  XVECEXP (pat, ...) = simplify_replace_rtx (XVECEXP (pat, ...), x, y);
because simplify_replace_rtx doesn't unshare all rtxes, just those required
not to modify the original expression (i.e. whenever changing x to copy_rtx
(y) somewhere also all expressions containing those), I've tried to avoid
too much GC creation by doing:
set_used_flags (pat);
  pat = shallow_copy_rtx (pat);
  XVEC (pat, 0) = shallow_copy_rtvec (XVEC (pat, 0));
  RTX_FLAG (pat, used) = 0;
...
  XVECEXP (pat, ...) = simplify_replace_rtx (XVECEXP (pat, ...), x, y);
...
  pat = copy_rtx_if_shared (pat);
I run into the problem that while simplify_replace_rtx if it actually does
copy_rtx (for y) or if it simplifies something through
simplify_gen_{unary,binary,relational,ternary}, the used bits on the newly
created rtxes are cleared, when we fall through into the fallback
simplify_replace_fn_rtx handling, it calls shallow_copy_rtx which copies the
set used bit and thus copy_rtx_if_shared copies it again.

The following patch improves it by reseting the used bit, similarly how
copy_rtx resets it.  In addition, I've noticed that copy_rtx_if_shared
documents that if some rtx with Es in format is unshared, it is required
that all the rtvecs it contains are unshared as well, partial unsharing
is not valid.  Most of rtxes contain at most a single E or are just used
in gen* programs, ASM_OPERANDS is an exception that contains 3.  So the
patch also ensures that if we copy ASM_OPERANDS, we unshare all the rtvecs.

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

Now that I think about it, the second hunk should also probably contain
the for (int k = 0; fmt[k]; k++) loop, but as the only rtx with more than
one E is ssiEEEi this isn't needed with the current rtx.def.

2016-11-30  Jakub Jelinek  <jakub@redhat.com>

	PR target/78614
	* simplify-rtx.c (simplify_replace_fn_rtx): When copying at least
	one 'E' format vector, copy all of them.  Clear used flag after
	shallow_copy_rtx.

--- gcc/simplify-rtx.c.jj	2016-11-30 13:57:12.000000000 +0100
+++ gcc/simplify-rtx.c	2016-11-30 17:42:08.606817145 +0100
@@ -547,13 +547,20 @@ simplify_replace_fn_rtx (rtx x, const_rt
 					  old_rtx, fn, data);
 	    if (op != RTVEC_ELT (vec, j))
 	      {
-		if (newvec == vec)
+		if (x == newx)
 		  {
-		    newvec = shallow_copy_rtvec (vec);
-		    if (x == newx)
-		      newx = shallow_copy_rtx (x);
-		    XVEC (newx, i) = newvec;
+		    newx = shallow_copy_rtx (x);
+		    RTX_FLAG (newx, used) = 0;
+		    /* If we copy X, we need to copy also all
+		       vectors in it, rather than copy only
+		       a subset of them and share the rest.  */
+		    for (int k = 0; fmt[k]; k++)
+		      if (fmt[k] == 'E')
+			XVEC (newx, k) = shallow_copy_rtvec (XVEC (x, k));
+		    newvec = XVEC (newx, i);
 		  }
+		else
+		  gcc_checking_assert (vec != newvec);
 		RTVEC_ELT (newvec, j) = op;
 	      }
 	  }
@@ -566,7 +573,10 @@ simplify_replace_fn_rtx (rtx x, const_rt
 	    if (op != XEXP (x, i))
 	      {
 		if (x == newx)
-		  newx = shallow_copy_rtx (x);
+		  {
+		    newx = shallow_copy_rtx (x);
+		    RTX_FLAG (newx, used) = 0;
+		  }
 		XEXP (newx, i) = op;
 	      }
 	  }

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]