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

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

[Jakub Jelinek]

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

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

[Bernd Schmidt]

On 11/30/2016 11:11 PM, Jakub Jelinek wrote:
> 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.

Shouldn't the bit be cleared in shallow_copy_rtx then?


Bernd

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

[Jakub Jelinek]

On Thu, Dec 01, 2016 at 01:19:16PM +0100, Bernd Schmidt wrote:
> On 11/30/2016 11:11 PM, Jakub Jelinek wrote:
> >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.
> 
> Shouldn't the bit be cleared in shallow_copy_rtx then?

Not very easily.  The problem is that the used bit means lots of things for
various rtls:

  /* At the end of RTL generation, 1 if this rtx is used.  This is used for
     copying shared structure.  See `unshare_all_rtl'.
     In a REG, this is not needed for that purpose, and used instead
     in `leaf_renumber_regs_insn'.
     1 in a SYMBOL_REF, means that emit_library_call
     has used it as the function.
     1 in a CONCAT is VAL_HOLDS_TRACK_EXPR in var-tracking.c.
     1 in a VALUE or DEBUG_EXPR is VALUE_RECURSED_INTO in var-tracking.c. 
     */
  unsigned int used : 1;

so we'd need to slow shallow_copy_rtx down by adding switch (code)
in there or something similar.  The callers of it have the advantage that
they know they call shallow_copy_rtx already on something that shouldn't be
shared, and many of them take care of the used bit already (some clear it,
other callers set it).  So clearing it in shallow_copy_rtx for the callers
that set it would be just waste of time.  But if you strongly prefer that
I can try to implement it.

That said, I found a bug in my patch, ADDR_DIFF_VEC has "eEee0" format, thus
if simplify_replace_fn_rtx replaces anything in the first operand, then
either it wouldn't properly duplicate the vector operand if no changes are
needed there, or it would ICE with the previous version of the patch.
So here is an updated patch that fixes this.

2016-12-01  Jakub Jelinek  <jakub@redhat.com>

	PR target/78614
	* simplify-rtx.c (simplify_replace_fn_rtx): When copying rtx with
	'E' in format, copy all vectors.  Clear used flag after
	shallow_copy_rtx.

