* [PATCH] Don't clobber dominator info in the combiner (PR target/77526)
@ 2016-09-15 23:20 Jakub Jelinek
2016-09-15 23:59 ` Segher Boessenkool
2016-09-16 7:05 ` Richard Biener
0 siblings, 2 replies; 5+ messages in thread
From: Jakub Jelinek @ 2016-09-15 23:20 UTC (permalink / raw)
To: Richard Biener, Segher Boessenkool; +Cc: gcc-patches
Hi!
As mentioned in the PR, combiner sometimes calls
purge_all_dead_edges or purge_dead_edges that can invalidate the dominator
info if it is computed. Other passes like CSE in that case free the
dominance info, this patch does the same in the combiner.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-09-16 Jakub Jelinek <jakub@redhat.com>
PR target/77526
* combine.c (rest_of_handle_combine): If any edges have been purged,
free dominators if available.
* gcc.target/i386/pr77526.c: New test.
--- gcc/combine.c.jj 2016-08-12 17:33:46.000000000 +0200
+++ gcc/combine.c 2016-09-15 16:12:26.154982064 +0200
@@ -14393,6 +14393,8 @@ rest_of_handle_combine (void)
instructions. */
if (rebuild_jump_labels_after_combine)
{
+ if (dom_info_available_p (CDI_DOMINATORS))
+ free_dominance_info (CDI_DOMINATORS);
timevar_push (TV_JUMP);
rebuild_jump_labels (get_insns ());
cleanup_cfg (0);
--- gcc/testsuite/gcc.target/i386/pr77526.c.jj 2016-09-15 16:13:37.149105476 +0200
+++ gcc/testsuite/gcc.target/i386/pr77526.c 2016-09-15 16:13:13.000000000 +0200
@@ -0,0 +1,13 @@
+/* PR target/77526 */
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-Os -fno-forward-propagate -fno-gcse -fno-rerun-cse-after-loop -mstringop-strategy=byte_loop -Wno-psabi" } */
+
+typedef char U __attribute__((vector_size(64)));
+typedef __int128 V __attribute__((vector_size(64)));
+
+V
+foo (int a, int b, __int128 c, U u)
+{
+ u = (u >> (u & 7)) | (u << -(u & 7));
+ return a + b + c + (V)u;
+}
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Don't clobber dominator info in the combiner (PR target/77526)
2016-09-15 23:20 [PATCH] Don't clobber dominator info in the combiner (PR target/77526) Jakub Jelinek
@ 2016-09-15 23:59 ` Segher Boessenkool
2016-09-16 0:20 ` Jakub Jelinek
2016-09-16 7:05 ` Richard Biener
1 sibling, 1 reply; 5+ messages in thread
From: Segher Boessenkool @ 2016-09-15 23:59 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches
Hi Jakub,
On Fri, Sep 16, 2016 at 12:50:44AM +0200, Jakub Jelinek wrote:
> As mentioned in the PR, combiner sometimes calls
> purge_all_dead_edges or purge_dead_edges that can invalidate the dominator
> info if it is computed. Other passes like CSE in that case free the
> dominance info, this patch does the same in the combiner.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Okay, but does it need the same for the post-dominators? Oh, those
never exist longer than a single pass?
Thanks,
Segher
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Don't clobber dominator info in the combiner (PR target/77526)
2016-09-15 23:59 ` Segher Boessenkool
@ 2016-09-16 0:20 ` Jakub Jelinek
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Jelinek @ 2016-09-16 0:20 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Richard Biener, gcc-patches
On Thu, Sep 15, 2016 at 06:20:11PM -0500, Segher Boessenkool wrote:
> On Fri, Sep 16, 2016 at 12:50:44AM +0200, Jakub Jelinek wrote:
> > As mentioned in the PR, combiner sometimes calls
> > purge_all_dead_edges or purge_dead_edges that can invalidate the dominator
> > info if it is computed. Other passes like CSE in that case free the
> > dominance info, this patch does the same in the combiner.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Okay, but does it need the same for the post-dominators? Oh, those
> never exist longer than a single pass?
Yeah, indeed, passes that compute post-dominators are required to free them
as well, and combine doesn't.
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Don't clobber dominator info in the combiner (PR target/77526)
2016-09-16 7:05 ` Richard Biener
@ 2016-09-16 7:05 ` Jakub Jelinek
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Jelinek @ 2016-09-16 7:05 UTC (permalink / raw)
To: Richard Biener; +Cc: Segher Boessenkool, gcc-patches
On Fri, Sep 16, 2016 at 08:47:28AM +0200, Richard Biener wrote:
> On Fri, 16 Sep 2016, Jakub Jelinek wrote:
>
> > Hi!
> >
> > As mentioned in the PR, combiner sometimes calls
> > purge_all_dead_edges or purge_dead_edges that can invalidate the dominator
> > info if it is computed. Other passes like CSE in that case free the
> > dominance info, this patch does the same in the combiner.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Ok. (I wonder if/why cleanup_cfg (0) doesn't do this then?)
cleanup_cfg only frees it in some cases if it does some kind of changes:
/* ??? We probably do this way too often. */
if (current_loops
&& (changed
|| (mode & CLEANUP_CFG_CHANGED)))
{
...
calculate_dominance_info (CDI_DOMINATORS);
But here the dominator info gets out of sync earlier, in particular on this
testcase in the
new_direct_jump_p |= purge_all_dead_edges ();
call.
Perhaps ideally we should teach all these functions to update the dominator
info, but I'm afraid it is a lot of work.
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Don't clobber dominator info in the combiner (PR target/77526)
2016-09-15 23:20 [PATCH] Don't clobber dominator info in the combiner (PR target/77526) Jakub Jelinek
2016-09-15 23:59 ` Segher Boessenkool
@ 2016-09-16 7:05 ` Richard Biener
2016-09-16 7:05 ` Jakub Jelinek
1 sibling, 1 reply; 5+ messages in thread
From: Richard Biener @ 2016-09-16 7:05 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Segher Boessenkool, gcc-patches
On Fri, 16 Sep 2016, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, combiner sometimes calls
> purge_all_dead_edges or purge_dead_edges that can invalidate the dominator
> info if it is computed. Other passes like CSE in that case free the
> dominance info, this patch does the same in the combiner.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok. (I wonder if/why cleanup_cfg (0) doesn't do this then?)
Thanks,
Richard.
> 2016-09-16 Jakub Jelinek <jakub@redhat.com>
>
> PR target/77526
> * combine.c (rest_of_handle_combine): If any edges have been purged,
> free dominators if available.
>
> * gcc.target/i386/pr77526.c: New test.
>
> --- gcc/combine.c.jj 2016-08-12 17:33:46.000000000 +0200
> +++ gcc/combine.c 2016-09-15 16:12:26.154982064 +0200
> @@ -14393,6 +14393,8 @@ rest_of_handle_combine (void)
> instructions. */
> if (rebuild_jump_labels_after_combine)
> {
> + if (dom_info_available_p (CDI_DOMINATORS))
> + free_dominance_info (CDI_DOMINATORS);
> timevar_push (TV_JUMP);
> rebuild_jump_labels (get_insns ());
> cleanup_cfg (0);
> --- gcc/testsuite/gcc.target/i386/pr77526.c.jj 2016-09-15 16:13:37.149105476 +0200
> +++ gcc/testsuite/gcc.target/i386/pr77526.c 2016-09-15 16:13:13.000000000 +0200
> @@ -0,0 +1,13 @@
> +/* PR target/77526 */
> +/* { dg-do compile { target int128 } } */
> +/* { dg-options "-Os -fno-forward-propagate -fno-gcse -fno-rerun-cse-after-loop -mstringop-strategy=byte_loop -Wno-psabi" } */
> +
> +typedef char U __attribute__((vector_size(64)));
> +typedef __int128 V __attribute__((vector_size(64)));
> +
> +V
> +foo (int a, int b, __int128 c, U u)
> +{
> + u = (u >> (u & 7)) | (u << -(u & 7));
> + return a + b + c + (V)u;
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-16 7:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-15 23:20 [PATCH] Don't clobber dominator info in the combiner (PR target/77526) Jakub Jelinek
2016-09-15 23:59 ` Segher Boessenkool
2016-09-16 0:20 ` Jakub Jelinek
2016-09-16 7:05 ` Richard Biener
2016-09-16 7: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).