Distinguish global and unkonwn memory accesses in ipa-modref

Message ID 20211212081735.GC50931@kam.mff.cuni.cz
State Committed
Commit 3305135c29e1c3e988bd9bad40aefc01d138aaca
Headers
Series Distinguish global and unkonwn memory accesses in ipa-modref |

Commit Message

Jan Hubicka Dec. 12, 2021, 8:17 a.m. UTC
  Hi,
As discussed in PR103585, fatigue2 is now only benchmark from my usual testing
set (SPEC2k6, SPEC2k17, CPP benchmarks, polyhedron, Firefox, clang) which sees
important regression when inlining functions called once is limited.  This
prevents us from solving runtime issues in roms benchmarks and elsewhere.

The problem is that there is perdida function that takes many arguments and
some of them are array descriptors.  We constant propagate most of their fields
but still keep their initialization. Because perdida is quite fast, the call
overhead dominates, since we need over 100 memory stores consuing about 35%
of the overall benchmark runtime.

The memory stores would be eliminated if perdida did not call fortran I/O which
makes modref to thin that the array descriptors could be accessed. We are
quite close discovering that they can't becuase they are non-escaping from
function.  This patch makes modref to distingush between global memory access
(only things that escapes) and unkonwn accesss (that may access also
nonescaping things reaching the function).  This makes disambiguation for
functions containing error handling better.

Unfortunately the patch hits two semi-latent issues in Fortran frontned.
First is wrong code in gfortran.dg/unlimited_polymorphic_3.f03. This can be
turned into wrong code testcase on both mainline and gcc11 if the runtime
call is removed, so I filled PR 103662 for it. There is TBAA mismatch for
structure produced in FE.

Second is issue with GOMP where Fortran marks certain parameters as
non-escaping and then makes them escape via GOMP_parallel.  For this I
disabled the use of escape info in verify_arg which also disables the
useful transform on perdida (since we now punt on any call that takes
non-constant parameters) but still does useful work for e.g. GCC error
handling where we call my_friendly_abort.  I will work on this
incrementally.

Bootstrapped/regtested x86_64-linux, lto-bootstrapped and also tested with
clang build.  I plan to commit this tomorrow if there are no complains
(the patch is not completely short but conceptualy simple and handles a lot
of common cases).


gcc/ChangeLog:

2021-12-12  Jan Hubicka  <hubicka@ucw.cz>

	PR ipa/103585
	* ipa-modref-tree.c (modref_access_node::range_info_useful_p): Handle
	MODREF_GLOBAL_MEMORY_PARM.
	(modref_access_node::dump): Likewise.
	(modref_access_node::get_call_arg): Likewise.
	* ipa-modref-tree.h (enum modref_special_parms): Add
	MODREF_GLOBAL_MEMORY_PARM.
	(modref_access_node::useful_for_kill): Handle
	MODREF_GLOBAL_MEMORY_PARM.
	(modref:tree::merge): Add promote_unknown_to_global.
	* ipa-modref.c (verify_arg):New function.
	(may_access_nonescaping_parm_p): New function.
	(modref_access_analysis::record_global_memory_load): New member
	function.
	(modref_access_analysis::record_global_memory_store): Likewise.
	(modref_access_analysis::process_fnspec): Distingush global and local
	memory.
	(modref_access_analysis::analyze_call): Likewise.
	* tree-ssa-alias.c (ref_may_access_global_memory_p): New function.
	(modref_may_conflict): Use it.

gcc/testsuite/ChangeLog:

2021-12-12  Jan Hubicka  <hubicka@ucw.cz>

	* gcc.dg/analyzer/data-model-1.c: Disable ipa-modref.
	* gcc.dg/uninit-38.c: Likewise.
	* gcc.dg/uninit-pr98578.c: Liewise.
  

Patch