--- gcc/simplify-rtx.c.jj	2016-12-01 08:53:37.134284466 +0100
+++ gcc/simplify-rtx.c	2016-12-01 23:32:45.074152770 +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,16 @@ 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;
+		    /* 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));
+		  }
 		XEXP (newx, i) = op;
 	      }
 	  }


	Jakub

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

[Bernd Schmidt]

On 12/01/2016 11:43 PM, Jakub Jelinek wrote:
>
> so we'd need to slow shallow_copy_rtx down by adding switch (code)
> in there or something similar.

Is there reason to assume it's time-critical? IMO eliminating the 
potential for error by not having callers need to do this outweighs that 
concern.


Bernd

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

[Jakub Jelinek]

On Thu, Dec 01, 2016 at 11:48:03PM +0100, Bernd Schmidt wrote:
> On 12/01/2016 11:43 PM, Jakub Jelinek wrote:
> >
> >so we'd need to slow shallow_copy_rtx down by adding switch (code)
> >in there or something similar.
> 
> Is there reason to assume it's time-critical? IMO eliminating the potential
> for error by not having callers need to do this outweighs that concern.

Well, it isn't an error not to clear it, just missed optimization if some caller
cares, and most of the shallow_copy_rtx users really don't care, e.g.
config/i386/i386.md and config/i386/i386.c callers.  cfgexpand.c doesn't
either, at that point the used bit isn't set, the calls in cselib.c don't care
either.  In emit-rtl.c one place explicitly sets used flag afterwards, another
clears it (that one could be removed if the logic is moved down).  The rs6000.c
use indeed could get rid of the explicit used bit clearing.  Most of the
other shallow_copy_rtx callers in the middle-end just don't care.
I think it is really just simplify_replace_fn_rtx where (at least in the
rs6000 case) it is useful to clear it, similar trick is not used in many
places after initial unsharing during expansion before which the bit should
be clear on everything shareable, I saw it in ifcvt.c (set_used_flags
+ some copying + unshare_*_chain afterwards).
After expansion when everything is unshared, gradually the used bits are no
longer relevant, they are set on stuff that has been originally unshared and
clear on newly added rtxes.  Then the reload/postreload
unshare_all_rtx_again clears it and unshares the shared stuff.
So it is really just about spots that do the trick of set_used_flags,
call some functions where clearing of used bit on new stuff is important,
and finally call copy_rtx_if_shared.

	Jakub

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

[Jakub Jelinek]

On Fri, Dec 02, 2016 at 12:34:13AM +0100, Jakub Jelinek wrote:
> On Thu, Dec 01, 2016 at 11:48:03PM +0100, Bernd Schmidt wrote:
> > On 12/01/2016 11:43 PM, Jakub Jelinek wrote:
> > >
> > >so we'd need to slow shallow_copy_rtx down by adding switch (code)
> > >in there or something similar.
> > 
> > Is there reason to assume it's time-critical? IMO eliminating the potential
> > for error by not having callers need to do this outweighs that concern.
> 
> Well, it isn't an error not to clear it, just missed optimization if some caller
> cares, and most of the shallow_copy_rtx users really don't care, e.g.
> config/i386/i386.md and config/i386/i386.c callers.  cfgexpand.c doesn't
> either, at that point the used bit isn't set, the calls in cselib.c don't care
> either.  In emit-rtl.c one place explicitly sets used flag afterwards, another
> clears it (that one could be removed if the logic is moved down).  The rs6000.c
> use indeed could get rid of the explicit used bit clearing.  Most of the
> other shallow_copy_rtx callers in the middle-end just don't care.
> I think it is really just simplify_replace_fn_rtx where (at least in the
> rs6000 case) it is useful to clear it, similar trick is not used in many
> places after initial unsharing during expansion before which the bit should
> be clear on everything shareable, I saw it in ifcvt.c (set_used_flags
> + some copying + unshare_*_chain afterwards).
> After expansion when everything is unshared, gradually the used bits are no
> longer relevant, they are set on stuff that has been originally unshared and
> clear on newly added rtxes.  Then the reload/postreload
> unshare_all_rtx_again clears it and unshares the shared stuff.
> So it is really just about spots that do the trick of set_used_flags,
> call some functions where clearing of used bit on new stuff is important,
> and finally call copy_rtx_if_shared.

Anyway, here is a patch that changes shallow_copy_rtx,
bootstrapped/regtested on x86_64-linux and i686-linux.  I believe only the
callers in the patch of the function actually care about used being 0 though
(including the simplify-rtx.c hunks).

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

	PR target/78614
	* rtl.c (copy_rtx): Don't clear used flag here.
	(shallow_copy_rtx_stat): Clear used flag here unless code the rtx
	is shareable.
	* simplify-rtx.c (simplify_replace_fn_rtx): When copying rtx with
	'E' in format, copy all vectors.
	* emit-rtl.c (copy_insn_1): Don't clear used flag here.
	* valtrack.c (cleanup_auto_inc_dec): Likewise.
	* config/rs6000/rs6000.c (rs6000_frame_related): Likewise.

--- gcc/rtl.c.jj	2016-10-31 13:28:12.000000000 +0100
+++ gcc/rtl.c	2016-12-02 11:01:12.557553040 +0100
@@ -318,10 +318,6 @@ copy_rtx (rtx orig)
      us to explicitly document why we are *not* copying a flag.  */
   copy = shallow_copy_rtx (orig);
 
-  /* We do not copy the USED flag, which is used as a mark bit during
-     walks over the RTL.  */
-  RTX_FLAG (copy, used) = 0;
-
   format_ptr = GET_RTX_FORMAT (GET_CODE (copy));
 
   for (i = 0; i < GET_RTX_LENGTH (GET_CODE (copy)); i++)
