Fix PR ipa/65722

Hello.

Attached patch fixes PR, where we should consider just cgraph_nodes as
objects referred by virtual table.

Patch survives regression tests on x86_64-linux-pc with enabled checking.

Ready for trunk?
Thanks,
Martin

On Fri, Apr 10, 2015 at 06:15:01PM +0200, Martin Liška wrote:
> Attached patch fixes PR, where we should consider just cgraph_nodes as
> objects referred by virtual table.
> 
> Patch survives regression tests on x86_64-linux-pc with enabled checking.
> 
> Ready for trunk?
> Thanks,
> Martin

> >From 8bff38438f6ba57732a6c3ccc3632f6789ca4d7e Mon Sep 17 00:00:00 2001
> From: mliska <mliska@suse.cz>
> Date: Fri, 10 Apr 2015 11:37:04 +0200
> Subject: [PATCH] Fix PR ipa/65722.
> 
> gcc/testsuite/ChangeLog:
> 
> 2015-04-10  Martin Liska  <mliska@suse.cz>
> 

Please add PR line here too.

> 	* g++.dg/ipa/pr65722.C: New test.
> 
> gcc/ChangeLog:
> 
> 2015-04-10  Martin Liska  <mliska@suse.cz>
> 
> 	PR ipa/65722
> 	* ipa-icf.c (sem_variable::equals_wpa): Consider comparsion just
> 	for references coming from cgraph nodes.
> ---
>  gcc/ipa-icf.c                      |  4 ++++
>  gcc/testsuite/g++.dg/ipa/pr65722.C | 21 +++++++++++++++++++++
>  2 files changed, 25 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/ipa/pr65722.C
> 
> diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
> index 8f8a0cf..9e5d19c 100644
> --- a/gcc/ipa-icf.c
> +++ b/gcc/ipa-icf.c
> @@ -1670,6 +1670,10 @@ sem_variable::equals_wpa (sem_item *item,
>        /* DECL_FINAL_P flag on methods referred by virtual tables is used
>  	 to decide on completeness possible_polymorphic_call_targets lists
>  	 and therefore it must match.  */
> +      if (!is_a <cgraph_node *> (ref->referred)
> +	  || !is_a <cgraph_node *> (ref2->referred))
> +	continue;
> +

Wouldn't it be better to move this check before the DECL_FINAL_P comment, so
that it is closer to the uses?

	Jakub

> 2015-04-10  Martin Liska  <mliska@suse.cz>
> 
> 	* g++.dg/ipa/pr65722.C: New test.
> 
> gcc/ChangeLog:
> 
> 2015-04-10  Martin Liska  <mliska@suse.cz>
> 
> 	PR ipa/65722
> 	* ipa-icf.c (sem_variable::equals_wpa): Consider comparsion just
> 	for references coming from cgraph nodes.

Please add into compare_cgraph_references to never return true when one
parameter is function and other variable. How it comes we do not get different
hash values here? Perhaps when adding the hash values of references, we sould
iteratively hash in some identifier saying if object is function or variable.

For vtables, we want to test DECL_VIRTUAL_P for match even for variables,
because we do not want to match RTTI and vtable (though it may be unlikely
that they are the same).
Refactor the code to test

/* For virtual tables we need to check flags used by ipa-devirt.  */
if (DECL_VIRTUAL_P (decl) || DECL_VIRTUAL_P (item->decl))
  {
    if (DECL_VIRTUAL_P (ref->referred->decl) != DECL_VIRTUAL_P (ref2->referred->decl))
      fail claiming that virutal flag mismatched
    if (is_a_funtion && DECL_VIRTUAL_P (ref->referred->decl)
        && DECL_FINAL (ref->referred->decl) != DECL_FINAL (ref2->referred->decl))
      fail chaliming that final flag mismatched
  }

The conditional is quite confusing written as it is. (probably by myself :)
Thanks!
Honza
> ---
>  gcc/ipa-icf.c                      |  4 ++++
>  gcc/testsuite/g++.dg/ipa/pr65722.C | 21 +++++++++++++++++++++
>  2 files changed, 25 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/ipa/pr65722.C
> 
> diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
> index 8f8a0cf..9e5d19c 100644
> --- a/gcc/ipa-icf.c
> +++ b/gcc/ipa-icf.c
> @@ -1670,6 +1670,10 @@ sem_variable::equals_wpa (sem_item *item,
>        /* DECL_FINAL_P flag on methods referred by virtual tables is used
>  	 to decide on completeness possible_polymorphic_call_targets lists
>  	 and therefore it must match.  */
> +      if (!is_a <cgraph_node *> (ref->referred)
> +	  || !is_a <cgraph_node *> (ref2->referred))
> +	continue;
> +
>        if ((DECL_VIRTUAL_P (decl) || DECL_VIRTUAL_P (item->decl))
>  	  && (DECL_VIRTUAL_P (ref->referred->decl)
>  	      || DECL_VIRTUAL_P (ref2->referred->decl))
> diff --git a/gcc/testsuite/g++.dg/ipa/pr65722.C b/gcc/testsuite/g++.dg/ipa/pr65722.C
> new file mode 100644
> index 0000000..ee4ea24
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ipa/pr65722.C
> @@ -0,0 +1,21 @@
> +// { dg-do compile }
> +// { dg-options "-O -fipa-icf -fno-rtti" }
> +
> +struct A
> +{
> +  virtual void f ()
> +  {
> +    __builtin_abort ();
> +  }
> +  virtual void g ();
> +};
> +
> +struct B : virtual A { };
> +struct C : B, virtual A { };
> +
> +void foo()
> +{
> +  C c;
> +  C *p = &c;
> +  p->f ();
> +}
> -- 
> 2.1.4
>