* [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
@ 2019-05-24 7:48 Martin Liška
2019-05-24 8:08 ` Jakub Jelinek
0 siblings, 1 reply; 11+ messages in thread
From: Martin Liška @ 2019-05-24 7:48 UTC (permalink / raw)
To: gcc-patches; +Cc: Jakub Jelinek, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 903 bytes --]
Hi.
The patch is about more fine comparison of loop fields that
are mentioned in the PR.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
gcc/ChangeLog:
2019-05-23 Martin Liska <mliska@suse.cz>
PR ipa/90555
* ipa-icf-gimple.c (func_checker::compare_loops): New function.
* ipa-icf-gimple.h (func_checker::compare_loops): Likewise.
* ipa-icf.c (sem_function::equals_private): Use compare_loops.
gcc/testsuite/ChangeLog:
2019-05-23 Martin Liska <mliska@suse.cz>
PR ipa/90555
* gcc.dg/ipa/pr90555.c: New test.
---
gcc/ipa-icf-gimple.c | 40 ++++++++++++++++++
gcc/ipa-icf-gimple.h | 3 ++
gcc/ipa-icf.c | 3 ++
gcc/testsuite/gcc.dg/ipa/pr90555.c | 67 ++++++++++++++++++++++++++++++
4 files changed, 113 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/ipa/pr90555.c
[-- Attachment #2: 0001-Handle-loop-fields-in-IPA-ICF-PR-ipa-90555.patch --]
[-- Type: text/x-patch, Size: 4250 bytes --]
diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 25284936bc3..4d4e4c7c350 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3. If not see
#include "ipa-utils.h"
#include "tree-eh.h"
#include "builtins.h"
+#include "cfgloop.h"
#include "ipa-icf-gimple.h"
@@ -605,6 +606,45 @@ func_checker::compare_variable_decl (tree t1, tree t2)
return return_with_debug (ret);
}
+/* Compare significant loop information. */
+bool
+func_checker::compare_loops (void)
+{
+ struct loop *loop;
+
+ auto_vec <struct loop *> loops1;
+ auto_vec <struct loop *> loops2;
+
+ FOR_EACH_LOOP_FN (DECL_STRUCT_FUNCTION (m_source_func_decl), loop, 0)
+ loops1.safe_push (loop);
+
+ FOR_EACH_LOOP_FN (DECL_STRUCT_FUNCTION (m_target_func_decl), loop, 0)
+ loops2.safe_push (loop);
+
+ gcc_assert (loops1.length () == loops2.length ());
+
+ for (unsigned i = 0; i < loops1.length (); i++)
+ {
+ struct loop *l1 = loops1[i];
+ struct loop *l2 = loops2[i];
+ if (l1->simdlen != l2->simdlen)
+ return return_false_with_msg ("simdlen");
+ if (l1->safelen != l2->safelen)
+ return return_false_with_msg ("safelen");
+ if (l1->can_be_parallel != l2->can_be_parallel)
+ return return_false_with_msg ("can_be_parallel");
+ if (l1->dont_vectorize != l2->dont_vectorize)
+ return return_false_with_msg ("dont_vectorize");
+ if (l1->force_vectorize != l2->force_vectorize)
+ return return_false_with_msg ("force_vectorize");
+ if (l1->unroll != l2->unroll)
+ return return_false_with_msg ("unroll");
+ if (!compare_variable_decl (l1->simduid, l2->simduid))
+ return return_false_with_msg ("simduid");
+ }
+
+ return true;
+}
/* Function visits all gimple labels and creates corresponding
mapping between basic blocks and labels. */
diff --git a/gcc/ipa-icf-gimple.h b/gcc/ipa-icf-gimple.h
index 0035db32e66..0fb0744d912 100644
--- a/gcc/ipa-icf-gimple.h
+++ b/gcc/ipa-icf-gimple.h
@@ -226,6 +226,9 @@ public:
/* Verifies that trees T1 and T2 do correspond. */
bool compare_variable_decl (tree t1, tree t2);
+ /* Compare significant loop information. */
+ bool compare_loops (void);
+
/* Return true if types are compatible for polymorphic call analysis.
COMPARE_PTR indicates if polymorphic type comparsion should be
done for pointers, too. */
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 074181491da..ba5121f2198 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -926,6 +926,9 @@ sem_function::equals_private (sem_item *item)
if (!compare_phi_node (bb_sorted[i]->bb, m_compared_func->bb_sorted[i]->bb))
return return_false_with_msg ("PHI node comparison returns false");
+ if (!m_checker->compare_loops ())
+ return return_false_with_msg ("loops are different");
+
return result;
}
diff --git a/gcc/testsuite/gcc.dg/ipa/pr90555.c b/gcc/testsuite/gcc.dg/ipa/pr90555.c
new file mode 100644
index 00000000000..a9c751cd63c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr90555.c
@@ -0,0 +1,67 @@
+/* { dg-do compile } */
+/* { dg-options "-fopenmp-simd -O2 -mavx512f -fdump-ipa-icf" } */
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+
+#define N 1024
+int a[N];
+
+void
+test_simdlen1 (void)
+{
+ int i;
+ #pragma omp simd simdlen (4)
+ for (i = 0; i < N; ++i)
+ a[i] = a[i] + 1;
+}
+
+void
+test_simdlen2 (void)
+{
+ int i;
+ #pragma omp simd simdlen (8)
+ for (i = 0; i < N; ++i)
+ a[i] = a[i] + 1;
+}
+
+void
+test_safelen1 (void)
+{
+ int i;
+ #pragma omp simd safelen (4)
+ for (i = 0; i < N; ++i)
+ a[i] = a[i] + 1;
+}
+
+void
+test_safelen2 (void)
+{
+ int i;
+ #pragma omp simd safelen (8)
+ for (i = 0; i < N; ++i)
+ a[i] = a[i] + 1;
+}
+
+int d[1024];
+
+int
+test_simduid1 (int j, int b)
+{
+ int l, c = 0;
+#pragma omp simd reduction(+: c)
+ for (l = 0; l < b; ++l)
+ c += d[j + l];
+ return c;
+}
+
+int
+test_simduid2 (int j, int b)
+{
+ int l, c2 = 0;
+#pragma omp simd reduction(+: c2)
+ for (l = 0; l < b; ++l)
+ c2 += d[j + l];
+ return c2;
+}
+
+/* { dg-final { scan-ipa-dump "Semantic equality hit:test_simduid1->test_simduid2" "icf" } } */
+/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
2019-05-24 7:48 [PATCH] Handle loop fields in IPA ICF (PR ipa/90555) Martin Liška
@ 2019-05-24 8:08 ` Jakub Jelinek
2019-05-24 9:38 ` Richard Biener
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2019-05-24 8:08 UTC (permalink / raw)
To: Martin Liška; +Cc: gcc-patches, Jan Hubicka
On Fri, May 24, 2019 at 09:48:03AM +0200, Martin Liška wrote:
> gcc/ChangeLog:
>
> 2019-05-23 Martin Liska <mliska@suse.cz>
>
> PR ipa/90555
> * ipa-icf-gimple.c (func_checker::compare_loops): New function.
> * ipa-icf-gimple.h (func_checker::compare_loops): Likewise.
> * ipa-icf.c (sem_function::equals_private): Use compare_loops.
>
> gcc/testsuite/ChangeLog:
>
> 2019-05-23 Martin Liska <mliska@suse.cz>
>
> PR ipa/90555
> * gcc.dg/ipa/pr90555.c: New test.
> @@ -605,6 +606,45 @@ func_checker::compare_variable_decl (tree t1, tree t2)
> return return_with_debug (ret);
> }
>
> +/* Compare significant loop information. */
> +bool
> +func_checker::compare_loops (void)
> +{
> + struct loop *loop;
> +
> + auto_vec <struct loop *> loops1;
> + auto_vec <struct loop *> loops2;
> +
> + FOR_EACH_LOOP_FN (DECL_STRUCT_FUNCTION (m_source_func_decl), loop, 0)
> + loops1.safe_push (loop);
> +
> + FOR_EACH_LOOP_FN (DECL_STRUCT_FUNCTION (m_target_func_decl), loop, 0)
> + loops2.safe_push (loop);
> +
> + gcc_assert (loops1.length () == loops2.length ());
> +
> + for (unsigned i = 0; i < loops1.length (); i++)
I wonder how likely/unlikely it is that the loops will be ordered the same
by the iterators. If they are the same always, then perhaps it would be
better to avoid the temporary vectors, and just do one FOR_EACH_LOOP_FN
with another manual loop_iterator use in sync with that.
If it isn't guaranteed they are ordered the same, then I think we need to
match the loops by the corresponding loop header and perhaps also verify
loop latch and other basic blocks in the loop?
> + {
> + struct loop *l1 = loops1[i];
> + struct loop *l2 = loops2[i];
> + if (l1->simdlen != l2->simdlen)
> + return return_false_with_msg ("simdlen");
> + if (l1->safelen != l2->safelen)
> + return return_false_with_msg ("safelen");
> + if (l1->can_be_parallel != l2->can_be_parallel)
> + return return_false_with_msg ("can_be_parallel");
> + if (l1->dont_vectorize != l2->dont_vectorize)
> + return return_false_with_msg ("dont_vectorize");
> + if (l1->force_vectorize != l2->force_vectorize)
> + return return_false_with_msg ("force_vectorize");
> + if (l1->unroll != l2->unroll)
> + return return_false_with_msg ("unroll");
> + if (!compare_variable_decl (l1->simduid, l2->simduid))
> + return return_false_with_msg ("simduid");
The above looks reasonable if we are guaranteed we are talking about a loop
corresponding between the two functions.
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/pr90555.c
> @@ -0,0 +1,67 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fopenmp-simd -O2 -mavx512f -fdump-ipa-icf" } */
> +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
Please don't use two dg-do compile directives, drop the first line,
move the third line to first one.
Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
2019-05-24 8:08 ` Jakub Jelinek
@ 2019-05-24 9:38 ` Richard Biener
2019-05-27 12:50 ` Martin Liška
0 siblings, 1 reply; 11+ messages in thread
From: Richard Biener @ 2019-05-24 9:38 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Martin Liška, GCC Patches, Jan Hubicka
On Fri, May 24, 2019 at 10:08 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Fri, May 24, 2019 at 09:48:03AM +0200, Martin Liška wrote:
> > gcc/ChangeLog:
> >
> > 2019-05-23 Martin Liska <mliska@suse.cz>
> >
> > PR ipa/90555
> > * ipa-icf-gimple.c (func_checker::compare_loops): New function.
> > * ipa-icf-gimple.h (func_checker::compare_loops): Likewise.
> > * ipa-icf.c (sem_function::equals_private): Use compare_loops.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2019-05-23 Martin Liska <mliska@suse.cz>
> >
> > PR ipa/90555
> > * gcc.dg/ipa/pr90555.c: New test.
>
> > @@ -605,6 +606,45 @@ func_checker::compare_variable_decl (tree t1, tree t2)
> > return return_with_debug (ret);
> > }
> >
> > +/* Compare significant loop information. */
> > +bool
> > +func_checker::compare_loops (void)
> > +{
> > + struct loop *loop;
> > +
> > + auto_vec <struct loop *> loops1;
> > + auto_vec <struct loop *> loops2;
> > +
> > + FOR_EACH_LOOP_FN (DECL_STRUCT_FUNCTION (m_source_func_decl), loop, 0)
> > + loops1.safe_push (loop);
> > +
> > + FOR_EACH_LOOP_FN (DECL_STRUCT_FUNCTION (m_target_func_decl), loop, 0)
> > + loops2.safe_push (loop);
> > +
> > + gcc_assert (loops1.length () == loops2.length ());
> > +
> > + for (unsigned i = 0; i < loops1.length (); i++)
>
> I wonder how likely/unlikely it is that the loops will be ordered the same
> by the iterators. If they are the same always, then perhaps it would be
> better to avoid the temporary vectors, and just do one FOR_EACH_LOOP_FN
> with another manual loop_iterator use in sync with that.
> If it isn't guaranteed they are ordered the same, then I think we need to
> match the loops by the corresponding loop header and perhaps also verify
> loop latch and other basic blocks in the loop?
In fact I wouldn't even compare loops this way but compare loop
paris when I run across two equal basic-blocks that are loop
headers (bb->loop_father == bb->loop_father->header).
> > + {
> > + struct loop *l1 = loops1[i];
> > + struct loop *l2 = loops2[i];
> > + if (l1->simdlen != l2->simdlen)
> > + return return_false_with_msg ("simdlen");
> > + if (l1->safelen != l2->safelen)
> > + return return_false_with_msg ("safelen");
> > + if (l1->can_be_parallel != l2->can_be_parallel)
> > + return return_false_with_msg ("can_be_parallel");
> > + if (l1->dont_vectorize != l2->dont_vectorize)
> > + return return_false_with_msg ("dont_vectorize");
> > + if (l1->force_vectorize != l2->force_vectorize)
> > + return return_false_with_msg ("force_vectorize");
> > + if (l1->unroll != l2->unroll)
> > + return return_false_with_msg ("unroll");
> > + if (!compare_variable_decl (l1->simduid, l2->simduid))
> > + return return_false_with_msg ("simduid");
>
> The above looks reasonable if we are guaranteed we are talking about a loop
> corresponding between the two functions.
>
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/ipa/pr90555.c
> > @@ -0,0 +1,67 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-fopenmp-simd -O2 -mavx512f -fdump-ipa-icf" } */
> > +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
>
> Please don't use two dg-do compile directives, drop the first line,
> move the third line to first one.
>
> Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
2019-05-24 9:38 ` Richard Biener
@ 2019-05-27 12:50 ` Martin Liška
2019-05-27 13:09 ` Jakub Jelinek
0 siblings, 1 reply; 11+ messages in thread
From: Martin Liška @ 2019-05-27 12:50 UTC (permalink / raw)
To: Richard Biener, Jakub Jelinek; +Cc: GCC Patches, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 3543 bytes --]
On 5/24/19 11:37 AM, Richard Biener wrote:
> On Fri, May 24, 2019 at 10:08 AM Jakub Jelinek <jakub@redhat.com> wrote:
>>
>> On Fri, May 24, 2019 at 09:48:03AM +0200, Martin Liška wrote:
>>> gcc/ChangeLog:
>>>
>>> 2019-05-23 Martin Liska <mliska@suse.cz>
>>>
>>> PR ipa/90555
>>> * ipa-icf-gimple.c (func_checker::compare_loops): New function.
>>> * ipa-icf-gimple.h (func_checker::compare_loops): Likewise.
>>> * ipa-icf.c (sem_function::equals_private): Use compare_loops.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2019-05-23 Martin Liska <mliska@suse.cz>
>>>
>>> PR ipa/90555
>>> * gcc.dg/ipa/pr90555.c: New test.
>>
>>> @@ -605,6 +606,45 @@ func_checker::compare_variable_decl (tree t1, tree t2)
>>> return return_with_debug (ret);
>>> }
>>>
>>> +/* Compare significant loop information. */
>>> +bool
>>> +func_checker::compare_loops (void)
>>> +{
>>> + struct loop *loop;
>>> +
>>> + auto_vec <struct loop *> loops1;
>>> + auto_vec <struct loop *> loops2;
>>> +
>>> + FOR_EACH_LOOP_FN (DECL_STRUCT_FUNCTION (m_source_func_decl), loop, 0)
>>> + loops1.safe_push (loop);
>>> +
>>> + FOR_EACH_LOOP_FN (DECL_STRUCT_FUNCTION (m_target_func_decl), loop, 0)
>>> + loops2.safe_push (loop);
>>> +
>>> + gcc_assert (loops1.length () == loops2.length ());
>>> +
>>> + for (unsigned i = 0; i < loops1.length (); i++)
>>
>> I wonder how likely/unlikely it is that the loops will be ordered the same
>> by the iterators. If they are the same always, then perhaps it would be
>> better to avoid the temporary vectors, and just do one FOR_EACH_LOOP_FN
>> with another manual loop_iterator use in sync with that.
>> If it isn't guaranteed they are ordered the same, then I think we need to
>> match the loops by the corresponding loop header and perhaps also verify
>> loop latch and other basic blocks in the loop?
>
> In fact I wouldn't even compare loops this way but compare loop
> paris when I run across two equal basic-blocks that are loop
> headers (bb->loop_father == bb->loop_father->header).
That's definitely nicer.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
>
>>> + {
>>> + struct loop *l1 = loops1[i];
>>> + struct loop *l2 = loops2[i];
>>> + if (l1->simdlen != l2->simdlen)
>>> + return return_false_with_msg ("simdlen");
>>> + if (l1->safelen != l2->safelen)
>>> + return return_false_with_msg ("safelen");
>>> + if (l1->can_be_parallel != l2->can_be_parallel)
>>> + return return_false_with_msg ("can_be_parallel");
>>> + if (l1->dont_vectorize != l2->dont_vectorize)
>>> + return return_false_with_msg ("dont_vectorize");
>>> + if (l1->force_vectorize != l2->force_vectorize)
>>> + return return_false_with_msg ("force_vectorize");
>>> + if (l1->unroll != l2->unroll)
>>> + return return_false_with_msg ("unroll");
>>> + if (!compare_variable_decl (l1->simduid, l2->simduid))
>>> + return return_false_with_msg ("simduid");
>>
>> The above looks reasonable if we are guaranteed we are talking about a loop
>> corresponding between the two functions.
>>
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/ipa/pr90555.c
>>> @@ -0,0 +1,67 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-fopenmp-simd -O2 -mavx512f -fdump-ipa-icf" } */
>>> +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
>>
>> Please don't use two dg-do compile directives, drop the first line,
>> move the third line to first one.
>>
>> Jakub
[-- Attachment #2: 0001-Handle-loop-fields-in-IPA-ICF-PR-ipa-90555.patch --]
[-- Type: text/x-patch, Size: 4621 bytes --]
From 306cab10e8ef918b25321132aa75d9400f7de247 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Thu, 23 May 2019 12:47:11 +0200
Subject: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
gcc/ChangeLog:
2019-05-23 Martin Liska <mliska@suse.cz>
PR ipa/90555
* ipa-icf-gimple.c (func_checker::compare_loops): New function.
* ipa-icf-gimple.h (func_checker::compare_loops): Likewise.
(func_checker::compare_bb): Call compare_loops.
gcc/testsuite/ChangeLog:
2019-05-23 Martin Liska <mliska@suse.cz>
PR ipa/90555
* gcc.dg/ipa/pr90555.c: New test.
---
gcc/ipa-icf-gimple.c | 31 ++++++++++++++
gcc/ipa-icf-gimple.h | 3 ++
gcc/testsuite/gcc.dg/ipa/pr90555.c | 67 ++++++++++++++++++++++++++++++
3 files changed, 101 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/ipa/pr90555.c
diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 25284936bc3..92b1813ec6c 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3. If not see
#include "ipa-utils.h"
#include "tree-eh.h"
#include "builtins.h"
+#include "cfgloop.h"
#include "ipa-icf-gimple.h"
@@ -605,6 +606,33 @@ func_checker::compare_variable_decl (tree t1, tree t2)
return return_with_debug (ret);
}
+/* Compare loop information for basic blocks BB1 and BB2. */
+
+bool
+func_checker::compare_loops (basic_block bb1, basic_block bb2)
+{
+ if ((bb1->loop_father == NULL) != (bb2->loop_father == NULL))
+ return return_false ();
+
+ struct loop *l1 = bb1->loop_father;
+ struct loop *l2 = bb2->loop_father;
+ if (l1->simdlen != l2->simdlen)
+ return return_false_with_msg ("simdlen");
+ if (l1->safelen != l2->safelen)
+ return return_false_with_msg ("safelen");
+ if (l1->can_be_parallel != l2->can_be_parallel)
+ return return_false_with_msg ("can_be_parallel");
+ if (l1->dont_vectorize != l2->dont_vectorize)
+ return return_false_with_msg ("dont_vectorize");
+ if (l1->force_vectorize != l2->force_vectorize)
+ return return_false_with_msg ("force_vectorize");
+ if (l1->unroll != l2->unroll)
+ return return_false_with_msg ("unroll");
+ if (!compare_variable_decl (l1->simduid, l2->simduid))
+ return return_false_with_msg ("simduid");
+
+ return true;
+}
/* Function visits all gimple labels and creates corresponding
mapping between basic blocks and labels. */
@@ -727,6 +755,9 @@ func_checker::compare_bb (sem_bb *bb1, sem_bb *bb2)
if (!gsi_end_p (gsi2))
return return_false ();
+ if (!compare_loops (bb1->bb, bb2->bb))
+ return return_false ();
+
return true;
}
diff --git a/gcc/ipa-icf-gimple.h b/gcc/ipa-icf-gimple.h
index 0035db32e66..51aadced9ea 100644
--- a/gcc/ipa-icf-gimple.h
+++ b/gcc/ipa-icf-gimple.h
@@ -226,6 +226,9 @@ public:
/* Verifies that trees T1 and T2 do correspond. */
bool compare_variable_decl (tree t1, tree t2);
+ /* Compare loop information for basic blocks BB1 and BB2. */
+ bool compare_loops (basic_block bb1, basic_block bb2);
+
/* Return true if types are compatible for polymorphic call analysis.
COMPARE_PTR indicates if polymorphic type comparsion should be
done for pointers, too. */
diff --git a/gcc/testsuite/gcc.dg/ipa/pr90555.c b/gcc/testsuite/gcc.dg/ipa/pr90555.c
new file mode 100644
index 00000000000..a9c751cd63c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr90555.c
@@ -0,0 +1,67 @@
+/* { dg-do compile } */
+/* { dg-options "-fopenmp-simd -O2 -mavx512f -fdump-ipa-icf" } */
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+
+#define N 1024
+int a[N];
+
+void
+test_simdlen1 (void)
+{
+ int i;
+ #pragma omp simd simdlen (4)
+ for (i = 0; i < N; ++i)
+ a[i] = a[i] + 1;
+}
+
+void
+test_simdlen2 (void)
+{
+ int i;
+ #pragma omp simd simdlen (8)
+ for (i = 0; i < N; ++i)
+ a[i] = a[i] + 1;
+}
+
+void
+test_safelen1 (void)
+{
+ int i;
+ #pragma omp simd safelen (4)
+ for (i = 0; i < N; ++i)
+ a[i] = a[i] + 1;
+}
+
+void
+test_safelen2 (void)
+{
+ int i;
+ #pragma omp simd safelen (8)
+ for (i = 0; i < N; ++i)
+ a[i] = a[i] + 1;
+}
+
+int d[1024];
+
+int
+test_simduid1 (int j, int b)
+{
+ int l, c = 0;
+#pragma omp simd reduction(+: c)
+ for (l = 0; l < b; ++l)
+ c += d[j + l];
+ return c;
+}
+
+int
+test_simduid2 (int j, int b)
+{
+ int l, c2 = 0;
+#pragma omp simd reduction(+: c2)
+ for (l = 0; l < b; ++l)
+ c2 += d[j + l];
+ return c2;
+}
+
+/* { dg-final { scan-ipa-dump "Semantic equality hit:test_simduid1->test_simduid2" "icf" } } */
+/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */
--
2.21.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
2019-05-27 12:50 ` Martin Liška
@ 2019-05-27 13:09 ` Jakub Jelinek
2019-05-28 11:41 ` Martin Liška
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2019-05-27 13:09 UTC (permalink / raw)
To: Martin Liška; +Cc: Richard Biener, GCC Patches, Jan Hubicka
On Mon, May 27, 2019 at 02:18:37PM +0200, Martin Liška wrote:
> +/* Compare loop information for basic blocks BB1 and BB2. */
> +
> +bool
> +func_checker::compare_loops (basic_block bb1, basic_block bb2)
> +{
> + if ((bb1->loop_father == NULL) != (bb2->loop_father == NULL))
> + return return_false ();
> +
> + struct loop *l1 = bb1->loop_father;
> + struct loop *l2 = bb2->loop_father;
Perhaps also
if ((bb1 == l1->header) != (bb2 == l2->header))
return return_false_with_msg ("header");
if ((bb1 == l1->latch) != (bb2 == l2->latch))
return return_false_with_msg ("latch");
too?
BTW, unrelated to this patch, what does ICF do if e.g. SSA_NAME_PTR_INFO
or SSA_NAME_RANGE_INFO is different between otherwise identical functions?
Say one having early
if (arg > 10)
__builtin_unreachable ();
and another one
if (arg > - 10)
__builtin_unreachable ();
both optimized away into SSA_NAME_RANGE_INFO of the argument before IPA (or
do we optimize that away only after IPA?).
Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
2019-05-27 13:09 ` Jakub Jelinek
@ 2019-05-28 11:41 ` Martin Liška
2019-05-28 11:51 ` Jakub Jelinek
2019-05-28 20:44 ` Christophe Lyon
0 siblings, 2 replies; 11+ messages in thread
From: Martin Liška @ 2019-05-28 11:41 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, GCC Patches, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]
On 5/27/19 2:50 PM, Jakub Jelinek wrote:
> On Mon, May 27, 2019 at 02:18:37PM +0200, Martin Liška wrote:
>> +/* Compare loop information for basic blocks BB1 and BB2. */
>> +
>> +bool
>> +func_checker::compare_loops (basic_block bb1, basic_block bb2)
>> +{
>> + if ((bb1->loop_father == NULL) != (bb2->loop_father == NULL))
>> + return return_false ();
>> +
>> + struct loop *l1 = bb1->loop_father;
>> + struct loop *l2 = bb2->loop_father;
>
> Perhaps also
> if ((bb1 == l1->header) != (bb2 == l2->header))
> return return_false_with_msg ("header");
> if ((bb1 == l1->latch) != (bb2 == l2->latch))
> return return_false_with_msg ("latch");
> too?
Yes, makes sense.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> BTW, unrelated to this patch, what does ICF do if e.g. SSA_NAME_PTR_INFO
> or SSA_NAME_RANGE_INFO is different between otherwise identical functions?
> Say one having early
> if (arg > 10)
> __builtin_unreachable ();
> and another one
> if (arg > - 10)
> __builtin_unreachable ();
> both optimized away into SSA_NAME_RANGE_INFO of the argument before IPA (or
> do we optimize that away only after IPA?).
>
> Jakub
>
Can you please provide a self-contained test-case?
Thanks,
Martin
[-- Attachment #2: 0001-Handle-loop-fields-in-IPA-ICF-PR-ipa-90555.patch --]
[-- Type: text/x-patch, Size: 4854 bytes --]
From 82b84914c3d40094be2ff68498a9120e9b7ece0b Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Thu, 23 May 2019 12:47:11 +0200
Subject: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
gcc/ChangeLog:
2019-05-23 Martin Liska <mliska@suse.cz>
PR ipa/90555
* ipa-icf-gimple.c (func_checker::compare_loops): New function.
* ipa-icf-gimple.h (func_checker::compare_loops): Likewise.
(func_checker::compare_bb): Call compare_loops.
gcc/testsuite/ChangeLog:
2019-05-23 Martin Liska <mliska@suse.cz>
PR ipa/90555
* gcc.dg/ipa/pr90555.c: New test.
---
gcc/ipa-icf-gimple.c | 38 +++++++++++++++++
gcc/ipa-icf-gimple.h | 3 ++
gcc/testsuite/gcc.dg/ipa/pr90555.c | 67 ++++++++++++++++++++++++++++++
3 files changed, 108 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/ipa/pr90555.c
diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 25284936bc3..0713e125898 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3. If not see
#include "ipa-utils.h"
#include "tree-eh.h"
#include "builtins.h"
+#include "cfgloop.h"
#include "ipa-icf-gimple.h"
@@ -605,6 +606,40 @@ func_checker::compare_variable_decl (tree t1, tree t2)
return return_with_debug (ret);
}
+/* Compare loop information for basic blocks BB1 and BB2. */
+
+bool
+func_checker::compare_loops (basic_block bb1, basic_block bb2)
+{
+ if ((bb1->loop_father == NULL) != (bb2->loop_father == NULL))
+ return return_false ();
+
+ struct loop *l1 = bb1->loop_father;
+ struct loop *l2 = bb2->loop_father;
+ if (l1 == NULL)
+ return true;
+
+ if ((bb1 == l1->header) != (bb2 == l2->header))
+ return return_false_with_msg ("header");
+ if ((bb1 == l1->latch) != (bb2 == l2->latch))
+ return return_false_with_msg ("latch");
+ if (l1->simdlen != l2->simdlen)
+ return return_false_with_msg ("simdlen");
+ if (l1->safelen != l2->safelen)
+ return return_false_with_msg ("safelen");
+ if (l1->can_be_parallel != l2->can_be_parallel)
+ return return_false_with_msg ("can_be_parallel");
+ if (l1->dont_vectorize != l2->dont_vectorize)
+ return return_false_with_msg ("dont_vectorize");
+ if (l1->force_vectorize != l2->force_vectorize)
+ return return_false_with_msg ("force_vectorize");
+ if (l1->unroll != l2->unroll)
+ return return_false_with_msg ("unroll");
+ if (!compare_variable_decl (l1->simduid, l2->simduid))
+ return return_false_with_msg ("simduid");
+
+ return true;
+}
/* Function visits all gimple labels and creates corresponding
mapping between basic blocks and labels. */
@@ -727,6 +762,9 @@ func_checker::compare_bb (sem_bb *bb1, sem_bb *bb2)
if (!gsi_end_p (gsi2))
return return_false ();
+ if (!compare_loops (bb1->bb, bb2->bb))
+ return return_false ();
+
return true;
}
diff --git a/gcc/ipa-icf-gimple.h b/gcc/ipa-icf-gimple.h
index 0035db32e66..51aadced9ea 100644
--- a/gcc/ipa-icf-gimple.h
+++ b/gcc/ipa-icf-gimple.h
@@ -226,6 +226,9 @@ public:
/* Verifies that trees T1 and T2 do correspond. */
bool compare_variable_decl (tree t1, tree t2);
+ /* Compare loop information for basic blocks BB1 and BB2. */
+ bool compare_loops (basic_block bb1, basic_block bb2);
+
/* Return true if types are compatible for polymorphic call analysis.
COMPARE_PTR indicates if polymorphic type comparsion should be
done for pointers, too. */
diff --git a/gcc/testsuite/gcc.dg/ipa/pr90555.c b/gcc/testsuite/gcc.dg/ipa/pr90555.c
new file mode 100644
index 00000000000..a9c751cd63c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr90555.c
@@ -0,0 +1,67 @@
+/* { dg-do compile } */
+/* { dg-options "-fopenmp-simd -O2 -mavx512f -fdump-ipa-icf" } */
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+
+#define N 1024
+int a[N];
+
+void
+test_simdlen1 (void)
+{
+ int i;
+ #pragma omp simd simdlen (4)
+ for (i = 0; i < N; ++i)
+ a[i] = a[i] + 1;
+}
+
+void
+test_simdlen2 (void)
+{
+ int i;
+ #pragma omp simd simdlen (8)
+ for (i = 0; i < N; ++i)
+ a[i] = a[i] + 1;
+}
+
+void
+test_safelen1 (void)
+{
+ int i;
+ #pragma omp simd safelen (4)
+ for (i = 0; i < N; ++i)
+ a[i] = a[i] + 1;
+}
+
+void
+test_safelen2 (void)
+{
+ int i;
+ #pragma omp simd safelen (8)
+ for (i = 0; i < N; ++i)
+ a[i] = a[i] + 1;
+}
+
+int d[1024];
+
+int
+test_simduid1 (int j, int b)
+{
+ int l, c = 0;
+#pragma omp simd reduction(+: c)
+ for (l = 0; l < b; ++l)
+ c += d[j + l];
+ return c;
+}
+
+int
+test_simduid2 (int j, int b)
+{
+ int l, c2 = 0;
+#pragma omp simd reduction(+: c2)
+ for (l = 0; l < b; ++l)
+ c2 += d[j + l];
+ return c2;
+}
+
+/* { dg-final { scan-ipa-dump "Semantic equality hit:test_simduid1->test_simduid2" "icf" } } */
+/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */
--
2.21.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
2019-05-28 11:41 ` Martin Liška
@ 2019-05-28 11:51 ` Jakub Jelinek
2019-05-28 11:53 ` Martin Liška
2019-05-28 12:12 ` Martin Jambor
2019-05-28 20:44 ` Christophe Lyon
1 sibling, 2 replies; 11+ messages in thread
From: Jakub Jelinek @ 2019-05-28 11:51 UTC (permalink / raw)
To: Martin Liška; +Cc: Richard Biener, GCC Patches, Jan Hubicka
On Tue, May 28, 2019 at 01:29:54PM +0200, Martin Liška wrote:
> Yes, makes sense.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ok, thanks.
> > BTW, unrelated to this patch, what does ICF do if e.g. SSA_NAME_PTR_INFO
> > or SSA_NAME_RANGE_INFO is different between otherwise identical functions?
> > Say one having early
> > if (arg > 10)
> > __builtin_unreachable ();
> > and another one
> > if (arg > - 10)
> > __builtin_unreachable ();
> > both optimized away into SSA_NAME_RANGE_INFO of the argument before IPA (or
> > do we optimize that away only after IPA?).
> >
> > Jakub
> >
>
> Can you please provide a self-contained test-case?
I don't have one so far, it might be only vrp1 that removes those and thus
be fine for the user written asserts. E.g. with the patch for C++ ::main argc
range it could perhaps affect code generation if we decide to ICF main with
some other function with the same content (but that patch is not in).
Another thing is I'm not sure how IPA-VRP vs. ICF interact, whether first
ranges would be set for the parameters and then ICF pick one function body,
or the other way around.
Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
2019-05-28 11:51 ` Jakub Jelinek
@ 2019-05-28 11:53 ` Martin Liška
2019-05-28 12:12 ` Martin Jambor
1 sibling, 0 replies; 11+ messages in thread
From: Martin Liška @ 2019-05-28 11:53 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, GCC Patches, Jan Hubicka
On 5/28/19 1:41 PM, Jakub Jelinek wrote:
> On Tue, May 28, 2019 at 01:29:54PM +0200, Martin Liška wrote:
>> Yes, makes sense.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ok, thanks.
>
>>> BTW, unrelated to this patch, what does ICF do if e.g. SSA_NAME_PTR_INFO
>>> or SSA_NAME_RANGE_INFO is different between otherwise identical functions?
>>> Say one having early
>>> if (arg > 10)
>>> __builtin_unreachable ();
>>> and another one
>>> if (arg > - 10)
>>> __builtin_unreachable ();
>>> both optimized away into SSA_NAME_RANGE_INFO of the argument before IPA (or
>>> do we optimize that away only after IPA?).
>>>
>>> Jakub
>>>
>>
>> Can you please provide a self-contained test-case?
>
> I don't have one so far, it might be only vrp1 that removes those and thus
> be fine for the user written asserts. E.g. with the patch for C++ ::main argc
> range it could perhaps affect code generation if we decide to ICF main with
> some other function with the same content (but that patch is not in).
That's handled here:
3493 if (DECL_NAME (source->decl)
3494 && MAIN_NAME_P (DECL_NAME (source->decl)))
3495 /* If merge via wrappers, picking main as the target can be
3496 problematic. */
3497 source = c->members[1];
> Another thing is I'm not sure how IPA-VRP vs. ICF interact, whether first
> ranges would be set for the parameters and then ICF pick one function body,
> or the other way around.
I'll discuss that with Martin Jambor.
Martin
>
> Jakub
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
2019-05-28 11:51 ` Jakub Jelinek
2019-05-28 11:53 ` Martin Liška
@ 2019-05-28 12:12 ` Martin Jambor
1 sibling, 0 replies; 11+ messages in thread
From: Martin Jambor @ 2019-05-28 12:12 UTC (permalink / raw)
To: Jakub Jelinek, Martin Liška; +Cc: Richard Biener, GCC Patches, Jan Hubicka
Hi,
On Tue, May 28 2019, Jakub Jelinek wrote:
> On Tue, May 28, 2019 at 01:29:54PM +0200, Martin Liška wrote:
>> Yes, makes sense.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ok, thanks.
>
>> > BTW, unrelated to this patch, what does ICF do if e.g. SSA_NAME_PTR_INFO
>> > or SSA_NAME_RANGE_INFO is different between otherwise identical functions?
>> > Say one having early
>> > if (arg > 10)
>> > __builtin_unreachable ();
>> > and another one
>> > if (arg > - 10)
>> > __builtin_unreachable ();
>> > both optimized away into SSA_NAME_RANGE_INFO of the argument before IPA (or
>> > do we optimize that away only after IPA?).
>> >
>> > Jakub
>> >
>>
>> Can you please provide a self-contained test-case?
>
> I don't have one so far, it might be only vrp1 that removes those and thus
> be fine for the user written asserts. E.g. with the patch for C++ ::main argc
> range it could perhaps affect code generation if we decide to ICF main with
> some other function with the same content (but that patch is not in).
> Another thing is I'm not sure how IPA-VRP vs. ICF interact, whether first
> ranges would be set for the parameters and then ICF pick one function body,
> or the other way around.
IPA-ICF runs first and unifies call graph nodes. At that point the
actual VRP information for IPA-VRP information are stored along call
graph edges (the other bit stored along nodes are types of formal
parameters), so assuming the actual call arguments have the same value
ranges in the unified bodies, it should work.
Martin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
2019-05-28 11:41 ` Martin Liška
2019-05-28 11:51 ` Jakub Jelinek
@ 2019-05-28 20:44 ` Christophe Lyon
2019-05-29 6:37 ` Martin Liška
1 sibling, 1 reply; 11+ messages in thread
From: Christophe Lyon @ 2019-05-28 20:44 UTC (permalink / raw)
To: Martin Liška; +Cc: Jakub Jelinek, Richard Biener, GCC Patches, Jan Hubicka
Hi Martin,
On Tue, 28 May 2019 at 13:30, Martin Liška <mliska@suse.cz> wrote:
>
> On 5/27/19 2:50 PM, Jakub Jelinek wrote:
> > On Mon, May 27, 2019 at 02:18:37PM +0200, Martin Liška wrote:
> >> +/* Compare loop information for basic blocks BB1 and BB2. */
> >> +
> >> +bool
> >> +func_checker::compare_loops (basic_block bb1, basic_block bb2)
> >> +{
> >> + if ((bb1->loop_father == NULL) != (bb2->loop_father == NULL))
> >> + return return_false ();
> >> +
> >> + struct loop *l1 = bb1->loop_father;
> >> + struct loop *l2 = bb2->loop_father;
> >
> > Perhaps also
> > if ((bb1 == l1->header) != (bb2 == l2->header))
> > return return_false_with_msg ("header");
> > if ((bb1 == l1->latch) != (bb2 == l2->latch))
> > return return_false_with_msg ("latch");
> > too?
>
> Yes, makes sense.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
The new testcase contains:
/* { dg-do compile } */
/* { dg-options "-fopenmp-simd -O2 -mavx512f -fdump-ipa-icf" } */
/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
I suspect line 3 should actually be line 1, because the test fails to
compile on non-x86 targets:
xgcc: error: unrecognized command line option '-mavx512f'
Was it your intention?
Thanks,
Christophe
> >
> > BTW, unrelated to this patch, what does ICF do if e.g. SSA_NAME_PTR_INFO
> > or SSA_NAME_RANGE_INFO is different between otherwise identical functions?
> > Say one having early
> > if (arg > 10)
> > __builtin_unreachable ();
> > and another one
> > if (arg > - 10)
> > __builtin_unreachable ();
> > both optimized away into SSA_NAME_RANGE_INFO of the argument before IPA (or
> > do we optimize that away only after IPA?).
> >
> > Jakub
> >
>
> Can you please provide a self-contained test-case?
>
> Thanks,
> Martin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).
2019-05-28 20:44 ` Christophe Lyon
@ 2019-05-29 6:37 ` Martin Liška
0 siblings, 0 replies; 11+ messages in thread
From: Martin Liška @ 2019-05-29 6:37 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Jakub Jelinek, Richard Biener, GCC Patches, Jan Hubicka
On 5/28/19 10:31 PM, Christophe Lyon wrote:
> Was it your intention?
Hi.
No ;) fixed as r271729.
Thanks for reporting,
Martin
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-05-29 6:10 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-24 7:48 [PATCH] Handle loop fields in IPA ICF (PR ipa/90555) Martin Liška
2019-05-24 8:08 ` Jakub Jelinek
2019-05-24 9:38 ` Richard Biener
2019-05-27 12:50 ` Martin Liška
2019-05-27 13:09 ` Jakub Jelinek
2019-05-28 11:41 ` Martin Liška
2019-05-28 11:51 ` Jakub Jelinek
2019-05-28 11:53 ` Martin Liška
2019-05-28 12:12 ` Martin Jambor
2019-05-28 20:44 ` Christophe Lyon
2019-05-29 6:37 ` Martin Liška
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).