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

Reply via email to