This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Unset used bit in simplify_replace_* on newly copied rtxs (PR target/78614)
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