* [PATCH] Fix rtl sharing bug in rs6000_frame_related (PR target/78614)
@ 2016-11-30 22:27 Jakub Jelinek
2016-11-30 23:57 ` Alan Modra
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Jakub Jelinek @ 2016-11-30 22:27 UTC (permalink / raw)
To: David Edelsohn, Segher Boessenkool
Cc: gcc-patches, Markus Trippelsdorf, Alan Modra
Hi!
As mentioned in the PR, the rs6000_frame_related rewrite broke rtl sharing
whose --enable-checking=rtl verification has been broken for the last 3.5
years until today.
The problem is that simplify_replace_rtx doesn't unshare everything, only
the minimum needed to replace what is needed without changing original
expression. But as we replace something from PATTERN of the insn and
want to stick that into REG_FRAME_RELATED_EXPR note in the insn stream
next to the PATTERN, there can't be any sharing except for shareable
rtxes.
The following patch is the more memory friendly version, it marks as used
everything in the original PATTERN, then does all those
simplify_replace_rtxs that usually (with the patch I've just posted more
often) when unsharing/creating new rtxes keep the used bit unset, and
finally does copy_rtx_if_shared which should unshare only whatever
simplify_replace_rtx has not unshared.
The last hunk just removes unnecessary condition, if the condition is not
true, we return from the function already earlier and don't do any
replacements.
Markus said he has bootstrapped this patch with rtl checking on powerpc64.
Ok for trunk?
2016-11-30 Jakub Jelinek <jakub@redhat.com>
PR target/78614
* config/rs6000/rs6000.c (rs6000_frame_related): Call
set_used_flags (pat) before any simplifications. Clear used flag on
PARALLEL copy. Don't guard add_reg_note call. Call
copy_rtx_if_shared on pat before storing it into
REG_FRAME_RELATED_EXPR.
--- gcc/config/rs6000/rs6000.c.jj 2016-11-29 07:31:02.000000000 +0100
+++ gcc/config/rs6000/rs6000.c 2016-11-30 17:10:40.805842306 +0100
@@ -27170,6 +27170,7 @@ rs6000_frame_related (rtx_insn *insn, rt
Call simplify_replace_rtx on the SETs rather than the whole insn
so as to leave the other stuff alone (for example USE of r12). */
+ set_used_flags (pat);
if (GET_CODE (pat) == SET)
{
if (repl)
@@ -27181,6 +27182,7 @@ 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)
@@ -27203,8 +27205,7 @@ rs6000_frame_related (rtx_insn *insn, rt
gcc_unreachable ();
RTX_FRAME_RELATED_P (insn) = 1;
- if (repl || reg2)
- add_reg_note (insn, REG_FRAME_RELATED_EXPR, pat);
+ add_reg_note (insn, REG_FRAME_RELATED_EXPR, copy_rtx_if_shared (pat));
return insn;
}
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix rtl sharing bug in rs6000_frame_related (PR target/78614)
2016-11-30 22:27 [PATCH] Fix rtl sharing bug in rs6000_frame_related (PR target/78614) Jakub Jelinek
@ 2016-11-30 23:57 ` Alan Modra
2016-12-01 1:06 ` Segher Boessenkool
2016-12-01 2:06 ` Alan Modra
2 siblings, 0 replies; 5+ messages in thread
From: Alan Modra @ 2016-11-30 23:57 UTC (permalink / raw)
To: Jakub Jelinek
Cc: David Edelsohn, Segher Boessenkool, gcc-patches, Markus Trippelsdorf
On Wed, Nov 30, 2016 at 11:27:40PM +0100, Jakub Jelinek wrote:
> The last hunk just removes unnecessary condition, if the condition is not
> true, we return from the function already earlier and don't do any
> replacements.
Yeah, that was a leftover from an earlier revision of the patch that
removed the "if (!repl && !reg2)" block.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix rtl sharing bug in rs6000_frame_related (PR target/78614)
2016-11-30 22:27 [PATCH] Fix rtl sharing bug in rs6000_frame_related (PR target/78614) Jakub Jelinek
2016-11-30 23:57 ` Alan Modra
@ 2016-12-01 1:06 ` Segher Boessenkool
2016-12-01 2:06 ` Alan Modra
2 siblings, 0 replies; 5+ messages in thread
From: Segher Boessenkool @ 2016-12-01 1:06 UTC (permalink / raw)
To: Jakub Jelinek
Cc: David Edelsohn, gcc-patches, Markus Trippelsdorf, Alan Modra
On Wed, Nov 30, 2016 at 11:27:40PM +0100, Jakub Jelinek wrote:
> As mentioned in the PR, the rs6000_frame_related rewrite broke rtl sharing
> whose --enable-checking=rtl verification has been broken for the last 3.5
> years until today.
Great eh! I am very scared.
> Markus said he has bootstrapped this patch with rtl checking on powerpc64.
>
> Ok for trunk?
Yes please. Thanks,
Segher
> 2016-11-30 Jakub Jelinek <jakub@redhat.com>
>
> PR target/78614
> * config/rs6000/rs6000.c (rs6000_frame_related): Call
> set_used_flags (pat) before any simplifications. Clear used flag on
> PARALLEL copy. Don't guard add_reg_note call. Call
> copy_rtx_if_shared on pat before storing it into
> REG_FRAME_RELATED_EXPR.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix rtl sharing bug in rs6000_frame_related (PR target/78614)
2016-11-30 22:27 [PATCH] Fix rtl sharing bug in rs6000_frame_related (PR target/78614) Jakub Jelinek
2016-11-30 23:57 ` Alan Modra
2016-12-01 1:06 ` Segher Boessenkool
@ 2016-12-01 2:06 ` Alan Modra
2016-12-01 3:27 ` Alan Modra
2 siblings, 1 reply; 5+ messages in thread
From: Alan Modra @ 2016-12-01 2:06 UTC (permalink / raw)
To: Jakub Jelinek
Cc: David Edelsohn, Segher Boessenkool, gcc-patches, Markus Trippelsdorf
On Wed, Nov 30, 2016 at 11:27:40PM +0100, Jakub Jelinek wrote:
> Markus said he has bootstrapped this patch with rtl checking on powerpc64.
I repeated the exercise and found a stage1 bootstrap failure due to
invalid rtl sharing on powerpc64le-linux, using
AS="/home/amodra/gnu/bin/as" LD="/home/amodra/gnu/bin/ld" \
~/src/gcc-pr78614/configure \
--build=powerpc64le-linux --prefix=/home/amodra/gnu \
--enable-targets=powerpc64-linux,powerpc-linux,powerpcle-linux --disable-multilib \
--enable-valgrind-annotations \
--disable-nls --with-cpu=power8 --enable-languages=all,go --enable-lto \
--enable-checking=yes,rtl
> 2016-11-30 Jakub Jelinek <jakub@redhat.com>
>
> PR target/78614
> * config/rs6000/rs6000.c (rs6000_frame_related): Call
> set_used_flags (pat) before any simplifications. Clear used flag on
> PARALLEL copy. Don't guard add_reg_note call. Call
> copy_rtx_if_shared on pat before storing it into
> REG_FRAME_RELATED_EXPR.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix rtl sharing bug in rs6000_frame_related (PR target/78614)
2016-12-01 2:06 ` Alan Modra
@ 2016-12-01 3:27 ` Alan Modra
0 siblings, 0 replies; 5+ messages in thread
From: Alan Modra @ 2016-12-01 3:27 UTC (permalink / raw)
To: Jakub Jelinek
Cc: David Edelsohn, Segher Boessenkool, gcc-patches, Markus Trippelsdorf
On Thu, Dec 01, 2016 at 12:36:49PM +1030, Alan Modra wrote:
> On Wed, Nov 30, 2016 at 11:27:40PM +0100, Jakub Jelinek wrote:
> > Markus said he has bootstrapped this patch with rtl checking on powerpc64.
>
> I repeated the exercise and found a stage1 bootstrap failure due to
> invalid rtl sharing on powerpc64le-linux, using
Apologies for the noise. I wasn't testing your patch.. Checking
again with it applied properly, and I'm past stage1 where the failures
happened.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-01 3:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-30 22:27 [PATCH] Fix rtl sharing bug in rs6000_frame_related (PR target/78614) Jakub Jelinek
2016-11-30 23:57 ` Alan Modra
2016-12-01 1:06 ` Segher Boessenkool
2016-12-01 2:06 ` Alan Modra
2016-12-01 3:27 ` Alan Modra
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).