diff mbox series

Fix inter-procedural EAF flags propagation with respect to !binds_to_current_def_p

Message ID 20211107114256.GA50485@kam.mff.cuni.cz
State New
Headers show
Series Fix inter-procedural EAF flags propagation with respect to !binds_to_current_def_p | expand

Commit Message

Jan Hubicka Nov. 7, 2021, 11:42 a.m. UTC
Hi,
while proofreading the code for handling EAF flags of
!binds_to_current_def_p I noticed that the interprocedural dataflow
actually ignores the flag possibly introducing wrong code on nterposable
functions in non-trivial recursion cycles or at ltrans partition
boundary.

This patch unifies the flags changes to single place (remove_useless_eaf_flags)
and does extend modref_merge_call_site_flags to do the right thing.

lto-bootstrapped/regtested x86_64-linux.  Plan to commit it today after bit
more testing (firefox/clang build).

gcc/ChangeLog:

	* gimple.c (gimple_call_arg_flags): Use interposable_eaf_flags.
	(gimple_call_retslot_flags): Likewise.
	(gimple_call_static_chain_flags): Likewise.
	* ipa-modref.c (remove_useless_eaf_flags): Do not remove everything for
	NOVOPS.
	(modref_summary::useful_p): Likewise.
	(modref_summary_lto::useful_p): Likewise.
	(analyze_parms): Do not kive up on NOVOPS.
	(modref_merge_call_site_flags): Compute implicit eaf flags
	based on callee ecf_flags and fnspec; if the function does not
	bind to current defs use interposable_eaf_flags.
	(modref_propagate_flags_in_scc): Update.
	* ipa-modref.h (interposable_eaf_flags): New function.
diff mbox series

Patch

diff --git a/gcc/gimple.c b/gcc/gimple.c
index 7a578f5113e..3d1d3a15b2c 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -1595,17 +1595,7 @@  gimple_call_arg_flags (const gcall *stmt, unsigned arg)
 
 	  /* We have possibly optimized out load.  Be conservative here.  */
 	  if (!node->binds_to_current_def_p ())
-	    {
-	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
-		{
-		  modref_flags &= ~EAF_UNUSED;
-		  modref_flags |= EAF_NOESCAPE;
-		}
-	      if ((modref_flags & EAF_NOREAD) && !(flags & EAF_NOREAD))
-		modref_flags &= ~EAF_NOREAD;
-	      if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
-		modref_flags &= ~EAF_DIRECT;
-	    }
+	    modref_flags = interposable_eaf_flags (modref_flags, flags);
 	  if (dbg_cnt (ipa_mod_ref_pta))
 	    flags |= modref_flags;
 	}
@@ -1633,13 +1623,7 @@  gimple_call_retslot_flags (const gcall *stmt)
 
 	  /* We have possibly optimized out load.  Be conservative here.  */
 	  if (!node->binds_to_current_def_p ())
-	    {
-	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
-		{
-		  modref_flags &= ~EAF_UNUSED;
-		  modref_flags |= EAF_NOESCAPE;
-		}
-	    }
+	    modref_flags = interposable_eaf_flags (modref_flags, flags);
 	  if (dbg_cnt (ipa_mod_ref_pta))
 	    flags |= modref_flags;
 	}
