On 5/24/19 11:37 AM, Richard Biener wrote:
> On Fri, May 24, 2019 at 10:08 AM Jakub Jelinek <ja...@redhat.com> wrote:
>>
>> On Fri, May 24, 2019 at 09:48:03AM +0200, Martin Liška wrote:
>>> gcc/ChangeLog:
>>>
>>> 2019-05-23 Martin Liska <mli...@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 <mli...@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
>From 306cab10e8ef918b25321132aa75d9400f7de247 Mon Sep 17 00:00:00 2001
From: Martin Liska <mli...@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 <mli...@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 <mli...@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