@@ -367,7 +363,29 @@ shallow_copy_rtx_stat (const_rtx orig ME
 {
   const unsigned int size = rtx_size (orig);
   rtx const copy = ggc_alloc_rtx_def_stat (size PASS_MEM_STAT);
-  return (rtx) memcpy (copy, orig, size);
+  memcpy (copy, orig, size);
+  switch (GET_CODE (orig))
+    {
+      /* RTX codes copy_rtx_if_shared_1 considers are shareable,
+	 the used flag is often used for other purposes.  */
+    case REG:
+    case DEBUG_EXPR:
+    case VALUE:
+    CASE_CONST_ANY:
+    case SYMBOL_REF:
+    case CODE_LABEL:
+    case PC:
+    case CC0:
+    case RETURN:
+    case SIMPLE_RETURN:
+    case SCRATCH:
+      break;
+    default:
+      /* For all other RTXes clear the used flag on the copy.  */
+      RTX_FLAG (copy, used) = 0;
+      break;
+    }
+  return copy;
 }
 \f
 /* Nonzero when we are generating CONCATs.  */
--- gcc/simplify-rtx.c.jj	2016-12-02 00:15:09.200779256 +0100
+++ gcc/simplify-rtx.c	2016-12-02 10:55:24.283989673 +0100
@@ -547,13 +547,19 @@ 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);
+		    /* 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 +572,15 @@ 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);
+		    /* 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));
+		  }
 		XEXP (newx, i) = op;
 	      }
 	  }
--- gcc/emit-rtl.c.jj	2016-12-02 09:43:19.000000000 +0100
+++ gcc/emit-rtl.c	2016-12-02 10:56:37.001061044 +0100
@@ -5552,10 +5552,6 @@ copy_insn_1 (rtx orig)
      us to explicitly document why we are *not* copying a flag.  */
   copy = shallow_copy_rtx (orig);
 