@@ -1665,19 +1649,9 @@  gimple_call_static_chain_flags (const gcall *stmt)
 	{
 	  int modref_flags = summary->static_chain_flags;
 
-	  /* We have possibly optimized out load.  Be conservative here.  */
+	  /* ??? Nested functions should always bind to current def.  */
 	  if (!node->binds_to_current_def_p ())
-	    {
-	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
-		{
-		  modref_flags &= ~EAF_UNUSED;
-		  modref_flags |= EAF_NOESCAPE;
-		}
-	      if ((modref_flags & EAF_NOREAD) && !(flags & EAF_NOREAD))
-		modref_flags &= ~EAF_NOREAD;
-	      if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
-		modref_flags &= ~EAF_DIRECT;
-	    }
+	    modref_flags = interposable_eaf_flags (modref_flags, flags);
 	  if (dbg_cnt (ipa_mod_ref_pta))
 	    flags |= modref_flags;
 	}
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 5209fbdfbf4..26a719ff40c 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -291,9 +291,7 @@  modref_summary::~modref_summary ()
 static int
 remove_useless_eaf_flags (int eaf_flags, int ecf_flags, bool returns_void)
 {
-  if (ecf_flags & ECF_NOVOPS)
-    return 0;
-  if (ecf_flags & ECF_CONST)
+  if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
     eaf_flags &= ~implicit_const_eaf_flags;
   else if (ecf_flags & ECF_PURE)
     eaf_flags &= ~implicit_pure_eaf_flags;
@@ -319,8 +317,6 @@  eaf_flags_useful_p (vec <eaf_flags_t> &flags, int ecf_flags)
 bool
 modref_summary::useful_p (int ecf_flags, bool check_flags)
 {
-  if (ecf_flags & ECF_NOVOPS)
-    return false;
   if (arg_flags.length () && !check_flags)
     return true;
   if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
@@ -331,7 +327,7 @@  modref_summary::useful_p (int ecf_flags, bool check_flags)
   if (check_flags
       && remove_useless_eaf_flags (static_chain_flags, ecf_flags, false))
     return true;
-  if (ecf_flags & ECF_CONST)
+  if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
     return false;
   if (loads && !loads->every_base)
     return true;
@@ -405,8 +401,6 @@  modref_summary_lto::~modref_summary_lto ()
 bool
 modref_summary_lto::useful_p (int ecf_flags, bool check_flags)
 {
-  if (ecf_flags & ECF_NOVOPS)
-    return false;
   if (arg_flags.length () && !check_flags)
     return true;
   if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
@@ -417,7 +411,7 @@  modref_summary_lto::useful_p (int ecf_flags, bool check_flags)
   if (check_flags
       && remove_useless_eaf_flags (static_chain_flags, ecf_flags, false))
     return true;
-  if (ecf_flags & ECF_CONST)
+  if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
     return false;
   if (loads && !loads->every_base)
     return true;
@@ -2321,10 +2315,6 @@  analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto,
   tree retslot = NULL;
   tree static_chain = NULL;
 
-  /* For novops functions we have nothing to gain by EAF flags.  */
-  if (ecf_flags & ECF_NOVOPS)
-    return;
-
   /* If there is return slot, look up its SSA name.  */
   if (DECL_RESULT (current_function_decl)
       && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
@@ -4039,12 +4029,15 @@  modref_merge_call_site_flags (escape_summary *sum,
 			      modref_summary *summary,
 			      modref_summary_lto *summary_lto,
 			      tree caller,
-			      int ecf_flags)
+			      cgraph_edge *e,
+			      int caller_ecf_flags,
+			      int callee_ecf_flags,
+			      bool binds_to_current_def)
 {
   escape_entry *ee;
   unsigned int i;
   bool changed = false;
-  bool ignore_stores = ignore_stores_p (caller, ecf_flags);
+  bool ignore_stores = ignore_stores_p (caller, callee_ecf_flags);
 
   /* If we have no useful info to propagate.  */
   if ((!cur_summary || !cur_summary->arg_flags.length ())
@@ -4055,6 +4048,8 @@  modref_merge_call_site_flags (escape_summary *sum,
     {
       int flags = 0;
       int flags_lto = 0;
+      /* Returning the value is already accounted to at local propagation.  */
+      int implicit_flags = EAF_NOT_RETURNED | EAF_NOT_RETURNED_DIRECTLY;
 
       if (summary && ee->arg < summary->arg_flags.length ())
 	flags = summary->arg_flags[ee->arg];
@@ -4066,14 +4061,42 @@  modref_merge_call_site_flags (escape_summary *sum,
 	  flags = deref_flags (flags, ignore_stores);
 	  flags_lto = deref_flags (flags_lto, ignore_stores);
 	}
-      else if (ignore_stores)
+      if (ignore_stores)
+	 implicit_flags |= ignore_stores_eaf_flags;
+      if (callee_ecf_flags & ECF_PURE)
+	implicit_flags |= implicit_pure_eaf_flags;
+      if (callee_ecf_flags & (ECF_CONST | ECF_NOVOPS))
+	implicit_flags |= implicit_const_eaf_flags;
+      class fnspec_summary *fnspec_sum = fnspec_summaries->get (e);
+      if (fnspec_sum)
 	{
-	  flags |= ignore_stores_eaf_flags;
-	  flags_lto |= ignore_stores_eaf_flags;
+	  attr_fnspec fnspec (fnspec_sum->fnspec);
+	  int fnspec_flags = 0;
+	  if (fnspec.arg_specified_p (ee->arg))
+	    {
+	      if (!fnspec.arg_used_p (ee->arg))
+		fnspec_flags = EAF_UNUSED;
+	      else
+		{
+		  if (fnspec.arg_direct_p (ee->arg))
+		    fnspec_flags |= EAF_DIRECT;
+		  if (fnspec.arg_noescape_p (ee->arg))
+		    fnspec_flags |= EAF_NOESCAPE | EAF_NODIRECTESCAPE;
+		  if (fnspec.arg_readonly_p (ee->arg))
+		    fnspec_flags |= EAF_NOCLOBBER;
+		}
+	    }
+	  if (!ee->direct)
+	    fnspec_flags = deref_flags (fnspec_flags, ignore_stores);
+	  implicit_flags |= fnspec_flags;
+	}
+      flags |= implicit_flags;
+      flags_lto |= implicit_flags;
+      if (!binds_to_current_def && (flags || flags_lto))
+	{
+	  flags = interposable_eaf_flags (flags, implicit_flags);
+	  flags_lto = interposable_eaf_flags (flags_lto, implicit_flags);
 	}
-      /* Returning the value is already accounted to at local propagation.  */
-      flags |= ee->min_flags | EAF_NOT_RETURNED | EAF_NOT_RETURNED_DIRECTLY;
-      flags_lto |= ee->min_flags | EAF_NOT_RETURNED | EAF_NOT_RETURNED_DIRECTLY;
       /* Noescape implies that value also does not escape directly.
 	 Fnspec machinery does set both so compensate for this.  */
       if (flags & EAF_NOESCAPE)
@@ -4095,7 +4118,7 @@  modref_merge_call_site_flags (escape_summary *sum,
 	  if ((f & flags) != f)
 	    {
 	      f = remove_useless_eaf_flags
-			 (f & flags, ecf_flags,
+			 (f & flags, caller_ecf_flags,
 			  VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
 	      changed = true;
 	    }
@@ -4112,7 +4135,7 @@  modref_merge_call_site_flags (escape_summary *sum,
 	  if ((f & flags_lto) != f)
 	    {
 	      f = remove_useless_eaf_flags
-			 (f & flags_lto, ecf_flags,
+			 (f & flags_lto, caller_ecf_flags,
 			  VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
 	      changed = true;
 	    }
@@ -4146,6 +4169,7 @@  modref_propagate_flags_in_scc (cgraph_node *component_node)
 
 	  if (!cur_summary && !cur_summary_lto)
 	    continue;
+	  int caller_ecf_flags = flags_from_decl_or_type (cur->decl);
 
 	  if (dump_file)
 	    fprintf (dump_file, "  Processing %s%s%s\n",
@@ -4164,7 +4188,11 @@  modref_propagate_flags_in_scc (cgraph_node *component_node)
 	      changed |= modref_merge_call_site_flags
 				(sum, cur_summary, cur_summary_lto,
 				 NULL, NULL,
-				 node->decl, e->indirect_info->ecf_flags);
+				 node->decl,
+				 e,
+				 caller_ecf_flags,
+				 e->indirect_info->ecf_flags,
+				 false);
 	    }
 
 	  if (!cur_summary && !cur_summary_lto)
@@ -4216,7 +4244,11 @@  modref_propagate_flags_in_scc (cgraph_node *component_node)
 	      changed |= modref_merge_call_site_flags
 				(sum, cur_summary, cur_summary_lto,
 				 callee_summary, callee_summary_lto,
-				 node->decl, ecf_flags);
+				 node->decl,
+				 callee_edge,
+				 caller_ecf_flags,
+				 ecf_flags,
+				 callee->binds_to_current_def_p ());
 	      if (dump_file && changed)
 		{
 		  if (cur_summary)
diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
index ddc86869069..20170a65ded 100644
--- a/gcc/ipa-modref.h
+++ b/gcc/ipa-modref.h
@@ -58,4 +58,33 @@  static const int implicit_pure_eaf_flags = EAF_NOCLOBBER | EAF_NOESCAPE
 static const int ignore_stores_eaf_flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE
 				    | EAF_NODIRECTESCAPE;
 
+/* If function does not bind to current def (i.e. it is inline in comdat
+   section), the modref analysis may not match the behaviour of function
+   which will be later symbol interposed to.  All side effects must match
+   however it is possible that the other function body contains more loads
+   which may trap.
+   MODREF_FLAGS are flags determined by analysis of function body while
+   FLAGS are flags known otherwise (i.e. by fnspec, pure/const attributes
+   etc.)  */
+static inline int
+interposable_eaf_flags (int modref_flags, int flags)
+{
+  /* If parameter was previously unused, we know it is only read
+     and its value is not used.  */
+  if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
+    {
+      modref_flags &= ~EAF_UNUSED;
+      modref_flags |= EAF_NOESCAPE | EAF_NOT_RETURNED
+		      | EAF_NOT_RETURNED_DIRECTLY | EAF_NOCLOBBER;
+    }
+  /* We can not deterine that value is not read at all.  */
+  if ((modref_flags & EAF_NOREAD) && !(flags & EAF_NOREAD))
+    modref_flags &= ~EAF_NOREAD;
+  /* Clear direct flags so we also know that value is possibly read
+     indirectly.  */
+  if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
+    modref_flags &= ~EAF_DIRECT;
+  return modref_flags;
+}
+
 #endif