Re: [PATCH] Fix PR ipa/65722

[Jan Hubicka <hubicka at ucw dot cz>]

> > 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

Hi,
this is variant I am testing. 

Index: ipa-icf.c
===================================================================
--- ipa-icf.c	(revision 222010)
+++ ipa-icf.c	(working copy)
@@ -368,6 +368,10 @@ sem_item::compare_cgraph_references (
   if (n1 == n2)
     return true;
 
+  /* Never match variable and function.  */
+  if (is_a <varpool_node *> (n1) != is_a <varpool_node *> (n2))
+    return false;
+
   /* Merging two definitions with a reference to equivalent vtables, but
      belonging to a different type may result in ipa-polymorphic-call analysis
      giving a wrong answer about the dynamic type of instance.  */
@@ -587,9 +591,6 @@ void
 sem_item::update_hash_by_addr_refs (hash_map <symtab_node *,
 				    sem_item *> &m_symtab_node_map)
 {
-  if (is_a <varpool_node *> (node) && DECL_VIRTUAL_P (node->decl))
-    return;
-
   ipa_ref* ref;
   inchash::hash hstate (hash);
   for (unsigned i = 0; i < node->num_references (); i++)
@@ -1667,17 +1668,19 @@ sem_variable::equals_wpa (sem_item *item
 				      ref->address_matters_p ()))
 	return false;
 
-      /* 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 ((DECL_VIRTUAL_P (decl) || DECL_VIRTUAL_P (item->decl))
-	  && (DECL_VIRTUAL_P (ref->referred->decl)
-	      || DECL_VIRTUAL_P (ref2->referred->decl))
-	  && ((DECL_VIRTUAL_P (ref->referred->decl)
-	       != DECL_VIRTUAL_P (ref2->referred->decl))
-	      || (DECL_FINAL_P (ref->referred->decl)
-		  != DECL_FINAL_P (ref2->referred->decl))))
-        return return_false_with_msg ("virtual or final flag mismatch");
+      /* When matching virtual tables, be sure to also match information
+ 	 relevant for polymorphic call analysis.  */
+      if (DECL_VIRTUAL_P (decl) || DECL_VIRTUAL_P (item->decl))
+	{
+	  if (DECL_VIRTUAL_P (ref->referred->decl)
+	      != DECL_VIRTUAL_P (ref2->referred->decl))
+            return return_false_with_msg ("virtual flag mismatch");
+	  if (DECL_VIRTUAL_P (ref->referred->decl)
+	      && is_a <cgraph_node *> (ref->referred)
+	      && (DECL_FINAL_P (ref->referred->decl)
+		  != DECL_FINAL_P (ref2->referred->decl)))
+            return return_false_with_msg ("final flag mismatch");
+	}
     }
 
   return true;