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?
> + {
> + 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
