This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

can_refer_decl_in_current_unit_p TLC


Hi,
can_refer_decl_in_current_unit_p is a black magic that carefuly papers around
a mine field created by combination of C++ ABI and ELF visibility attributes.
As such it is not pretty and grown up into quite a maze of rules.

Well, I think it is time to clean it up: for example it does twice very similar
check - for statics and for comdats it checks that definition is still around,
but the code used is very different (one written before symtab, other more
modern). It is also wrong in some cases that apparently don't happen in pracitce;
for example it check presence of symtab node for static symbol but it does not
check that the definition is actually around to be output.

This patch cleans it up and should make it more robust and also less
conservative for WHOPR and COMDATs put into other partition. It also fixes
tree-optimization/60899 (and updates devirt-11.C testcase that tests the
wrong devirtualization happens), I will send release branch fix shortly.

Bootstrapped/regtested x86_64-linux, will comitted.

Honza

	PR tree-optimization/60899
	* gimple-fold.c (can_refer_decl_in_current_unit_p): Cleanup;
	assume all static symbols will have definition wile parsing and
	check the do have definition later in compilation; check that
	variable referring symbol will be output before concluding that
	reference is safe; be conservative for referring local statics;
	be more precise about when comdat is output in other partition.

	g++.dg/ipa/devirt-11.C: Update template.

Index: gimple-fold.c
===================================================================
--- gimple-fold.c	(revision 210653)
+++ gimple-fold.c	(working copy)
@@ -93,8 +93,12 @@ can_refer_decl_in_current_unit_p (tree d
   /* Static objects can be referred only if they was not optimized out yet.  */
   if (!TREE_PUBLIC (decl) && !DECL_EXTERNAL (decl))
     {
+      /* Before we start optimizing unreachable code we can be sure all
+	 static objects are defined.  */
+      if (cgraph_function_flags_ready)
+	return true;
       snode = symtab_get_node (decl);
-      if (!snode)
+      if (!snode || !snode->definition)
 	return false;
       node = dyn_cast <cgraph_node *> (snode);
       return !node || !node->global.inlined_to;
@@ -102,10 +106,12 @@ can_refer_decl_in_current_unit_p (tree d
 
   /* We will later output the initializer, so we can refer to it.
      So we are concerned only when DECL comes from initializer of
-     external var.  */
+     external var or var that has been optimized out.  */
   if (!from_decl
       || TREE_CODE (from_decl) != VAR_DECL
-      || !DECL_EXTERNAL (from_decl)
+      || (!DECL_EXTERNAL (from_decl)
+	  && (vnode = varpool_get_node (from_decl)) != NULL
+	  && vnode->definition)
       || (flag_ltrans
 	  && symtab_get_node (from_decl)->in_other_partition))
     return true;
@@ -122,9 +128,9 @@ can_refer_decl_in_current_unit_p (tree d
      reference imply need to include function body in the curren tunit.  */
   if (TREE_PUBLIC (decl) && !DECL_COMDAT (decl))
     return true;
-  /* We are not at ltrans stage; so don't worry about WHOPR.
-     Also when still gimplifying all referred comdat functions will be
-     produced.
+  /* We have COMDAT.  We are going to check if we still have definition
+     or if the definition is going to be output in other partition.
+     Bypass this when gimplifying; all needed functions will be produced.
 
      As observed in PR20991 for already optimized out comdat virtual functions
      it may be tempting to not necessarily give up because the copy will be
@@ -133,35 +139,17 @@ can_refer_decl_in_current_unit_p (tree d
      units where they are used and when the other unit was compiled with LTO
      it is possible that vtable was kept public while the function itself
      was privatized. */
-  if (!flag_ltrans && (!DECL_COMDAT (decl) || !cgraph_function_flags_ready))
+  if (!cgraph_function_flags_ready)
     return true;
 
-  /* OK we are seeing either COMDAT or static variable.  In this case we must
-     check that the definition is still around so we can refer it.  */
-  if (TREE_CODE (decl) == FUNCTION_DECL)
-    {
-      node = cgraph_get_node (decl);
-      /* Check that we still have function body and that we didn't took
-         the decision to eliminate offline copy of the function yet.
-         The second is important when devirtualization happens during final
-         compilation stage when making a new reference no longer makes callee
-         to be compiled.  */
-      if (!node || !node->definition || node->global.inlined_to)
-	{
-	  gcc_checking_assert (!TREE_ASM_WRITTEN (decl));
-	  return false;
-	}
-    }
-  else if (TREE_CODE (decl) == VAR_DECL)
-    {
-      vnode = varpool_get_node (decl);
-      if (!vnode || !vnode->definition)
-	{
-	  gcc_checking_assert (!TREE_ASM_WRITTEN (decl));
-	  return false;
-	}
-    }
-  return true;
+  snode = symtab_get_node (decl);
+  if (!snode
+      || ((!snode->definition || DECL_EXTERNAL (decl))
+	  && (!snode->in_other_partition
+	      || (!snode->forced_by_abi && !snode->force_output))))
+    return false;
+  node = dyn_cast <cgraph_node *> (snode);
+  return !node || !node->global.inlined_to;
 }
 
 /* CVAL is value taken from DECL_INITIAL of variable.  Try to transform it into
Index: testsuite/g++.dg/ipa/devirt-11.C
===================================================================
--- testsuite/g++.dg/ipa/devirt-11.C	(revision 210673)
+++ testsuite/g++.dg/ipa/devirt-11.C	(working copy)
@@ -42,8 +42,7 @@ bar ()
   baz ();
   c + d;
 }
-/* While inlining function called once we should devirtualize a new call to fn2
-   and two to fn3. While doing so the new symbol for fn2 needs to be
-   introduced.  */
-/* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known target" 3 "inline"  } } */
+/* While inlining function called once we should devirtualize a new call to fn3.
+   Because fn2 is already removed, we should not devirtualize.  */
+/* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known target" 1 "inline"  } } */
 /* { dg-final { cleanup-ipa-dump "inline" } } */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]