-  /* We do not copy the USED flag, which is used as a mark bit during
-     walks over the RTL.  */
-  RTX_FLAG (copy, used) = 0;
-
   /* We do not copy JUMP, CALL, or FRAME_RELATED for INSNs.  */
   if (INSN_P (orig))
     {
--- gcc/valtrack.c.jj	2016-10-31 13:28:06.000000000 +0100
+++ gcc/valtrack.c	2016-12-02 11:01:44.690144705 +0100
@@ -119,10 +119,6 @@ cleanup_auto_inc_dec (rtx src, machine_m
      us to explicitly document why we are *not* copying a flag.  */
   x = shallow_copy_rtx (x);
 
-  /* We do not copy the USED flag, which is used as a mark bit during
-     walks over the RTL.  */
-  RTX_FLAG (x, used) = 0;
-
   /* We do not copy FRAME_RELATED for INSNs.  */
   if (INSN_P (x))
     RTX_FLAG (x, frame_related) = 0;
--- gcc/config/rs6000/rs6000.c.jj	2016-12-01 08:56:27.137105707 +0100
+++ gcc/config/rs6000/rs6000.c	2016-12-02 11:02:14.796762115 +0100
@@ -27186,7 +27186,6 @@ rs6000_frame_related (rtx_insn *insn, rt
     {
       pat = shallow_copy_rtx (pat);
       XVEC (pat, 0) = shallow_copy_rtvec (XVEC (pat, 0));
-      RTX_FLAG (pat, used) = 0;
 
       for (int i = 0; i < XVECLEN (pat, 0); i++)
 	if (GET_CODE (XVECEXP (pat, 0, i)) == SET)


	Jakub

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

[Bernd Schmidt]

On 12/02/2016 03:12 PM, Jakub Jelinek wrote:
> --- gcc/rtl.c.jj	2016-10-31 13:28:12.000000000 +0100
> +++ gcc/rtl.c	2016-12-02 11:01:12.557553040 +0100
> @@ -318,10 +318,6 @@ copy_rtx (rtx orig)
>       us to explicitly document why we are *not* copying a flag.  */
>    copy = shallow_copy_rtx (orig);
>
> -  /* We do not copy the USED flag, which is used as a mark bit during
> -     walks over the RTL.  */
> -  RTX_FLAG (copy, used) = 0;
> -
>    format_ptr = GET_RTX_FORMAT (GET_CODE (copy));
>
>    for (i = 0; i < GET_RTX_LENGTH (GET_CODE (copy)); i++)
> @@ -367,7 +363,29 @@ shallow_copy_rtx_stat (const_rtx orig ME
>  {
>    const unsigned int size = rtx_size (orig);
>    rtx const copy = ggc_alloc_rtx_def_stat (size PASS_MEM_STAT);
> -  return (rtx) memcpy (copy, orig, size);
> +  memcpy (copy, orig, size);
> +  switch (GET_CODE (orig))
> +    {
> +      /* RTX codes copy_rtx_if_shared_1 considers are shareable,
> +	 the used flag is often used for other purposes.  */
> +    case REG:
> +    case DEBUG_EXPR:
> +    case VALUE:
> +    CASE_CONST_ANY:
> +    case SYMBOL_REF:
> +    case CODE_LABEL:
> +    case PC:
> +    case CC0:
> +    case RETURN:
> +    case SIMPLE_RETURN:
> +    case SCRATCH:
> +      break;
> +    default:
> +      /* For all other RTXes clear the used flag on the copy.  */
> +      RTX_FLAG (copy, used) = 0;
> +      break;
> +    }
> +  return copy;
>  }

I like this a lot better. Of course now that it's spelled out it seems 
like several of these (PC, CC0, RETURN, maybe SCRATCH) should never be 
passed to shallow_copy_rtx and maybe a checking_assert to that effect 
might be in order. This part is OK.

>  /* Nonzero when we are generating CONCATs.  */
> --- gcc/simplify-rtx.c.jj	2016-12-02 00:15:09.200779256 +0100
> +++ gcc/simplify-rtx.c	2016-12-02 10:55:24.283989673 +0100
> @@ -547,13 +547,19 @@ 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);
> +		    /* 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 +572,15 @@ 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);
> +		    /* 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));
> +		  }
>  		XEXP (newx, i) = op;
>  	      }
>  	  }

After looking at it more, I feel that here as well it seems strange for 
simplify_replace_fn_rtx to have knowledge about these issues. Doesn't 
this belong in shallow_copy_rtx as well?

> --- gcc/emit-rtl.c.jj	2016-12-02 09:43:19.000000000 +0100
> +++ gcc/emit-rtl.c	2016-12-02 10:56:37.001061044 +0100
> @@ -5552,10 +5552,6 @@ copy_insn_1 (rtx orig)
>       us to explicitly document why we are *not* copying a flag.  */
>    copy = shallow_copy_rtx (orig);
>
> -  /* We do not copy the USED flag, which is used as a mark bit during
> -     walks over the RTL.  */
> -  RTX_FLAG (copy, used) = 0;
> -
>    /* We do not copy JUMP, CALL, or FRAME_RELATED for INSNs.  */
>    if (INSN_P (orig))
>      {
> --- gcc/valtrack.c.jj	2016-10-31 13:28:06.000000000 +0100
> +++ gcc/valtrack.c	2016-12-02 11:01:44.690144705 +0100
> @@ -119,10 +119,6 @@ cleanup_auto_inc_dec (rtx src, machine_m
>       us to explicitly document why we are *not* copying a flag.  */
>    x = shallow_copy_rtx (x);
>
> -  /* We do not copy the USED flag, which is used as a mark bit during
> -     walks over the RTL.  */
> -  RTX_FLAG (x, used) = 0;
> -
>    /* We do not copy FRAME_RELATED for INSNs.  */
>    if (INSN_P (x))
>      RTX_FLAG (x, frame_related) = 0;
> --- gcc/config/rs6000/rs6000.c.jj	2016-12-01 08:56:27.137105707 +0100
> +++ gcc/config/rs6000/rs6000.c	2016-12-02 11:02:14.796762115 +0100
> @@ -27186,7 +27186,6 @@ rs6000_frame_related (rtx_insn *insn, rt
>      {
>        pat = shallow_copy_rtx (pat);
>        XVEC (pat, 0) = shallow_copy_rtvec (XVEC (pat, 0));
> -      RTX_FLAG (pat, used) = 0;
>
>        for (int i = 0; i < XVECLEN (pat, 0); i++)
>  	if (GET_CODE (XVECEXP (pat, 0, i)) == SET)

These are also ok along with the first part - to me these changes 
suggest we're on the right track.


Bernd

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

[Jakub Jelinek]

On Fri, Dec 02, 2016 at 03:51:52PM +0100, Bernd Schmidt wrote:
> I like this a lot better. Of course now that it's spelled out it seems like
> several of these (PC, CC0, RETURN, maybe SCRATCH) should never be passed to
> shallow_copy_rtx and maybe a checking_assert to that effect might be in
> order. This part is OK.

Ok, I've committed the parts except simplify-rtx.c.

> After looking at it more, I feel that here as well it seems strange for
> simplify_replace_fn_rtx to have knowledge about these issues. Doesn't this
> belong in shallow_copy_rtx as well?

Started working on that, but it seems e.g. copy_insn_1 doesn't want this
behavior, it has a complex system of sharing some of the subvectors
of ASM_OPERANDS.

	Jakub