diff --git a/gcc/ipa-modref-tree.c b/gcc/ipa-modref-tree.c
index a868e3e3983..64ef0772343 100644
--- a/gcc/ipa-modref-tree.c
+++ b/gcc/ipa-modref-tree.c
@@ -36,7 +36,8 @@  modref_access_node::operator == (modref_access_node &a) const
 {
   if (parm_index != a.parm_index)
     return false;
-  if (parm_index != MODREF_UNKNOWN_PARM)
+  if (parm_index != MODREF_UNKNOWN_PARM
+      && parm_index != MODREF_GLOBAL_MEMORY_PARM)
     {
       if (parm_offset_known != a.parm_offset_known)
 	return false;
@@ -613,7 +614,9 @@  modref_access_node::insert (vec <modref_access_node, va_gc> *&accesses,
 bool
 modref_access_node::range_info_useful_p () const
 {
-  return parm_index != MODREF_UNKNOWN_PARM && parm_offset_known
+  return parm_index != MODREF_UNKNOWN_PARM
+	 && parm_index != MODREF_GLOBAL_MEMORY_PARM
+	 && parm_offset_known
 	 && (known_size_p (size)
 	     || known_size_p (max_size)
 	     || known_ge (offset, 0));
@@ -625,7 +628,9 @@  modref_access_node::dump (FILE *out)
 {
   if (parm_index != MODREF_UNKNOWN_PARM)
     {
-      if (parm_index >= 0)
+      if (parm_index == MODREF_GLOBAL_MEMORY_PARM)
+	fprintf (out, " Base in global memory");
+      else if (parm_index >= 0)
 	fprintf (out, " Parm %i", parm_index);
       else if (parm_index == MODREF_STATIC_CHAIN_PARM)
 	fprintf (out, " Static chain");
@@ -655,7 +660,8 @@  modref_access_node::dump (FILE *out)
 tree
 modref_access_node::get_call_arg (const gcall *stmt) const
 {
-  if (parm_index == MODREF_UNKNOWN_PARM)
+  if (parm_index == MODREF_UNKNOWN_PARM
+      || parm_index == MODREF_GLOBAL_MEMORY_PARM)
     return NULL;
   if (parm_index == MODREF_STATIC_CHAIN_PARM)
     return gimple_call_chain (stmt);
diff --git a/gcc/ipa-modref-tree.h b/gcc/ipa-modref-tree.h
index 4ad556fbe36..94fcebda468 100644
--- a/gcc/ipa-modref-tree.h
+++ b/gcc/ipa-modref-tree.h
@@ -48,10 +48,12 @@  enum modref_special_parms {
   MODREF_UNKNOWN_PARM = -1,
   MODREF_STATIC_CHAIN_PARM = -2,
   MODREF_RETSLOT_PARM = -3,
+  /* Used for bases that points to memory that escapes from function.  */
+  MODREF_GLOBAL_MEMORY_PARM = -4,
   /* Used in modref_parm_map to tak references which can be removed
      from the summary during summary update since they now points to loca
      memory.  */
-  MODREF_LOCAL_MEMORY_PARM = -4
+  MODREF_LOCAL_MEMORY_PARM = -5
 };
 
 /* Modref record accesses relative to function parameters.
@@ -86,6 +88,7 @@  struct GTY(()) modref_access_node
   bool useful_for_kill_p () const
     {
       return parm_offset_known && parm_index != MODREF_UNKNOWN_PARM
+	     && parm_index != MODREF_GLOBAL_MEMORY_PARM
 	     && parm_index != MODREF_RETSLOT_PARM && known_size_p (size)
 	     && known_eq (max_size, size)
 	     && known_gt (size, 0);
@@ -175,6 +178,7 @@  struct GTY((user)) modref_ref_node
        in the caller.  */
     gcc_checking_assert (a.parm_index >= 0
 			 || a.parm_index == MODREF_STATIC_CHAIN_PARM
+			 || a.parm_index == MODREF_GLOBAL_MEMORY_PARM
 			 || a.parm_index == MODREF_UNKNOWN_PARM);
 
     if (!a.useful_p ())
@@ -524,7 +528,8 @@  struct GTY((user)) modref_tree
 	      unsigned int max_accesses,
 	      modref_tree <T> *other, vec <modref_parm_map> *parm_map,
 	      modref_parm_map *static_chain_map,
-	      bool record_accesses)
+	      bool record_accesses,
+	      bool promote_unknown_to_global = false)
   {
     if (!other || every_base)
       return false;
@@ -579,7 +584,9 @@  struct GTY((user)) modref_tree
 		  {
 		    modref_access_node a = *access_node;
 
-		    if (a.parm_index != MODREF_UNKNOWN_PARM && parm_map)
+		    if (a.parm_index != MODREF_UNKNOWN_PARM
+			&& a.parm_index != MODREF_GLOBAL_MEMORY_PARM
+			&& parm_map)
 		      {
 			if (a.parm_index >= (int)parm_map->length ())
 			  a.parm_index = MODREF_UNKNOWN_PARM;
@@ -596,6 +603,9 @@  struct GTY((user)) modref_tree
 			    a.parm_index = m.parm_index;
 			  }
 		      }
+		    if (a.parm_index == MODREF_UNKNOWN_PARM
+			&& promote_unknown_to_global)
+		      a.parm_index = MODREF_GLOBAL_MEMORY_PARM;
 		    changed |= insert (max_bases, max_refs, max_accesses,
 				       base_node->base, ref_node->ref,
 				       a, record_accesses);
@@ -614,12 +624,14 @@  struct GTY((user)) modref_tree
   bool merge (tree fndecl,
 	      modref_tree <T> *other, vec <modref_parm_map> *parm_map,
 	      modref_parm_map *static_chain_map,
-	      bool record_accesses)
+	      bool record_accesses,
+	      bool promote_unknown_to_global = false)
   {
      return merge (opt_for_fn (fndecl, param_modref_max_bases),
 		   opt_for_fn (fndecl, param_modref_max_refs),
 		   opt_for_fn (fndecl, param_modref_max_accesses),
-		   other, parm_map, static_chain_map, record_accesses);
+		   other, parm_map, static_chain_map, record_accesses,
+		   promote_unknown_to_global);
   }
 
   /* Copy OTHER to THIS.  */
@@ -657,7 +669,8 @@  struct GTY((user)) modref_tree
 	    if (ref_node->every_access)
 	      return true;
 	    FOR_EACH_VEC_SAFE_ELT (ref_node->accesses, k, access_node)
-	      if (access_node->parm_index == MODREF_UNKNOWN_PARM)
+	      if (access_node->parm_index == MODREF_UNKNOWN_PARM
+		  || access_node->parm_index == MODREF_GLOBAL_MEMORY_PARM)
 		return true;
 	  }
       }
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 55fa873e1f0..e290b149754 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -869,6 +869,66 @@  parm_map_for_ptr (tree op)
   return parm_map;
 }
 
+/* Return true if ARG with EAF flags FLAGS can not make any caller's parameter
+   used (if LOAD is true we check loads, otherwise stores).  */
+
+static bool
+verify_arg (tree arg, int flags, bool load)
+{
+  if (flags & EAF_UNUSED)
+    return true;
+  if (load && (flags & EAF_NO_DIRECT_READ))
+    return true;
+  if (!load
+      && (flags & (EAF_NO_DIRECT_CLOBBER | EAF_NO_INDIRECT_CLOBBER))
+	  == (EAF_NO_DIRECT_CLOBBER | EAF_NO_INDIRECT_CLOBBER))
+    return true;
+  if (is_gimple_constant (arg))
+    return true;
+  if (DECL_P (arg) && TREE_READONLY (arg))
+    return true;
+  if (TREE_CODE (arg) == ADDR_EXPR)
+    {
+      tree t = get_base_address (TREE_OPERAND (arg, 0));
+      if (is_gimple_constant (t))
+	return true;
+      if (DECL_P (t)
+	  && (TREE_READONLY (t) || TREE_CODE (t) == FUNCTION_DECL))
+	return true;
+    }
+  return false;
+}
+
+/* Return true if STMT may access memory that is pointed to by parameters
+   of caller and which is not seen as an escape by PTA.
+   CALLEE_ECF_FLAGS are ECF flags of callee.  If LOAD is true then by access
+   we mean load, otherwise we mean store.  */
+
+static bool
+may_access_nonescaping_parm_p (gcall *call, int callee_ecf_flags, bool load)
+{
+  int implicit_flags = 0;
+
+  if (ignore_stores_p (current_function_decl, callee_ecf_flags))
+    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;
+  if (gimple_call_chain (call)
+      && !verify_arg (gimple_call_chain (call),
+		      gimple_call_static_chain_flags (call) | implicit_flags,
+		      load))
+    return true;
+  for (unsigned int i = 0; i < gimple_call_num_args (call); i++)
+    if (!verify_arg (gimple_call_arg (call, i),
+		     gimple_call_arg_flags (call, i) | implicit_flags,
+		     load))
+      return true;
+  return false;
+}
+
+
 /* Analyze memory accesses (loads, stores and kills) performed
    by the function.  Set also side_effects, calls_interposable
    and nondeterminism flags.  */
@@ -892,6 +952,8 @@  private:
   bool record_access_p (tree);
   bool record_unknown_load ();
   bool record_unknown_store ();
+  bool record_global_memory_load ();
+  bool record_global_memory_store ();
   bool merge_call_side_effects (gimple *, modref_summary *,
 				cgraph_node *, bool);
   modref_access_node get_access_for_fnspec (gcall *, attr_fnspec &,
@@ -1147,6 +1209,41 @@  modref_access_analysis::record_unknown_store ()
   return changed;
 }
 
+/* Record unknown load from gloal memory.  */
+
+bool
+modref_access_analysis::record_global_memory_load ()
+{
+  bool changed = false;
+  modref_access_node a = {0, -1, -1,
+			  0, MODREF_GLOBAL_MEMORY_PARM, false, 0};
+
+  if (m_summary && !m_summary->loads->every_base)
+    changed |= m_summary->loads->insert (current_function_decl, 0, 0, a, false);
+  if (m_summary_lto && !m_summary_lto->loads->every_base)
+    changed |= m_summary_lto->loads->insert (current_function_decl,
+					     0, 0, a, false);
+  return changed;
+}
+
+/* Record unknown store from gloal memory.  */
+
+bool
+modref_access_analysis::record_global_memory_store ()
+{
+  bool changed = false;
+  modref_access_node a = {0, -1, -1,
+			  0, MODREF_GLOBAL_MEMORY_PARM, false, 0};
+
+  if (m_summary && !m_summary->stores->every_base)
+    changed |= m_summary->stores->insert (current_function_decl,
+					  0, 0, a, false);
+  if (m_summary_lto && !m_summary_lto->stores->every_base)
+    changed |= m_summary_lto->stores->insert (current_function_decl,
+					     0, 0, a, false);
+  return changed;
+}
+
 /* Merge side effects of call STMT to function with CALLEE_SUMMARY.
    Return true if something changed.
    If IGNORE_STORES is true, do not merge stores.
@@ -1158,7 +1255,8 @@  modref_access_analysis::merge_call_side_effects
 	 (gimple *stmt, modref_summary *callee_summary,
 	  cgraph_node *callee_node, bool record_adjustments)
 {
-  int flags = gimple_call_flags (stmt);
+  gcall *call = as_a <gcall *> (stmt);
+  int flags = gimple_call_flags (call);
 
   /* Nothing to do for non-looping cont functions.  */
   if ((flags & (ECF_CONST | ECF_NOVOPS))
@@ -1221,10 +1319,10 @@  modref_access_analysis::merge_call_side_effects
     fprintf (dump_file, "   Parm map:");
 
   auto_vec <modref_parm_map, 32> parm_map;
-  parm_map.safe_grow_cleared (gimple_call_num_args (stmt), true);
-  for (unsigned i = 0; i < gimple_call_num_args (stmt); i++)
+  parm_map.safe_grow_cleared (gimple_call_num_args (call), true);
+  for (unsigned i = 0; i < gimple_call_num_args (call); i++)
     {
-      parm_map[i] = parm_map_for_ptr (gimple_call_arg (stmt, i));
+      parm_map[i] = parm_map_for_ptr (gimple_call_arg (call, i));
       if (dump_file)
 	{
 	  fprintf (dump_file, " %i", parm_map[i].parm_index);
@@ -1238,9 +1336,9 @@  modref_access_analysis::merge_call_side_effects
     }
 
   modref_parm_map chain_map;
-  if (gimple_call_chain (stmt))
+  if (gimple_call_chain (call))
     {
-      chain_map = parm_map_for_ptr (gimple_call_chain (stmt));
+      chain_map = parm_map_for_ptr (gimple_call_chain (call));
       if (dump_file)
 	{
 	  fprintf (dump_file, "static chain %i", chain_map.parm_index);
@@ -1260,7 +1358,7 @@  modref_access_analysis::merge_call_side_effects
   if (m_always_executed
       && callee_summary->kills.length ()
       && (!cfun->can_throw_non_call_exceptions
-	  || !stmt_could_throw_p (cfun, stmt)))
+	  || !stmt_could_throw_p (cfun, call)))
     {
       /* Watch for self recursive updates.  */
       auto_vec<modref_access_node, 32> saved_kills;
@@ -1293,14 +1391,18 @@  modref_access_analysis::merge_call_side_effects
   changed |= m_summary->loads->merge (current_function_decl,
 				      callee_summary->loads,
 				      &parm_map, &chain_map,
-				      record_adjustments);
+				      record_adjustments,
+				      !may_access_nonescaping_parm_p
+					 (call, flags, true));
   /* Merge in stores.  */
   if (!ignore_stores_p (current_function_decl, flags))
     {
       changed |= m_summary->stores->merge (current_function_decl,
 					   callee_summary->stores,
 					   &parm_map, &chain_map,
-					   record_adjustments);
+					   record_adjustments,
+					   !may_access_nonescaping_parm_p
+					       (call, flags, false));
       if (!m_summary->writes_errno
 	  && callee_summary->writes_errno)
 	{
@@ -1348,7 +1450,6 @@  modref_access_analysis::get_access_for_fnspec (gcall *call, attr_fnspec &fnspec,
     }
   return a;
 }
-
 /* Apply side effects of call STMT to CUR_SUMMARY using FNSPEC.
    If IGNORE_STORES is true ignore them.
    Return false if no useful summary can be produced.   */
@@ -1381,14 +1482,34 @@  modref_access_analysis::process_fnspec (gcall *call)
       if (dump_file && gimple_call_builtin_p (call, BUILT_IN_NORMAL))
 	fprintf (dump_file, "      Builtin with no fnspec: %s\n",
 		 IDENTIFIER_POINTER (DECL_NAME (gimple_call_fndecl (call))));
-      record_unknown_load ();
       if (!ignore_stores_p (current_function_decl, flags))
-	record_unknown_store ();
+	{
+	  if (!may_access_nonescaping_parm_p (call, flags, false))
+	    record_global_memory_store ();
+	  else
+	    record_unknown_store ();
+	  if (!may_access_nonescaping_parm_p (call, flags, true))
+	    record_global_memory_load ();
+	  else
+	    record_unknown_load ();
+	}
+      else
+	{
+	  if (!may_access_nonescaping_parm_p (call, flags, true))
+	    record_global_memory_load ();
+	  else
+	    record_unknown_load ();
+	}
       return;
     }
   /* Process fnspec.  */
   if (fnspec.global_memory_read_p ())
-    record_unknown_load ();
+    {
+      if (may_access_nonescaping_parm_p (call, flags, true))
+	record_unknown_load ();
+      else
+	record_global_memory_load ();
+    }
   else
     {
       for (unsigned int i = 0; i < gimple_call_num_args (call); i++)
@@ -1420,7 +1541,12 @@  modref_access_analysis::process_fnspec (gcall *call)
   if (ignore_stores_p (current_function_decl, flags))
     return;
   if (fnspec.global_memory_written_p ())
-    record_unknown_store ();
+    {
+      if (may_access_nonescaping_parm_p (call, flags, false))
+	record_unknown_store ();
+      else
+	record_global_memory_store ();
+    }
   else
     {
       for (unsigned int i = 0; i < gimple_call_num_args (call); i++)
@@ -1468,6 +1594,12 @@  modref_access_analysis::analyze_call (gcall *stmt)
      simplified.  */
   int flags = gimple_call_flags (stmt);
 
+  if (dump_file)
+    {
+      fprintf (dump_file, " - Analyzing call:");
+      print_gimple_stmt (dump_file, stmt, 0);
+    }
+
   if ((flags & (ECF_CONST | ECF_NOVOPS))
       && !(flags & ECF_LOOPING_CONST_OR_PURE))
     {
diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
index 908d99981a6..511ed4b5297 100644
--- a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
@@ -1,4 +1,5 @@ 
 /* { dg-require-effective-target alloca } */
+/* { dg-additional-options "-fno-ipa-modref" } */
 
 #include <stdlib.h>
 #include <string.h>
diff --git a/gcc/testsuite/gcc.dg/uninit-38.c b/gcc/testsuite/gcc.dg/uninit-38.c
index 0d70bcd8e98..ff2aee6cccf 100644
--- a/gcc/testsuite/gcc.dg/uninit-38.c
+++ b/gcc/testsuite/gcc.dg/uninit-38.c
@@ -6,7 +6,7 @@ 
    be adjusted.  Ditto if -Wuninitialized output changes for some
    other reason.
    { dg-do compile { target { { lp64 || ilp32 } || llp64 } } }
-   { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */
+   { dg-options "-O2 -Wall -ftrack-macro-expansion=0 -fno-ipa-modref" } */
 
 #define CONCAT(x, y)   x ## y
 #define CAT(x, y)      CONCAT(x, y)
diff --git a/gcc/testsuite/gcc.dg/uninit-pr98578.c b/gcc/testsuite/gcc.dg/uninit-pr98578.c
index 98d611757ab..745328b9f8d 100644
--- a/gcc/testsuite/gcc.dg/uninit-pr98578.c
+++ b/gcc/testsuite/gcc.dg/uninit-pr98578.c
@@ -1,6 +1,6 @@ 
 /* PR middle-end/98578 - ICE warning on uninitialized VLA access
    { dg-do compile }
-   { dg-options "-O2 -Wall" } */
+   { dg-options "-O2 -Wall -fno-ipa-modref" } */
 
 void* malloc (__SIZE_TYPE__);
 
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 88fd7821c5e..011298998b7 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -2544,6 +2544,30 @@  refs_output_dependent_p (tree store1, tree store2)
   return refs_may_alias_p_1 (&r1, &r2, false);
 }
 
+/* Return ture if REF may access global memory.  */
+
+bool
+ref_may_access_global_memory_p (ao_ref *ref)
+{
+  if (!ref->ref)
+    return true;
+  tree base = ao_ref_base (ref);
+  if (TREE_CODE (base) == MEM_REF
+      || TREE_CODE (base) == TARGET_MEM_REF)
+    {
+      if (ptr_deref_may_alias_global_p (TREE_OPERAND (base, 0)))
+	return true;
+    }
+  else
+    {
+      if (!auto_var_in_fn_p (base, current_function_decl)
+	  || pt_solution_includes (&cfun->gimple_df->escaped,
+				   base))
+	return true;
+    }
+  return false;
+}
+
 /* Returns true if and only if REF may alias any access stored in TT.
    IF TBAA_P is true, use TBAA oracle.  */
 
@@ -2552,6 +2576,7 @@  modref_may_conflict (const gcall *stmt,
 		     modref_tree <alias_set_type> *tt, ao_ref *ref, bool tbaa_p)
 {
   alias_set_type base_set, ref_set;
+  bool global_memory_ok = false;
 
   if (tt->every_base)
     return true;
@@ -2602,6 +2627,17 @@  modref_may_conflict (const gcall *stmt,
 	      if (num_tests >= max_tests)
 		return true;
 
+	      if (access_node.parm_index == MODREF_GLOBAL_MEMORY_PARM)
+		{
+		  if (global_memory_ok)
+		    continue;
+		  if (ref_may_access_global_memory_p (ref))
+		    return true;
+		  global_memory_ok = true;
+		  num_tests++;
+		  continue;
+		}
+
 	      tree arg = access_node.get_call_arg (stmt);
 	      if (!arg)
 		return true;