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 <ja...@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;
}
/* 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
