* [PATCH] Unset used bit in simplify_replace_* on newly copied rtxs (PR target/78614) @ 2016-11-30 22:11 Jakub Jelinek 2016-12-01 12:19 ` Bernd Schmidt 0 siblings, 1 reply; 8+ messages in thread From: Jakub Jelinek @ 2016-11-30 22:11 UTC (permalink / raw) To: Richard Biener, Jeff Law, Bernd Schmidt, Eric Botcazou; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Unset used bit in simplify_replace_* on newly copied rtxs (PR target/78614) 2016-11-30 22:11 [PATCH] Unset used bit in simplify_replace_* on newly copied rtxs (PR target/78614) Jakub Jelinek @ 2016-12-01 12:19 ` Bernd Schmidt 2016-12-01 22:43 ` Jakub Jelinek 0 siblings, 1 reply; 8+ messages in thread From: Bernd Schmidt @ 2016-12-01 12:19 UTC (permalink / raw) To: Jakub Jelinek, Richard Biener, Jeff Law, Eric Botcazou; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Unset used bit in simplify_replace_* on newly copied rtxs (PR target/78614) 2016-12-01 12:19 ` Bernd Schmidt @ 2016-12-01 22:43 ` Jakub Jelinek 2016-12-01 22:48 ` Bernd Schmidt 0 siblings, 1 reply; 8+ messages in thread From: Jakub Jelinek @ 2016-12-01 22:43 UTC (permalink / raw) To: Bernd Schmidt; +Cc: Richard Biener, Jeff Law, Eric Botcazou, gcc-patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Unset used bit in simplify_replace_* on newly copied rtxs (PR target/78614) 2016-12-01 22:43 ` Jakub Jelinek @ 2016-12-01 22:48 ` Bernd Schmidt 2016-12-01 23:34 ` Jakub Jelinek 0 siblings, 1 reply; 8+ messages in thread From: Bernd Schmidt @ 2016-12-01 22:48 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Richard Biener, Jeff Law, Eric Botcazou, gcc-patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Unset used bit in simplify_replace_* on newly copied rtxs (PR target/78614) 2016-12-01 22:48 ` Bernd Schmidt @ 2016-12-01 23:34 ` Jakub Jelinek 2016-12-02 14:13 ` Jakub Jelinek 0 siblings, 1 reply; 8+ messages in thread From: Jakub Jelinek @ 2016-12-01 23:34 UTC (permalink / raw) To: Bernd Schmidt; +Cc: Richard Biener, Jeff Law, Eric Botcazou, gcc-patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Unset used bit in simplify_replace_* on newly copied rtxs (PR target/78614) 2016-12-01 23:34 ` Jakub Jelinek @ 2016-12-02 14:13 ` Jakub Jelinek 2016-12-02 14:51 ` Bernd Schmidt 0 siblings, 1 reply; 8+ messages in thread From: Jakub Jelinek @ 2016-12-02 14:13 UTC (permalink / raw) To: Bernd Schmidt; +Cc: Richard Biener, Jeff Law, Eric Botcazou, gcc-patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Unset used bit in simplify_replace_* on newly copied rtxs (PR target/78614) 2016-12-02 14:13 ` Jakub Jelinek @ 2016-12-02 14:51 ` Bernd Schmidt 2016-12-02 16:05 ` Jakub Jelinek 0 siblings, 1 reply; 8+ messages in thread From: Bernd Schmidt @ 2016-12-02 14:51 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Richard Biener, Jeff Law, Eric Botcazou, gcc-patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Unset used bit in simplify_replace_* on newly copied rtxs (PR target/78614) 2016-12-02 14:51 ` Bernd Schmidt @ 2016-12-02 16:05 ` Jakub Jelinek 0 siblings, 0 replies; 8+ messages in thread From: Jakub Jelinek @ 2016-12-02 16:05 UTC (permalink / raw) To: Bernd Schmidt; +Cc: Richard Biener, Jeff Law, Eric Botcazou, gcc-patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-12-02 16:05 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-30 22:11 [PATCH] Unset used bit in simplify_replace_* on newly copied rtxs (PR target/78614) Jakub Jelinek 2016-12-01 12:19 ` Bernd Schmidt 2016-12-01 22:43 ` Jakub Jelinek 2016-12-01 22:48 ` Bernd Schmidt 2016-12-01 23:34 ` Jakub Jelinek 2016-12-02 14:13 ` Jakub Jelinek 2016-12-02 14:51 ` Bernd Schmidt 2016-12-02 16:05 ` Jakub Jelinek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).