[TCWG CI] Regression caused by gcc: handle retslot in modref

public inbox for gcc-regression@sourceware.org
help / color / mirror / Atom feed
* [TCWG CI] Regression caused by gcc: handle retslot in modref
@ 2021-10-30  2:32 ci_notify
  2021-11-03 11:33 ` Maxim Kuvyrkov
  0 siblings, 1 reply; 5+ messages in thread
From: ci_notify @ 2021-10-30  2:32 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-regression

[TCWG CI] Regression caused by gcc: handle retslot in modref:
commit b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
Author: Jan Hubicka <hubicka@ucw.cz>

    handle retslot in modref

Results regressed to
# reset_artifacts:
-10
# true:
0
# build_abe binutils:
1
# First few build errors in logs:
# 00:04:02 make[3]: [Makefile:1772: aarch64-unknown-linux-gnu/bits/largefile-config.h] Error 1 (ignored)
# 00:04:02 make[3]: [Makefile:1773: aarch64-unknown-linux-gnu/bits/largefile-config.h] Error 1 (ignored)
# 00:19:19 checking for suffix of object files... configure: error: in `/home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gcc-gcc.git~master-stage2/aarch64-unknown-linux-gnu/libgcc':
# 00:19:19 configure: error: cannot compute suffix of object files: cannot compile
# 00:19:19 make[2]: *** [Makefile:19796: configure-stage2-target-libgcc] Error 1
# 00:19:19 make[1]: *** [Makefile:25351: stage2-bubble] Error 2
# 00:19:19 make: *** [Makefile:1019: all] Error 2

from
# reset_artifacts:
-10
# true:
0
# build_abe binutils:
1
# build_abe bootstrap_lto:
2

THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.

This commit has regressed these CI configurations:
 - tcwg_gcc_bootstrap/master-aarch64-bootstrap_lto

First_bad build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9/
Last_good build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-4045d5fa42f2ee7b284977c8f2f0edc300a63e43/
Baseline build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-baseline/
Even more details: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/

Reproduce builds:
<cut>
mkdir investigate-gcc-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
cd investigate-gcc-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9

# Fetch scripts
git clone https://git.linaro.org/toolchain/jenkins-scripts

# Fetch manifests and test.sh script
mkdir -p artifacts/manifests
curl -o artifacts/manifests/build-baseline.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/manifests/build-baseline.sh --fail
curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/manifests/build-parameters.sh --fail
curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/test.sh --fail
chmod +x artifacts/test.sh

# Reproduce the baseline build (build all pre-requisites)
./jenkins-scripts/tcwg_gnu-build.sh @@ artifacts/manifests/build-baseline.sh

# Save baseline build state (which is then restored in artifacts/test.sh)
mkdir -p ./bisect
rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ --exclude /gcc/ ./ ./bisect/baseline/

cd gcc

# Reproduce first_bad build
git checkout --detach b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
../artifacts/test.sh

# Reproduce last_good build
git checkout --detach 4045d5fa42f2ee7b284977c8f2f0edc300a63e43
../artifacts/test.sh

cd ..
</cut>

Full commit (up to 1000 lines):
<cut>
commit b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
Author: Jan Hubicka <hubicka@ucw.cz>
Date:   Fri Oct 29 16:01:51 2021 +0200

    handle retslot in modref
    
    Extend modref and tree-ssa-structalias to handle retslot flags.
    Since retslot it essentially a hidden argument that is known to be write-only
    we can do pretty much the same stuff as we do for regular parameters.
    I plan to add static chain handling similar way.
    
    We do not handle IPA propagation of retslot flags (where return slot is
    initialized via return slot of other function). For this ipa-prop needs
    to be extended to understand retslot as well.
    
    Bootstrapped/regtested x86_64-linux, OK for the gimple bits?
    
    Honza
    
    gcc/ChangeLog:
    
            * gimple.c (gimple_call_retslot_flags): New function.
            * gimple.h (gimple_call_retslot_flags): Declare.
            * ipa-modref.c: Include tree-cfg.h.
            (struct escape_entry): Turn parm_index to signed.
            (modref_summary_lto::modref_summary_lto): Add retslot_flags.
            (modref_summary::modref_summary): Initialize retslot_flags.
            (struct modref_summary_lto): Likewise.
            (modref_summary::useful_p): Check retslot_flags.
            (modref_summary_lto::useful_p): Likewise.
            (modref_summary::dump): Dump retslot_flags.
            (modref_summary_lto::dump): Likewise.
            (struct escape_point): Add hidden_args enum.
            (analyze_ssa_name_flags): Ignore return slot return;
            use gimple_call_retslot_flags.
            (record_escape_points): Break out from ...
            (analyze_parms): ... here; handle retslot_flags.
            (modref_summaries::duplicate): Duplicate retslot_flags.
            (modref_summaries_lto::duplicate): Likewise.
            (modref_write_escape_summary): Stream parm_index as signed.
            (modref_read_escape_summary): Likewise.
            (modref_write): Stream retslot_flags.
            (read_section): Likewise.
            (struct escape_map): Fix typo in comment.
            (update_escape_summary_1): Fix whitespace.
            (ipa_merge_modref_summary_after_inlining): Drop retslot_flags.
            (modref_merge_call_site_flags): Merge retslot_flags.
            * ipa-modref.h (struct modref_summary): Add retslot_flags.
            * tree-ssa-structalias.c (handle_rhs_call): Handle retslot_flags.
---
 gcc/gimple.c               |  41 ++++++++++++-
 gcc/gimple.h               |   1 +
 gcc/ipa-modref.c           | 145 +++++++++++++++++++++++++++++++++------------
 gcc/ipa-modref.h           |   1 +
 gcc/tree-ssa-structalias.c |  29 ++++++---
 5 files changed, 170 insertions(+), 47 deletions(-)

diff --git a/gcc/gimple.c b/gcc/gimple.c
index cc7a88e822b..22dd6417d19 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -1597,7 +1597,12 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
 	  if (!node->binds_to_current_def_p ())
 	    {
 	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
-		modref_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;
 	    }
@@ -1608,6 +1613,40 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
   return flags;
 }
 
+/* Detects argument flags for return slot on call STMT.  */
+
+int
+gimple_call_retslot_flags (const gcall *stmt)
+{
+  int flags = EAF_DIRECT | EAF_NOREAD;
+
+  tree callee = gimple_call_fndecl (stmt);
+  if (callee)
+    {
+      cgraph_node *node = cgraph_node::get (callee);
+      modref_summary *summary = node ? get_modref_function_summary (node)
+				: NULL;
+
+      if (summary)
+	{
+	  int modref_flags = summary->retslot_flags;
+
+	  /* 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 (dbg_cnt (ipa_mod_ref_pta))
+	    flags |= modref_flags;
+	}
+    }
+  return flags;
+}
+
 /* Detects return flags for the call STMT.  */
 
 int
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 303623b3ced..23a124ec769 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1589,6 +1589,7 @@ gimple_seq gimple_seq_copy (gimple_seq);
 bool gimple_call_same_target_p (const gimple *, const gimple *);
 int gimple_call_flags (const gimple *);
 int gimple_call_arg_flags (const gcall *, unsigned);
+int gimple_call_retslot_flags (const gcall *);
 int gimple_call_return_flags (const gcall *);
 bool gimple_call_nonnull_result_p (gcall *);
 tree gimple_call_nonnull_arg (gcall *);
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 0bbec8df0a2..3539cb43d11 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -86,6 +86,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "stringpool.h"
 #include "tree-ssanames.h"
 #include "attribs.h"
+#include "tree-cfg.h"
 
 
 namespace {
@@ -133,7 +134,7 @@ static fnspec_summaries_t *fnspec_summaries = NULL;
 struct escape_entry
 {
   /* Parameter that escapes at a given call.  */
-  unsigned int parm_index;
+  int parm_index;
   /* Argument it escapes to.  */
   unsigned int arg;
   /* Minimal flags known about the argument.  */
@@ -269,7 +270,7 @@ static GTY(()) fast_function_summary <modref_summary_lto *, va_gc>
 /* Summary for a single function which this pass produces.  */
 
 modref_summary::modref_summary ()
-  : loads (NULL), stores (NULL), writes_errno (NULL)
+  : loads (NULL), stores (NULL), retslot_flags (0), writes_errno (false)
 {
 }
 
@@ -322,6 +323,8 @@ modref_summary::useful_p (int ecf_flags, bool check_flags)
   if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
     return true;
   arg_flags.release ();
+  if (check_flags && remove_useless_eaf_flags (retslot_flags, ecf_flags, false))
+    return true;
   if (ecf_flags & ECF_CONST)
     return false;
   if (loads && !loads->every_base)
@@ -363,6 +366,7 @@ struct GTY(()) modref_summary_lto
   modref_records_lto *loads;
   modref_records_lto *stores;
   auto_vec<eaf_flags_t> GTY((skip)) arg_flags;
+  eaf_flags_t retslot_flags;
   bool writes_errno;
 
   modref_summary_lto ();
@@ -374,7 +378,7 @@ struct GTY(()) modref_summary_lto
 /* Summary for a single function which this pass produces.  */
 
 modref_summary_lto::modref_summary_lto ()
-  : loads (NULL), stores (NULL), writes_errno (NULL)
+  : loads (NULL), stores (NULL), retslot_flags (0), writes_errno (false)
 {
 }
 
@@ -400,6 +404,8 @@ modref_summary_lto::useful_p (int ecf_flags, bool check_flags)
   if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
     return true;
   arg_flags.release ();
+  if (check_flags && remove_useless_eaf_flags (retslot_flags, ecf_flags, false))
+    return true;
   if (ecf_flags & ECF_CONST)
     return false;
   if (loads && !loads->every_base)
@@ -608,6 +614,11 @@ modref_summary::dump (FILE *out)
 	    dump_eaf_flags (out, arg_flags[i]);
 	  }
     }
+  if (retslot_flags)
+    {
+      fprintf (out, "  Retslot flags:");
+      dump_eaf_flags (out, retslot_flags);
+    }
 }
 
 /* Dump summary.  */
@@ -630,6 +641,11 @@ modref_summary_lto::dump (FILE *out)
 	    dump_eaf_flags (out, arg_flags[i]);
 	  }
     }
+  if (retslot_flags)
+    {
+      fprintf (out, "  Retslot flags:");
+      dump_eaf_flags (out, retslot_flags);
+    }
 }
 
 /* Get function summary for FUNC if it exists, return NULL otherwise.  */
@@ -1396,6 +1412,11 @@ namespace {
 
 struct escape_point
 {
+  /* Extra hidden args we keep track of.  */
+  enum hidden_args
+  {
+    retslot_arg = -1
+  };
   /* Value escapes to this call.  */
   gcall *call;
   /* Argument it escapes to.  */
@@ -1705,7 +1726,11 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
 	 Returning name counts as an use by tree-ssa-structalias.c  */
       if (greturn *ret = dyn_cast <greturn *> (use_stmt))
 	{
-	  if (gimple_return_retval (ret) == name)
+	  /* Returning through return slot is seen as memory write earlier.  */
+	  if (DECL_RESULT (current_function_decl)
+	      && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
+	    ;
+	  else if (gimple_return_retval (ret) == name)
 	    lattice[index].merge (~(EAF_UNUSED | EAF_NOT_RETURNED));
 	  else if (memory_access_to (gimple_return_retval (ret), name))
 	    {
@@ -1748,7 +1773,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
 		     may make LHS to escape.  See PR 98499.  */
 		  if (gimple_call_return_slot_opt_p (call)
 		      && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (call))))
-		    lattice[index].merge (EAF_NOREAD | EAF_DIRECT);
+		    lattice[index].merge (gimple_call_retslot_flags (call));
 		}
 
 	      /* We do not track accesses to the static chain (we could)
@@ -1777,7 +1802,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
 			  lattice[index].merge (call_flags);
 			else
 			  lattice[index].add_escape_point (call, i,
-			     				   call_flags, true);
+							   call_flags, true);
 		      }
 		    if (!ignore_retval)
 		      merge_call_lhs_flags (call, i, index, false,
@@ -1912,6 +1937,29 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
   lattice[index].known = true;
 }
 
+/* Record escape points of PARM_INDEX according to LATTICE.  */
+
+static void
+record_escape_points (modref_lattice &lattice, int parm_index, int flags)
+{
+  if (lattice.escape_points.length ())
+    {
+      escape_point *ep;
+      unsigned int ip;
+      cgraph_node *node = cgraph_node::get (current_function_decl);
+
+      FOR_EACH_VEC_ELT (lattice.escape_points, ip, ep)
+	if ((ep->min_flags & flags) != flags)
+	  {
+	    cgraph_edge *e = node->get_edge (ep->call);
+	    struct escape_entry ee = {parm_index, ep->arg,
+				      ep->min_flags, ep->direct};
+
+	    escape_summaries->get_create (e)->esc.safe_push (ee);
+	  }
+    }
+}
+
 /* Determine EAF flags for function parameters.  */
 
 static void
@@ -1921,16 +1969,22 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto,
   unsigned int parm_index = 0;
   unsigned int count = 0;
   int ecf_flags = flags_from_decl_or_type (current_function_decl);
+  tree retslot = 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)))
+    retslot = ssa_default_def (cfun, DECL_RESULT (current_function_decl));
+
   for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
        parm = TREE_CHAIN (parm))
     count++;
 
-  if (!count)
+  if (!count && !retslot)
     return;
 
   auto_vec<modref_lattice> lattice;
@@ -1984,24 +2038,24 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto,
 		summary_lto->arg_flags.safe_grow_cleared (count, true);
 	      summary_lto->arg_flags[parm_index] = flags;
 	    }
-	  if (lattice[SSA_NAME_VERSION (name)].escape_points.length ())
-	    {
-	      escape_point *ep;
-	      unsigned int ip;
-	      cgraph_node *node = cgraph_node::get (current_function_decl);
-
-	      gcc_checking_assert (ipa);
-	      FOR_EACH_VEC_ELT
-		  (lattice[SSA_NAME_VERSION (name)].escape_points, ip, ep)
-		if ((ep->min_flags & flags) != flags)
-		  {
-		    cgraph_edge *e = node->get_edge (ep->call);
-		    struct escape_entry ee = {parm_index, ep->arg,
-		     			      ep->min_flags, ep->direct};
+	  record_escape_points (lattice[SSA_NAME_VERSION (name)],
+				parm_index, flags);
+	}
+    }
+  if (retslot)
+    {
+      analyze_ssa_name_flags (retslot, lattice, 0, ipa);
+      int flags = lattice[SSA_NAME_VERSION (retslot)].flags;
 
-		    escape_summaries->get_create (e)->esc.safe_push (ee);
-		  }
-	    }
+      flags = remove_useless_eaf_flags (flags, ecf_flags, false);
+      if (flags)
+	{
+	  if (summary)
+	    summary->retslot_flags = flags;
+	  if (summary_lto)
+	    summary_lto->retslot_flags = flags;
+	  record_escape_points (lattice[SSA_NAME_VERSION (retslot)],
+				escape_point::retslot_arg, flags);
 	}
     }
   if (ipa)
@@ -2287,6 +2341,7 @@ modref_summaries::duplicate (cgraph_node *, cgraph_node *dst,
   dst_data->writes_errno = src_data->writes_errno;
   if (src_data->arg_flags.length ())
     dst_data->arg_flags = src_data->arg_flags.copy ();
+  dst_data->retslot_flags = src_data->retslot_flags;
 }
 
 /* Called when new clone is inserted to callgraph late.  */
@@ -2312,6 +2367,7 @@ modref_summaries_lto::duplicate (cgraph_node *, cgraph_node *,
   dst_data->writes_errno = src_data->writes_errno;
   if (src_data->arg_flags.length ())
     dst_data->arg_flags = src_data->arg_flags.copy ();
+  dst_data->retslot_flags = src_data->retslot_flags;
 }
 
 namespace
@@ -2551,7 +2607,7 @@ modref_write_escape_summary (struct bitpack_d *bp, escape_summary *esum)
   escape_entry *ee;
   FOR_EACH_VEC_ELT (esum->esc, i, ee)
     {
-      bp_pack_var_len_unsigned (bp, ee->parm_index);
+      bp_pack_var_len_int (bp, ee->parm_index);
       bp_pack_var_len_unsigned (bp, ee->arg);
       bp_pack_var_len_unsigned (bp, ee->min_flags);
       bp_pack_value (bp, ee->direct, 1);
@@ -2571,7 +2627,7 @@ modref_read_escape_summary (struct bitpack_d *bp, cgraph_edge *e)
   for (unsigned int i = 0; i < n; i++)
     {
       escape_entry ee;
-      ee.parm_index = bp_unpack_var_len_unsigned (bp);
+      ee.parm_index = bp_unpack_var_len_int (bp);
       ee.arg = bp_unpack_var_len_unsigned (bp);
       ee.min_flags = bp_unpack_var_len_unsigned (bp);
       ee.direct = bp_unpack_value (bp, 1);
@@ -2628,6 +2684,7 @@ modref_write ()
 	  streamer_write_uhwi (ob, r->arg_flags.length ());
 	  for (unsigned int i = 0; i < r->arg_flags.length (); i++)
 	    streamer_write_uhwi (ob, r->arg_flags[i]);
+	  streamer_write_uhwi (ob, r->retslot_flags);
 
 	  write_modref_records (r->loads, ob);
 	  write_modref_records (r->stores, ob);
@@ -2724,6 +2781,11 @@ read_section (struct lto_file_decl_data *file_data, const char *data,
 	  if (modref_sum_lto)
 	    modref_sum_lto->arg_flags.quick_push (flags);
 	}
+      eaf_flags_t flags = streamer_read_uhwi (&ib);
+      if (modref_sum)
+	modref_sum->retslot_flags = flags;
+      if (modref_sum_lto)
+	modref_sum_lto->retslot_flags = flags;
       read_modref_records (&ib, data_in,
 			   modref_sum ? &modref_sum->loads : NULL,
 			   modref_sum_lto ? &modref_sum_lto->loads : NULL);
@@ -3098,7 +3160,7 @@ struct escape_map
   bool direct;
 };
 
-/* Update escape map fo E.  */
+/* Update escape map for E.  */
 
 static void
 update_escape_summary_1 (cgraph_edge *e,
@@ -3117,7 +3179,10 @@ update_escape_summary_1 (cgraph_edge *e,
     {
       unsigned int j;
       struct escape_map *em;
-      if (ee->parm_index >= map.length ())
+      /* TODO: We do not have jump functions for return slots, so we
+	 never propagate them to outer function.  */
+      if (ee->parm_index >= (int)map.length ()
+	  || ee->parm_index < 0)
 	continue;
       FOR_EACH_VEC_ELT (map[ee->parm_index], j, em)
 	{
@@ -3125,7 +3190,7 @@ update_escape_summary_1 (cgraph_edge *e,
 	  if (ee->direct && !em->direct)
 	    min_flags = deref_flags (min_flags, ignore_stores);
 	  struct escape_entry entry = {em->parm_index, ee->arg,
-	    			       ee->min_flags,
+				       ee->min_flags,
 				       ee->direct & em->direct};
 	  sum->esc.safe_push (entry);
 	}
@@ -3245,7 +3310,11 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
     FOR_EACH_VEC_ELT (sum->esc, i, ee)
       {
 	bool needed = false;
-	if (to_info && to_info->arg_flags.length () > ee->parm_index)
+	/* TODO: We do not have jump functions for return slots, so we
+	   never propagate them to outer function.  */
+	if (ee->parm_index < 0)
+	  continue;
+	if (to_info && (int)to_info->arg_flags.length () > ee->parm_index)
 	  {
 	    int flags = callee_info
 			&& callee_info->arg_flags.length () > ee->arg
@@ -3259,7 +3328,7 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
 	    if (to_info->arg_flags[ee->parm_index])
 	      needed = true;
 	  }
-	if (to_info_lto && to_info_lto->arg_flags.length () > ee->parm_index)
+	if (to_info_lto && (int)to_info_lto->arg_flags.length () > ee->parm_index)
 	  {
 	    int flags = callee_info_lto
 			&& callee_info_lto->arg_flags.length () > ee->arg
@@ -3798,29 +3867,31 @@ modref_merge_call_site_flags (escape_summary *sum,
       if (flags_lto & EAF_NOESCAPE)
 	flags_lto |= EAF_NODIRECTESCAPE;
       if (!(flags & EAF_UNUSED)
-	  && cur_summary && ee->parm_index < cur_summary->arg_flags.length ())
+	  && cur_summary && ee->parm_index < (int)cur_summary->arg_flags.length ())
 	{
-	  int f = cur_summary->arg_flags[ee->parm_index];
+	  eaf_flags_t &f = ee->parm_index == escape_point::retslot_arg
+			   ? cur_summary->retslot_flags
+			   : cur_summary->arg_flags[ee->parm_index];
 	  if ((f & flags) != f)
 	    {
 	      f = remove_useless_eaf_flags
 			 (f & flags, ecf_flags,
 			  VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
-	      cur_summary->arg_flags[ee->parm_index] = f;
 	      changed = true;
 	    }
 	}
       if (!(flags_lto & EAF_UNUSED)
 	  && cur_summary_lto
-	  && ee->parm_index < cur_summary_lto->arg_flags.length ())
+	  && ee->parm_index < (int)cur_summary_lto->arg_flags.length ())
 	{
-	  int f = cur_summary_lto->arg_flags[ee->parm_index];
+	  eaf_flags_t &f = ee->parm_index == escape_point::retslot_arg
+			   ? cur_summary_lto->retslot_flags
+			   : cur_summary_lto->arg_flags[ee->parm_index];
 	  if ((f & flags_lto) != f)
 	    {
 	      f = remove_useless_eaf_flags
 			 (f & flags_lto, ecf_flags,
 			  VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
-	      cur_summary_lto->arg_flags[ee->parm_index] = f;
 	      changed = true;
 	    }
 	}
diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
index 5afa3aa439f..a4db27471eb 100644
--- a/gcc/ipa-modref.h
+++ b/gcc/ipa-modref.h
@@ -31,6 +31,7 @@ struct GTY(()) modref_summary
   modref_records *loads;
   modref_records *stores;
   auto_vec<eaf_flags_t> GTY((skip)) arg_flags;
+  eaf_flags_t retslot_flags;
   bool writes_errno;
 
   modref_summary ();
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 35971a54e02..99072df0768 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -4254,17 +4254,28 @@ handle_rhs_call (gcall *stmt, vec<ce_s> *results,
       && gimple_call_lhs (stmt) != NULL_TREE
       && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (stmt))))
     {
-      auto_vec<ce_s> tmpc;
-      struct constraint_expr *c;
-      unsigned i;
+      int flags = gimple_call_retslot_flags (stmt);
+      if ((flags & (EAF_NOESCAPE | EAF_NOT_RETURNED))
+	  != (EAF_NOESCAPE | EAF_NOT_RETURNED))
+	{
+	  auto_vec<ce_s> tmpc;
 
-      get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
+	  get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
 
-      make_constraints_to (callescape->id, tmpc);
-      if (writes_global_memory)
-	make_constraints_to (escaped_id, tmpc);
-      FOR_EACH_VEC_ELT (tmpc, i, c)
-	results->safe_push (*c);
+	  if (!(flags & (EAF_NOESCAPE | EAF_NODIRECTESCAPE)))
+	    {
+	      make_constraints_to (callescape->id, tmpc);
+	      if (writes_global_memory)
+		make_constraints_to (escaped_id, tmpc);
+	    }
+	  if (!(flags & EAF_NOT_RETURNED))
+	    {
+	      struct constraint_expr *c;
+	      unsigned i;
+	      FOR_EACH_VEC_ELT (tmpc, i, c)
+		results->safe_push (*c);
+	    }
+	}
     }
 }
 
</cut>
>From hjl@sc.intel.com  Sat Oct 30 08:08:07 2021
Return-Path: <hjl@sc.intel.com>
X-Original-To: gcc-regression@gcc.gnu.org
Delivered-To: gcc-regression@gcc.gnu.org
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
 by sourceware.org (Postfix) with ESMTPS id DB7CB385842D
 for <gcc-regression@gcc.gnu.org>; Sat, 30 Oct 2021 08:08:04 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DB7CB385842D
X-IronPort-AV: E=McAfee;i="6200,9189,10152"; a="317002131"
X-IronPort-AV: E=Sophos;i="5.87,194,1631602800"; d="scan'208";a="317002131"
Received: from fmsmga008.fm.intel.com ([10.253.24.58])
 by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 30 Oct 2021 01:08:03 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.87,194,1631602800"; d="scan'208";a="538991241"
Received: from scymds02.sc.intel.com ([10.82.73.244])
 by fmsmga008.fm.intel.com with ESMTP; 30 Oct 2021 01:07:59 -0700
Received: from gnu-snb-1.sc.intel.com (gnu-snb-1.sc.intel.com [172.25.33.219])
 by scymds02.sc.intel.com with ESMTP id 19U87xCk002443;
 Sat, 30 Oct 2021 01:07:59 -0700
Received: by gnu-snb-1.sc.intel.com (Postfix, from userid 1000)
 id 3229B180870; Sat, 30 Oct 2021 01:07:59 -0700 (PDT)
Date: Sat, 30 Oct 2021 01:07:59 -0700
To: skpgkp2@gmail.com, hjl.tools@gmail.com, gcc-regression@gcc.gnu.org
Subject: Regressions on master at commit r12-4799 vs commit r12-4781 on
 Linux/i686
User-Agent: Heirloom mailx 12.5 7/5/10
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Message-Id: <20211030080759.3229B180870@gnu-snb-1.sc.intel.com>
From: "H.J. Lu" <hjl@sc.intel.com>
X-Spam-Status: No, score=-3469.2 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS,
 KAM_LAZY_DOMAIN_SECURITY, KAM_NUMSUBJECT, SPF_HELO_NONE, SPF_NONE,
 TXREP autolearn=no autolearn_force=no version=3.4.4
X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on
 server2.sourceware.org
X-BeenThere: gcc-regression@gcc.gnu.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Gcc-regression mailing list <gcc-regression.gcc.gnu.org>
List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-regression>,
 <mailto:gcc-regression-request@gcc.gnu.org?subject=unsubscribe>
List-Archive: <https://gcc.gnu.org/pipermail/gcc-regression/>
List-Post: <mailto:gcc-regression@gcc.gnu.org>
List-Help: <mailto:gcc-regression-request@gcc.gnu.org?subject=help>
List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-regression>,
 <mailto:gcc-regression-request@gcc.gnu.org?subject=subscribe>
X-List-Received-Date: Sat, 30 Oct 2021 08:08:07 -0000

New failures:
FAIL: gcc.dg/vect/complex/fast-math-bb-slp-complex-add-float.c scan-tree-dump slp1 "Found COMPLEX_ADD_ROT270"
FAIL: gcc.dg/vect/complex/fast-math-bb-slp-complex-add-float.c scan-tree-dump slp1 "Found COMPLEX_ADD_ROT90"
FAIL: gcc.dg/vect/complex/fast-math-bb-slp-complex-add-pattern-float.c scan-tree-dump slp1 "Found COMPLEX_ADD_ROT270"
FAIL: gcc.dg/vect/complex/fast-math-bb-slp-complex-add-pattern-half-float.c scan-tree-dump slp1 "Found COMPLEX_ADD_ROT90"

New passes:
FAIL: gcc.dg/guality/pr41616-1.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  -DPREVENT_OPTIMIZATION execution test
FAIL: g++.dg/warn/Wstringop-overflow-4.C  -std=gnu++98 (test for excess errors)


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [TCWG CI] Regression caused by gcc: handle retslot in modref
  2021-10-30  2:32 [TCWG CI] Regression caused by gcc: handle retslot in modref ci_notify
@ 2021-11-03 11:33 ` Maxim Kuvyrkov
  2021-11-03 13:00   ` Jan Hubicka
       [not found]   ` <20211103151227.GA60321@kam.mff.cuni.cz>
  0 siblings, 2 replies; 5+ messages in thread
From: Maxim Kuvyrkov @ 2021-11-03 11:33 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-regression

Hi Jan,

Just wanted to make sure this is on your radar.

Your "handle retslot in modref” patch broken LTO bootstrap on aarch64 in all configurations that we test:

- tcwg_gcc_bootstrap/master-aarch64-bootstrap_lto
- tcwg_gcc_bootstrap/master-aarch64-bootstrap_profiled_lto
- tcwg_gcc_bootstrap/master-aarch64-bootstrap_profiled_lto_lean

Regards,

--
Maxim Kuvyrkov
https://www.linaro.org

> On 30 Oct 2021, at 05:32, ci_notify@linaro.org wrote:
> 
> [TCWG CI] Regression caused by gcc: handle retslot in modref:
> commit b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> Author: Jan Hubicka <hubicka@ucw.cz>
> 
>    handle retslot in modref
> 
> Results regressed to
> # reset_artifacts:
> -10
> # true:
> 0
> # build_abe binutils:
> 1
> # First few build errors in logs:
> # 00:04:02 make[3]: [Makefile:1772: aarch64-unknown-linux-gnu/bits/largefile-config.h] Error 1 (ignored)
> # 00:04:02 make[3]: [Makefile:1773: aarch64-unknown-linux-gnu/bits/largefile-config.h] Error 1 (ignored)
> # 00:19:19 checking for suffix of object files... configure: error: in `/home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gcc-gcc.git~master-stage2/aarch64-unknown-linux-gnu/libgcc':
> # 00:19:19 configure: error: cannot compute suffix of object files: cannot compile
> # 00:19:19 make[2]: *** [Makefile:19796: configure-stage2-target-libgcc] Error 1
> # 00:19:19 make[1]: *** [Makefile:25351: stage2-bubble] Error 2
> # 00:19:19 make: *** [Makefile:1019: all] Error 2
> 
> from
> # reset_artifacts:
> -10
> # true:
> 0
> # build_abe binutils:
> 1
> # build_abe bootstrap_lto:
> 2
> 
> THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.
> 
> This commit has regressed these CI configurations:
> - tcwg_gcc_bootstrap/master-aarch64-bootstrap_lto
> 
> First_bad build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9/
> Last_good build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-4045d5fa42f2ee7b284977c8f2f0edc300a63e43/
> Baseline build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-baseline/
> Even more details: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/
> 
> Reproduce builds:
> <cut>
> mkdir investigate-gcc-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> cd investigate-gcc-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> 
> # Fetch scripts
> git clone https://git.linaro.org/toolchain/jenkins-scripts
> 
> # Fetch manifests and test.sh script
> mkdir -p artifacts/manifests
> curl -o artifacts/manifests/build-baseline.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/manifests/build-baseline.sh --fail
> curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/manifests/build-parameters.sh --fail
> curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/test.sh --fail
> chmod +x artifacts/test.sh
> 
> # Reproduce the baseline build (build all pre-requisites)
> ./jenkins-scripts/tcwg_gnu-build.sh @@ artifacts/manifests/build-baseline.sh
> 
> # Save baseline build state (which is then restored in artifacts/test.sh)
> mkdir -p ./bisect
> rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ --exclude /gcc/ ./ ./bisect/baseline/
> 
> cd gcc
> 
> # Reproduce first_bad build
> git checkout --detach b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> ../artifacts/test.sh
> 
> # Reproduce last_good build
> git checkout --detach 4045d5fa42f2ee7b284977c8f2f0edc300a63e43
> ../artifacts/test.sh
> 
> cd ..
> </cut>
> 
> Full commit (up to 1000 lines):
> <cut>
> commit b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> Author: Jan Hubicka <hubicka@ucw.cz>
> Date:   Fri Oct 29 16:01:51 2021 +0200
> 
>    handle retslot in modref
> 
>    Extend modref and tree-ssa-structalias to handle retslot flags.
>    Since retslot it essentially a hidden argument that is known to be write-only
>    we can do pretty much the same stuff as we do for regular parameters.
>    I plan to add static chain handling similar way.
> 
>    We do not handle IPA propagation of retslot flags (where return slot is
>    initialized via return slot of other function). For this ipa-prop needs
>    to be extended to understand retslot as well.
> 
>    Bootstrapped/regtested x86_64-linux, OK for the gimple bits?
> 
>    Honza
> 
>    gcc/ChangeLog:
> 
>            * gimple.c (gimple_call_retslot_flags): New function.
>            * gimple.h (gimple_call_retslot_flags): Declare.
>            * ipa-modref.c: Include tree-cfg.h.
>            (struct escape_entry): Turn parm_index to signed.
>            (modref_summary_lto::modref_summary_lto): Add retslot_flags.
>            (modref_summary::modref_summary): Initialize retslot_flags.
>            (struct modref_summary_lto): Likewise.
>            (modref_summary::useful_p): Check retslot_flags.
>            (modref_summary_lto::useful_p): Likewise.
>            (modref_summary::dump): Dump retslot_flags.
>            (modref_summary_lto::dump): Likewise.
>            (struct escape_point): Add hidden_args enum.
>            (analyze_ssa_name_flags): Ignore return slot return;
>            use gimple_call_retslot_flags.
>            (record_escape_points): Break out from ...
>            (analyze_parms): ... here; handle retslot_flags.
>            (modref_summaries::duplicate): Duplicate retslot_flags.
>            (modref_summaries_lto::duplicate): Likewise.
>            (modref_write_escape_summary): Stream parm_index as signed.
>            (modref_read_escape_summary): Likewise.
>            (modref_write): Stream retslot_flags.
>            (read_section): Likewise.
>            (struct escape_map): Fix typo in comment.
>            (update_escape_summary_1): Fix whitespace.
>            (ipa_merge_modref_summary_after_inlining): Drop retslot_flags.
>            (modref_merge_call_site_flags): Merge retslot_flags.
>            * ipa-modref.h (struct modref_summary): Add retslot_flags.
>            * tree-ssa-structalias.c (handle_rhs_call): Handle retslot_flags.
> ---
> gcc/gimple.c               |  41 ++++++++++++-
> gcc/gimple.h               |   1 +
> gcc/ipa-modref.c           | 145 +++++++++++++++++++++++++++++++++------------
> gcc/ipa-modref.h           |   1 +
> gcc/tree-ssa-structalias.c |  29 ++++++---
> 5 files changed, 170 insertions(+), 47 deletions(-)
> 
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index cc7a88e822b..22dd6417d19 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -1597,7 +1597,12 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
> 	  if (!node->binds_to_current_def_p ())
> 	    {
> 	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
> -		modref_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;
> 	    }
> @@ -1608,6 +1613,40 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
>   return flags;
> }
> 
> +/* Detects argument flags for return slot on call STMT.  */
> +
> +int
> +gimple_call_retslot_flags (const gcall *stmt)
> +{
> +  int flags = EAF_DIRECT | EAF_NOREAD;
> +
> +  tree callee = gimple_call_fndecl (stmt);
> +  if (callee)
> +    {
> +      cgraph_node *node = cgraph_node::get (callee);
> +      modref_summary *summary = node ? get_modref_function_summary (node)
> +				: NULL;
> +
> +      if (summary)
> +	{
> +	  int modref_flags = summary->retslot_flags;
> +
> +	  /* 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 (dbg_cnt (ipa_mod_ref_pta))
> +	    flags |= modref_flags;
> +	}
> +    }
> +  return flags;
> +}
> +
> /* Detects return flags for the call STMT.  */
> 
> int
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index 303623b3ced..23a124ec769 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -1589,6 +1589,7 @@ gimple_seq gimple_seq_copy (gimple_seq);
> bool gimple_call_same_target_p (const gimple *, const gimple *);
> int gimple_call_flags (const gimple *);
> int gimple_call_arg_flags (const gcall *, unsigned);
> +int gimple_call_retslot_flags (const gcall *);
> int gimple_call_return_flags (const gcall *);
> bool gimple_call_nonnull_result_p (gcall *);
> tree gimple_call_nonnull_arg (gcall *);
> diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> index 0bbec8df0a2..3539cb43d11 100644
> --- a/gcc/ipa-modref.c
> +++ b/gcc/ipa-modref.c
> @@ -86,6 +86,7 @@ along with GCC; see the file COPYING3.  If not see
> #include "stringpool.h"
> #include "tree-ssanames.h"
> #include "attribs.h"
> +#include "tree-cfg.h"
> 
> 
> namespace {
> @@ -133,7 +134,7 @@ static fnspec_summaries_t *fnspec_summaries = NULL;
> struct escape_entry
> {
>   /* Parameter that escapes at a given call.  */
> -  unsigned int parm_index;
> +  int parm_index;
>   /* Argument it escapes to.  */
>   unsigned int arg;
>   /* Minimal flags known about the argument.  */
> @@ -269,7 +270,7 @@ static GTY(()) fast_function_summary <modref_summary_lto *, va_gc>
> /* Summary for a single function which this pass produces.  */
> 
> modref_summary::modref_summary ()
> -  : loads (NULL), stores (NULL), writes_errno (NULL)
> +  : loads (NULL), stores (NULL), retslot_flags (0), writes_errno (false)
> {
> }
> 
> @@ -322,6 +323,8 @@ modref_summary::useful_p (int ecf_flags, bool check_flags)
>   if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
>     return true;
>   arg_flags.release ();
> +  if (check_flags && remove_useless_eaf_flags (retslot_flags, ecf_flags, false))
> +    return true;
>   if (ecf_flags & ECF_CONST)
>     return false;
>   if (loads && !loads->every_base)
> @@ -363,6 +366,7 @@ struct GTY(()) modref_summary_lto
>   modref_records_lto *loads;
>   modref_records_lto *stores;
>   auto_vec<eaf_flags_t> GTY((skip)) arg_flags;
> +  eaf_flags_t retslot_flags;
>   bool writes_errno;
> 
>   modref_summary_lto ();
> @@ -374,7 +378,7 @@ struct GTY(()) modref_summary_lto
> /* Summary for a single function which this pass produces.  */
> 
> modref_summary_lto::modref_summary_lto ()
> -  : loads (NULL), stores (NULL), writes_errno (NULL)
> +  : loads (NULL), stores (NULL), retslot_flags (0), writes_errno (false)
> {
> }
> 
> @@ -400,6 +404,8 @@ modref_summary_lto::useful_p (int ecf_flags, bool check_flags)
>   if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
>     return true;
>   arg_flags.release ();
> +  if (check_flags && remove_useless_eaf_flags (retslot_flags, ecf_flags, false))
> +    return true;
>   if (ecf_flags & ECF_CONST)
>     return false;
>   if (loads && !loads->every_base)
> @@ -608,6 +614,11 @@ modref_summary::dump (FILE *out)
> 	    dump_eaf_flags (out, arg_flags[i]);
> 	  }
>     }
> +  if (retslot_flags)
> +    {
> +      fprintf (out, "  Retslot flags:");
> +      dump_eaf_flags (out, retslot_flags);
> +    }
> }
> 
> /* Dump summary.  */
> @@ -630,6 +641,11 @@ modref_summary_lto::dump (FILE *out)
> 	    dump_eaf_flags (out, arg_flags[i]);
> 	  }
>     }
> +  if (retslot_flags)
> +    {
> +      fprintf (out, "  Retslot flags:");
> +      dump_eaf_flags (out, retslot_flags);
> +    }
> }
> 
> /* Get function summary for FUNC if it exists, return NULL otherwise.  */
> @@ -1396,6 +1412,11 @@ namespace {
> 
> struct escape_point
> {
> +  /* Extra hidden args we keep track of.  */
> +  enum hidden_args
> +  {
> +    retslot_arg = -1
> +  };
>   /* Value escapes to this call.  */
>   gcall *call;
>   /* Argument it escapes to.  */
> @@ -1705,7 +1726,11 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
> 	 Returning name counts as an use by tree-ssa-structalias.c  */
>       if (greturn *ret = dyn_cast <greturn *> (use_stmt))
> 	{
> -	  if (gimple_return_retval (ret) == name)
> +	  /* Returning through return slot is seen as memory write earlier.  */
> +	  if (DECL_RESULT (current_function_decl)
> +	      && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
> +	    ;
> +	  else if (gimple_return_retval (ret) == name)
> 	    lattice[index].merge (~(EAF_UNUSED | EAF_NOT_RETURNED));
> 	  else if (memory_access_to (gimple_return_retval (ret), name))
> 	    {
> @@ -1748,7 +1773,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
> 		     may make LHS to escape.  See PR 98499.  */
> 		  if (gimple_call_return_slot_opt_p (call)
> 		      && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (call))))
> -		    lattice[index].merge (EAF_NOREAD | EAF_DIRECT);
> +		    lattice[index].merge (gimple_call_retslot_flags (call));
> 		}
> 
> 	      /* We do not track accesses to the static chain (we could)
> @@ -1777,7 +1802,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
> 			  lattice[index].merge (call_flags);
> 			else
> 			  lattice[index].add_escape_point (call, i,
> -			     				   call_flags, true);
> +							   call_flags, true);
> 		      }
> 		    if (!ignore_retval)
> 		      merge_call_lhs_flags (call, i, index, false,
> @@ -1912,6 +1937,29 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
>   lattice[index].known = true;
> }
> 
> +/* Record escape points of PARM_INDEX according to LATTICE.  */
> +
> +static void
> +record_escape_points (modref_lattice &lattice, int parm_index, int flags)
> +{
> +  if (lattice.escape_points.length ())
> +    {
> +      escape_point *ep;
> +      unsigned int ip;
> +      cgraph_node *node = cgraph_node::get (current_function_decl);
> +
> +      FOR_EACH_VEC_ELT (lattice.escape_points, ip, ep)
> +	if ((ep->min_flags & flags) != flags)
> +	  {
> +	    cgraph_edge *e = node->get_edge (ep->call);
> +	    struct escape_entry ee = {parm_index, ep->arg,
> +				      ep->min_flags, ep->direct};
> +
> +	    escape_summaries->get_create (e)->esc.safe_push (ee);
> +	  }
> +    }
> +}
> +
> /* Determine EAF flags for function parameters.  */
> 
> static void
> @@ -1921,16 +1969,22 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto,
>   unsigned int parm_index = 0;
>   unsigned int count = 0;
>   int ecf_flags = flags_from_decl_or_type (current_function_decl);
> +  tree retslot = 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)))
> +    retslot = ssa_default_def (cfun, DECL_RESULT (current_function_decl));
> +
>   for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
>        parm = TREE_CHAIN (parm))
>     count++;
> 
> -  if (!count)
> +  if (!count && !retslot)
>     return;
> 
>   auto_vec<modref_lattice> lattice;
> @@ -1984,24 +2038,24 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto,
> 		summary_lto->arg_flags.safe_grow_cleared (count, true);
> 	      summary_lto->arg_flags[parm_index] = flags;
> 	    }
> -	  if (lattice[SSA_NAME_VERSION (name)].escape_points.length ())
> -	    {
> -	      escape_point *ep;
> -	      unsigned int ip;
> -	      cgraph_node *node = cgraph_node::get (current_function_decl);
> -
> -	      gcc_checking_assert (ipa);
> -	      FOR_EACH_VEC_ELT
> -		  (lattice[SSA_NAME_VERSION (name)].escape_points, ip, ep)
> -		if ((ep->min_flags & flags) != flags)
> -		  {
> -		    cgraph_edge *e = node->get_edge (ep->call);
> -		    struct escape_entry ee = {parm_index, ep->arg,
> -		     			      ep->min_flags, ep->direct};
> +	  record_escape_points (lattice[SSA_NAME_VERSION (name)],
> +				parm_index, flags);
> +	}
> +    }
> +  if (retslot)
> +    {
> +      analyze_ssa_name_flags (retslot, lattice, 0, ipa);
> +      int flags = lattice[SSA_NAME_VERSION (retslot)].flags;
> 
> -		    escape_summaries->get_create (e)->esc.safe_push (ee);
> -		  }
> -	    }
> +      flags = remove_useless_eaf_flags (flags, ecf_flags, false);
> +      if (flags)
> +	{
> +	  if (summary)
> +	    summary->retslot_flags = flags;
> +	  if (summary_lto)
> +	    summary_lto->retslot_flags = flags;
> +	  record_escape_points (lattice[SSA_NAME_VERSION (retslot)],
> +				escape_point::retslot_arg, flags);
> 	}
>     }
>   if (ipa)
> @@ -2287,6 +2341,7 @@ modref_summaries::duplicate (cgraph_node *, cgraph_node *dst,
>   dst_data->writes_errno = src_data->writes_errno;
>   if (src_data->arg_flags.length ())
>     dst_data->arg_flags = src_data->arg_flags.copy ();
> +  dst_data->retslot_flags = src_data->retslot_flags;
> }
> 
> /* Called when new clone is inserted to callgraph late.  */
> @@ -2312,6 +2367,7 @@ modref_summaries_lto::duplicate (cgraph_node *, cgraph_node *,
>   dst_data->writes_errno = src_data->writes_errno;
>   if (src_data->arg_flags.length ())
>     dst_data->arg_flags = src_data->arg_flags.copy ();
> +  dst_data->retslot_flags = src_data->retslot_flags;
> }
> 
> namespace
> @@ -2551,7 +2607,7 @@ modref_write_escape_summary (struct bitpack_d *bp, escape_summary *esum)
>   escape_entry *ee;
>   FOR_EACH_VEC_ELT (esum->esc, i, ee)
>     {
> -      bp_pack_var_len_unsigned (bp, ee->parm_index);
> +      bp_pack_var_len_int (bp, ee->parm_index);
>       bp_pack_var_len_unsigned (bp, ee->arg);
>       bp_pack_var_len_unsigned (bp, ee->min_flags);
>       bp_pack_value (bp, ee->direct, 1);
> @@ -2571,7 +2627,7 @@ modref_read_escape_summary (struct bitpack_d *bp, cgraph_edge *e)
>   for (unsigned int i = 0; i < n; i++)
>     {
>       escape_entry ee;
> -      ee.parm_index = bp_unpack_var_len_unsigned (bp);
> +      ee.parm_index = bp_unpack_var_len_int (bp);
>       ee.arg = bp_unpack_var_len_unsigned (bp);
>       ee.min_flags = bp_unpack_var_len_unsigned (bp);
>       ee.direct = bp_unpack_value (bp, 1);
> @@ -2628,6 +2684,7 @@ modref_write ()
> 	  streamer_write_uhwi (ob, r->arg_flags.length ());
> 	  for (unsigned int i = 0; i < r->arg_flags.length (); i++)
> 	    streamer_write_uhwi (ob, r->arg_flags[i]);
> +	  streamer_write_uhwi (ob, r->retslot_flags);
> 
> 	  write_modref_records (r->loads, ob);
> 	  write_modref_records (r->stores, ob);
> @@ -2724,6 +2781,11 @@ read_section (struct lto_file_decl_data *file_data, const char *data,
> 	  if (modref_sum_lto)
> 	    modref_sum_lto->arg_flags.quick_push (flags);
> 	}
> +      eaf_flags_t flags = streamer_read_uhwi (&ib);
> +      if (modref_sum)
> +	modref_sum->retslot_flags = flags;
> +      if (modref_sum_lto)
> +	modref_sum_lto->retslot_flags = flags;
>       read_modref_records (&ib, data_in,
> 			   modref_sum ? &modref_sum->loads : NULL,
> 			   modref_sum_lto ? &modref_sum_lto->loads : NULL);
> @@ -3098,7 +3160,7 @@ struct escape_map
>   bool direct;
> };
> 
> -/* Update escape map fo E.  */
> +/* Update escape map for E.  */
> 
> static void
> update_escape_summary_1 (cgraph_edge *e,
> @@ -3117,7 +3179,10 @@ update_escape_summary_1 (cgraph_edge *e,
>     {
>       unsigned int j;
>       struct escape_map *em;
> -      if (ee->parm_index >= map.length ())
> +      /* TODO: We do not have jump functions for return slots, so we
> +	 never propagate them to outer function.  */
> +      if (ee->parm_index >= (int)map.length ()
> +	  || ee->parm_index < 0)
> 	continue;
>       FOR_EACH_VEC_ELT (map[ee->parm_index], j, em)
> 	{
> @@ -3125,7 +3190,7 @@ update_escape_summary_1 (cgraph_edge *e,
> 	  if (ee->direct && !em->direct)
> 	    min_flags = deref_flags (min_flags, ignore_stores);
> 	  struct escape_entry entry = {em->parm_index, ee->arg,
> -	    			       ee->min_flags,
> +				       ee->min_flags,
> 				       ee->direct & em->direct};
> 	  sum->esc.safe_push (entry);
> 	}
> @@ -3245,7 +3310,11 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
>     FOR_EACH_VEC_ELT (sum->esc, i, ee)
>       {
> 	bool needed = false;
> -	if (to_info && to_info->arg_flags.length () > ee->parm_index)
> +	/* TODO: We do not have jump functions for return slots, so we
> +	   never propagate them to outer function.  */
> +	if (ee->parm_index < 0)
> +	  continue;
> +	if (to_info && (int)to_info->arg_flags.length () > ee->parm_index)
> 	  {
> 	    int flags = callee_info
> 			&& callee_info->arg_flags.length () > ee->arg
> @@ -3259,7 +3328,7 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
> 	    if (to_info->arg_flags[ee->parm_index])
> 	      needed = true;
> 	  }
> -	if (to_info_lto && to_info_lto->arg_flags.length () > ee->parm_index)
> +	if (to_info_lto && (int)to_info_lto->arg_flags.length () > ee->parm_index)
> 	  {
> 	    int flags = callee_info_lto
> 			&& callee_info_lto->arg_flags.length () > ee->arg
> @@ -3798,29 +3867,31 @@ modref_merge_call_site_flags (escape_summary *sum,
>       if (flags_lto & EAF_NOESCAPE)
> 	flags_lto |= EAF_NODIRECTESCAPE;
>       if (!(flags & EAF_UNUSED)
> -	  && cur_summary && ee->parm_index < cur_summary->arg_flags.length ())
> +	  && cur_summary && ee->parm_index < (int)cur_summary->arg_flags.length ())
> 	{
> -	  int f = cur_summary->arg_flags[ee->parm_index];
> +	  eaf_flags_t &f = ee->parm_index == escape_point::retslot_arg
> +			   ? cur_summary->retslot_flags
> +			   : cur_summary->arg_flags[ee->parm_index];
> 	  if ((f & flags) != f)
> 	    {
> 	      f = remove_useless_eaf_flags
> 			 (f & flags, ecf_flags,
> 			  VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
> -	      cur_summary->arg_flags[ee->parm_index] = f;
> 	      changed = true;
> 	    }
> 	}
>       if (!(flags_lto & EAF_UNUSED)
> 	  && cur_summary_lto
> -	  && ee->parm_index < cur_summary_lto->arg_flags.length ())
> +	  && ee->parm_index < (int)cur_summary_lto->arg_flags.length ())
> 	{
> -	  int f = cur_summary_lto->arg_flags[ee->parm_index];
> +	  eaf_flags_t &f = ee->parm_index == escape_point::retslot_arg
> +			   ? cur_summary_lto->retslot_flags
> +			   : cur_summary_lto->arg_flags[ee->parm_index];
> 	  if ((f & flags_lto) != f)
> 	    {
> 	      f = remove_useless_eaf_flags
> 			 (f & flags_lto, ecf_flags,
> 			  VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
> -	      cur_summary_lto->arg_flags[ee->parm_index] = f;
> 	      changed = true;
> 	    }
> 	}
> diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
> index 5afa3aa439f..a4db27471eb 100644
> --- a/gcc/ipa-modref.h
> +++ b/gcc/ipa-modref.h
> @@ -31,6 +31,7 @@ struct GTY(()) modref_summary
>   modref_records *loads;
>   modref_records *stores;
>   auto_vec<eaf_flags_t> GTY((skip)) arg_flags;
> +  eaf_flags_t retslot_flags;
>   bool writes_errno;
> 
>   modref_summary ();
> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
> index 35971a54e02..99072df0768 100644
> --- a/gcc/tree-ssa-structalias.c
> +++ b/gcc/tree-ssa-structalias.c
> @@ -4254,17 +4254,28 @@ handle_rhs_call (gcall *stmt, vec<ce_s> *results,
>       && gimple_call_lhs (stmt) != NULL_TREE
>       && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (stmt))))
>     {
> -      auto_vec<ce_s> tmpc;
> -      struct constraint_expr *c;
> -      unsigned i;
> +      int flags = gimple_call_retslot_flags (stmt);
> +      if ((flags & (EAF_NOESCAPE | EAF_NOT_RETURNED))
> +	  != (EAF_NOESCAPE | EAF_NOT_RETURNED))
> +	{
> +	  auto_vec<ce_s> tmpc;
> 
> -      get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
> +	  get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
> 
> -      make_constraints_to (callescape->id, tmpc);
> -      if (writes_global_memory)
> -	make_constraints_to (escaped_id, tmpc);
> -      FOR_EACH_VEC_ELT (tmpc, i, c)
> -	results->safe_push (*c);
> +	  if (!(flags & (EAF_NOESCAPE | EAF_NODIRECTESCAPE)))
> +	    {
> +	      make_constraints_to (callescape->id, tmpc);
> +	      if (writes_global_memory)
> +		make_constraints_to (escaped_id, tmpc);
> +	    }
> +	  if (!(flags & EAF_NOT_RETURNED))
> +	    {
> +	      struct constraint_expr *c;
> +	      unsigned i;
> +	      FOR_EACH_VEC_ELT (tmpc, i, c)
> +		results->safe_push (*c);
> +	    }
> +	}
>     }
> }
> 
> </cut>



^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [TCWG CI] Regression caused by gcc: handle retslot in modref
  2021-11-03 11:33 ` Maxim Kuvyrkov
@ 2021-11-03 13:00   ` Jan Hubicka
       [not found]   ` <20211103151227.GA60321@kam.mff.cuni.cz>
  1 sibling, 0 replies; 5+ messages in thread
From: Jan Hubicka @ 2021-11-03 13:00 UTC (permalink / raw)
  To: Maxim Kuvyrkov; +Cc: gcc-regression

> Hi Jan,
> 
> Just wanted to make sure this is on your radar.
> 
> Your "handle retslot in modref” patch broken LTO bootstrap on aarch64 in all configurations that we test:
> 
> - tcwg_gcc_bootstrap/master-aarch64-bootstrap_lto
> - tcwg_gcc_bootstrap/master-aarch64-bootstrap_profiled_lto
> - tcwg_gcc_bootstrap/master-aarch64-bootstrap_profiled_lto_lean

I tried to reproduce this on the compile farm and the bootstrap worked
w/o problems.
What is the easiest way to reproduce this?

Honza
> 
> Regards,
> 
> --
> Maxim Kuvyrkov
> https://www.linaro.org
> 
> > On 30 Oct 2021, at 05:32, ci_notify@linaro.org wrote:
> > 
> > [TCWG CI] Regression caused by gcc: handle retslot in modref:
> > commit b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> > Author: Jan Hubicka <hubicka@ucw.cz>
> > 
> >    handle retslot in modref
> > 
> > Results regressed to
> > # reset_artifacts:
> > -10
> > # true:
> > 0
> > # build_abe binutils:
> > 1
> > # First few build errors in logs:
> > # 00:04:02 make[3]: [Makefile:1772: aarch64-unknown-linux-gnu/bits/largefile-config.h] Error 1 (ignored)
> > # 00:04:02 make[3]: [Makefile:1773: aarch64-unknown-linux-gnu/bits/largefile-config.h] Error 1 (ignored)
> > # 00:19:19 checking for suffix of object files... configure: error: in `/home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gcc-gcc.git~master-stage2/aarch64-unknown-linux-gnu/libgcc':
> > # 00:19:19 configure: error: cannot compute suffix of object files: cannot compile
> > # 00:19:19 make[2]: *** [Makefile:19796: configure-stage2-target-libgcc] Error 1
> > # 00:19:19 make[1]: *** [Makefile:25351: stage2-bubble] Error 2
> > # 00:19:19 make: *** [Makefile:1019: all] Error 2
> > 
> > from
> > # reset_artifacts:
> > -10
> > # true:
> > 0
> > # build_abe binutils:
> > 1
> > # build_abe bootstrap_lto:
> > 2
> > 
> > THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.
> > 
> > This commit has regressed these CI configurations:
> > - tcwg_gcc_bootstrap/master-aarch64-bootstrap_lto
> > 
> > First_bad build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9/
> > Last_good build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-4045d5fa42f2ee7b284977c8f2f0edc300a63e43/
> > Baseline build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-baseline/
> > Even more details: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/
> > 
> > Reproduce builds:
> > <cut>
> > mkdir investigate-gcc-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> > cd investigate-gcc-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> > 
> > # Fetch scripts
> > git clone https://git.linaro.org/toolchain/jenkins-scripts
> > 
> > # Fetch manifests and test.sh script
> > mkdir -p artifacts/manifests
> > curl -o artifacts/manifests/build-baseline.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/manifests/build-baseline.sh --fail
> > curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/manifests/build-parameters.sh --fail
> > curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/test.sh --fail
> > chmod +x artifacts/test.sh
> > 
> > # Reproduce the baseline build (build all pre-requisites)
> > ./jenkins-scripts/tcwg_gnu-build.sh @@ artifacts/manifests/build-baseline.sh
> > 
> > # Save baseline build state (which is then restored in artifacts/test.sh)
> > mkdir -p ./bisect
> > rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ --exclude /gcc/ ./ ./bisect/baseline/
> > 
> > cd gcc
> > 
> > # Reproduce first_bad build
> > git checkout --detach b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> > ../artifacts/test.sh
> > 
> > # Reproduce last_good build
> > git checkout --detach 4045d5fa42f2ee7b284977c8f2f0edc300a63e43
> > ../artifacts/test.sh
> > 
> > cd ..
> > </cut>
> > 
> > Full commit (up to 1000 lines):
> > <cut>
> > commit b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> > Author: Jan Hubicka <hubicka@ucw.cz>
> > Date:   Fri Oct 29 16:01:51 2021 +0200
> > 
> >    handle retslot in modref
> > 
> >    Extend modref and tree-ssa-structalias to handle retslot flags.
> >    Since retslot it essentially a hidden argument that is known to be write-only
> >    we can do pretty much the same stuff as we do for regular parameters.
> >    I plan to add static chain handling similar way.
> > 
> >    We do not handle IPA propagation of retslot flags (where return slot is
> >    initialized via return slot of other function). For this ipa-prop needs
> >    to be extended to understand retslot as well.
> > 
> >    Bootstrapped/regtested x86_64-linux, OK for the gimple bits?
> > 
> >    Honza
> > 
> >    gcc/ChangeLog:
> > 
> >            * gimple.c (gimple_call_retslot_flags): New function.
> >            * gimple.h (gimple_call_retslot_flags): Declare.
> >            * ipa-modref.c: Include tree-cfg.h.
> >            (struct escape_entry): Turn parm_index to signed.
> >            (modref_summary_lto::modref_summary_lto): Add retslot_flags.
> >            (modref_summary::modref_summary): Initialize retslot_flags.
> >            (struct modref_summary_lto): Likewise.
> >            (modref_summary::useful_p): Check retslot_flags.
> >            (modref_summary_lto::useful_p): Likewise.
> >            (modref_summary::dump): Dump retslot_flags.
> >            (modref_summary_lto::dump): Likewise.
> >            (struct escape_point): Add hidden_args enum.
> >            (analyze_ssa_name_flags): Ignore return slot return;
> >            use gimple_call_retslot_flags.
> >            (record_escape_points): Break out from ...
> >            (analyze_parms): ... here; handle retslot_flags.
> >            (modref_summaries::duplicate): Duplicate retslot_flags.
> >            (modref_summaries_lto::duplicate): Likewise.
> >            (modref_write_escape_summary): Stream parm_index as signed.
> >            (modref_read_escape_summary): Likewise.
> >            (modref_write): Stream retslot_flags.
> >            (read_section): Likewise.
> >            (struct escape_map): Fix typo in comment.
> >            (update_escape_summary_1): Fix whitespace.
> >            (ipa_merge_modref_summary_after_inlining): Drop retslot_flags.
> >            (modref_merge_call_site_flags): Merge retslot_flags.
> >            * ipa-modref.h (struct modref_summary): Add retslot_flags.
> >            * tree-ssa-structalias.c (handle_rhs_call): Handle retslot_flags.
> > ---
> > gcc/gimple.c               |  41 ++++++++++++-
> > gcc/gimple.h               |   1 +
> > gcc/ipa-modref.c           | 145 +++++++++++++++++++++++++++++++++------------
> > gcc/ipa-modref.h           |   1 +
> > gcc/tree-ssa-structalias.c |  29 ++++++---
> > 5 files changed, 170 insertions(+), 47 deletions(-)
> > 
> > diff --git a/gcc/gimple.c b/gcc/gimple.c
> > index cc7a88e822b..22dd6417d19 100644
> > --- a/gcc/gimple.c
> > +++ b/gcc/gimple.c
> > @@ -1597,7 +1597,12 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
> > 	  if (!node->binds_to_current_def_p ())
> > 	    {
> > 	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
> > -		modref_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;
> > 	    }
> > @@ -1608,6 +1613,40 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
> >   return flags;
> > }
> > 
> > +/* Detects argument flags for return slot on call STMT.  */
> > +
> > +int
> > +gimple_call_retslot_flags (const gcall *stmt)
> > +{
> > +  int flags = EAF_DIRECT | EAF_NOREAD;
> > +
> > +  tree callee = gimple_call_fndecl (stmt);
> > +  if (callee)
> > +    {
> > +      cgraph_node *node = cgraph_node::get (callee);
> > +      modref_summary *summary = node ? get_modref_function_summary (node)
> > +				: NULL;
> > +
> > +      if (summary)
> > +	{
> > +	  int modref_flags = summary->retslot_flags;
> > +
> > +	  /* 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 (dbg_cnt (ipa_mod_ref_pta))
> > +	    flags |= modref_flags;
> > +	}
> > +    }
> > +  return flags;
> > +}
> > +
> > /* Detects return flags for the call STMT.  */
> > 
> > int
> > diff --git a/gcc/gimple.h b/gcc/gimple.h
> > index 303623b3ced..23a124ec769 100644
> > --- a/gcc/gimple.h
> > +++ b/gcc/gimple.h
> > @@ -1589,6 +1589,7 @@ gimple_seq gimple_seq_copy (gimple_seq);
> > bool gimple_call_same_target_p (const gimple *, const gimple *);
> > int gimple_call_flags (const gimple *);
> > int gimple_call_arg_flags (const gcall *, unsigned);
> > +int gimple_call_retslot_flags (const gcall *);
> > int gimple_call_return_flags (const gcall *);
> > bool gimple_call_nonnull_result_p (gcall *);
> > tree gimple_call_nonnull_arg (gcall *);
> > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> > index 0bbec8df0a2..3539cb43d11 100644
> > --- a/gcc/ipa-modref.c
> > +++ b/gcc/ipa-modref.c
> > @@ -86,6 +86,7 @@ along with GCC; see the file COPYING3.  If not see
> > #include "stringpool.h"
> > #include "tree-ssanames.h"
> > #include "attribs.h"
> > +#include "tree-cfg.h"
> > 
> > 
> > namespace {
> > @@ -133,7 +134,7 @@ static fnspec_summaries_t *fnspec_summaries = NULL;
> > struct escape_entry
> > {
> >   /* Parameter that escapes at a given call.  */
> > -  unsigned int parm_index;
> > +  int parm_index;
> >   /* Argument it escapes to.  */
> >   unsigned int arg;
> >   /* Minimal flags known about the argument.  */
> > @@ -269,7 +270,7 @@ static GTY(()) fast_function_summary <modref_summary_lto *, va_gc>
> > /* Summary for a single function which this pass produces.  */
> > 
> > modref_summary::modref_summary ()
> > -  : loads (NULL), stores (NULL), writes_errno (NULL)
> > +  : loads (NULL), stores (NULL), retslot_flags (0), writes_errno (false)
> > {
> > }
> > 
> > @@ -322,6 +323,8 @@ modref_summary::useful_p (int ecf_flags, bool check_flags)
> >   if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
> >     return true;
> >   arg_flags.release ();
> > +  if (check_flags && remove_useless_eaf_flags (retslot_flags, ecf_flags, false))
> > +    return true;
> >   if (ecf_flags & ECF_CONST)
> >     return false;
> >   if (loads && !loads->every_base)
> > @@ -363,6 +366,7 @@ struct GTY(()) modref_summary_lto
> >   modref_records_lto *loads;
> >   modref_records_lto *stores;
> >   auto_vec<eaf_flags_t> GTY((skip)) arg_flags;
> > +  eaf_flags_t retslot_flags;
> >   bool writes_errno;
> > 
> >   modref_summary_lto ();
> > @@ -374,7 +378,7 @@ struct GTY(()) modref_summary_lto
> > /* Summary for a single function which this pass produces.  */
> > 
> > modref_summary_lto::modref_summary_lto ()
> > -  : loads (NULL), stores (NULL), writes_errno (NULL)
> > +  : loads (NULL), stores (NULL), retslot_flags (0), writes_errno (false)
> > {
> > }
> > 
> > @@ -400,6 +404,8 @@ modref_summary_lto::useful_p (int ecf_flags, bool check_flags)
> >   if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
> >     return true;
> >   arg_flags.release ();
> > +  if (check_flags && remove_useless_eaf_flags (retslot_flags, ecf_flags, false))
> > +    return true;
> >   if (ecf_flags & ECF_CONST)
> >     return false;
> >   if (loads && !loads->every_base)
> > @@ -608,6 +614,11 @@ modref_summary::dump (FILE *out)
> > 	    dump_eaf_flags (out, arg_flags[i]);
> > 	  }
> >     }
> > +  if (retslot_flags)
> > +    {
> > +      fprintf (out, "  Retslot flags:");
> > +      dump_eaf_flags (out, retslot_flags);
> > +    }
> > }
> > 
> > /* Dump summary.  */
> > @@ -630,6 +641,11 @@ modref_summary_lto::dump (FILE *out)
> > 	    dump_eaf_flags (out, arg_flags[i]);
> > 	  }
> >     }
> > +  if (retslot_flags)
> > +    {
> > +      fprintf (out, "  Retslot flags:");
> > +      dump_eaf_flags (out, retslot_flags);
> > +    }
> > }
> > 
> > /* Get function summary for FUNC if it exists, return NULL otherwise.  */
> > @@ -1396,6 +1412,11 @@ namespace {
> > 
> > struct escape_point
> > {
> > +  /* Extra hidden args we keep track of.  */
> > +  enum hidden_args
> > +  {
> > +    retslot_arg = -1
> > +  };
> >   /* Value escapes to this call.  */
> >   gcall *call;
> >   /* Argument it escapes to.  */
> > @@ -1705,7 +1726,11 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
> > 	 Returning name counts as an use by tree-ssa-structalias.c  */
> >       if (greturn *ret = dyn_cast <greturn *> (use_stmt))
> > 	{
> > -	  if (gimple_return_retval (ret) == name)
> > +	  /* Returning through return slot is seen as memory write earlier.  */
> > +	  if (DECL_RESULT (current_function_decl)
> > +	      && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
> > +	    ;
> > +	  else if (gimple_return_retval (ret) == name)
> > 	    lattice[index].merge (~(EAF_UNUSED | EAF_NOT_RETURNED));
> > 	  else if (memory_access_to (gimple_return_retval (ret), name))
> > 	    {
> > @@ -1748,7 +1773,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
> > 		     may make LHS to escape.  See PR 98499.  */
> > 		  if (gimple_call_return_slot_opt_p (call)
> > 		      && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (call))))
> > -		    lattice[index].merge (EAF_NOREAD | EAF_DIRECT);
> > +		    lattice[index].merge (gimple_call_retslot_flags (call));
> > 		}
> > 
> > 	      /* We do not track accesses to the static chain (we could)
> > @@ -1777,7 +1802,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
> > 			  lattice[index].merge (call_flags);
> > 			else
> > 			  lattice[index].add_escape_point (call, i,
> > -			     				   call_flags, true);
> > +							   call_flags, true);
> > 		      }
> > 		    if (!ignore_retval)
> > 		      merge_call_lhs_flags (call, i, index, false,
> > @@ -1912,6 +1937,29 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
> >   lattice[index].known = true;
> > }
> > 
> > +/* Record escape points of PARM_INDEX according to LATTICE.  */
> > +
> > +static void
> > +record_escape_points (modref_lattice &lattice, int parm_index, int flags)
> > +{
> > +  if (lattice.escape_points.length ())
> > +    {
> > +      escape_point *ep;
> > +      unsigned int ip;
> > +      cgraph_node *node = cgraph_node::get (current_function_decl);
> > +
> > +      FOR_EACH_VEC_ELT (lattice.escape_points, ip, ep)
> > +	if ((ep->min_flags & flags) != flags)
> > +	  {
> > +	    cgraph_edge *e = node->get_edge (ep->call);
> > +	    struct escape_entry ee = {parm_index, ep->arg,
> > +				      ep->min_flags, ep->direct};
> > +
> > +	    escape_summaries->get_create (e)->esc.safe_push (ee);
> > +	  }
> > +    }
> > +}
> > +
> > /* Determine EAF flags for function parameters.  */
> > 
> > static void
> > @@ -1921,16 +1969,22 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto,
> >   unsigned int parm_index = 0;
> >   unsigned int count = 0;
> >   int ecf_flags = flags_from_decl_or_type (current_function_decl);
> > +  tree retslot = 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)))
> > +    retslot = ssa_default_def (cfun, DECL_RESULT (current_function_decl));
> > +
> >   for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
> >        parm = TREE_CHAIN (parm))
> >     count++;
> > 
> > -  if (!count)
> > +  if (!count && !retslot)
> >     return;
> > 
> >   auto_vec<modref_lattice> lattice;
> > @@ -1984,24 +2038,24 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto,
> > 		summary_lto->arg_flags.safe_grow_cleared (count, true);
> > 	      summary_lto->arg_flags[parm_index] = flags;
> > 	    }
> > -	  if (lattice[SSA_NAME_VERSION (name)].escape_points.length ())
> > -	    {
> > -	      escape_point *ep;
> > -	      unsigned int ip;
> > -	      cgraph_node *node = cgraph_node::get (current_function_decl);
> > -
> > -	      gcc_checking_assert (ipa);
> > -	      FOR_EACH_VEC_ELT
> > -		  (lattice[SSA_NAME_VERSION (name)].escape_points, ip, ep)
> > -		if ((ep->min_flags & flags) != flags)
> > -		  {
> > -		    cgraph_edge *e = node->get_edge (ep->call);
> > -		    struct escape_entry ee = {parm_index, ep->arg,
> > -		     			      ep->min_flags, ep->direct};
> > +	  record_escape_points (lattice[SSA_NAME_VERSION (name)],
> > +				parm_index, flags);
> > +	}
> > +    }
> > +  if (retslot)
> > +    {
> > +      analyze_ssa_name_flags (retslot, lattice, 0, ipa);
> > +      int flags = lattice[SSA_NAME_VERSION (retslot)].flags;
> > 
> > -		    escape_summaries->get_create (e)->esc.safe_push (ee);
> > -		  }
> > -	    }
> > +      flags = remove_useless_eaf_flags (flags, ecf_flags, false);
> > +      if (flags)
> > +	{
> > +	  if (summary)
> > +	    summary->retslot_flags = flags;
> > +	  if (summary_lto)
> > +	    summary_lto->retslot_flags = flags;
> > +	  record_escape_points (lattice[SSA_NAME_VERSION (retslot)],
> > +				escape_point::retslot_arg, flags);
> > 	}
> >     }
> >   if (ipa)
> > @@ -2287,6 +2341,7 @@ modref_summaries::duplicate (cgraph_node *, cgraph_node *dst,
> >   dst_data->writes_errno = src_data->writes_errno;
> >   if (src_data->arg_flags.length ())
> >     dst_data->arg_flags = src_data->arg_flags.copy ();
> > +  dst_data->retslot_flags = src_data->retslot_flags;
> > }
> > 
> > /* Called when new clone is inserted to callgraph late.  */
> > @@ -2312,6 +2367,7 @@ modref_summaries_lto::duplicate (cgraph_node *, cgraph_node *,
> >   dst_data->writes_errno = src_data->writes_errno;
> >   if (src_data->arg_flags.length ())
> >     dst_data->arg_flags = src_data->arg_flags.copy ();
> > +  dst_data->retslot_flags = src_data->retslot_flags;
> > }
> > 
> > namespace
> > @@ -2551,7 +2607,7 @@ modref_write_escape_summary (struct bitpack_d *bp, escape_summary *esum)
> >   escape_entry *ee;
> >   FOR_EACH_VEC_ELT (esum->esc, i, ee)
> >     {
> > -      bp_pack_var_len_unsigned (bp, ee->parm_index);
> > +      bp_pack_var_len_int (bp, ee->parm_index);
> >       bp_pack_var_len_unsigned (bp, ee->arg);
> >       bp_pack_var_len_unsigned (bp, ee->min_flags);
> >       bp_pack_value (bp, ee->direct, 1);
> > @@ -2571,7 +2627,7 @@ modref_read_escape_summary (struct bitpack_d *bp, cgraph_edge *e)
> >   for (unsigned int i = 0; i < n; i++)
> >     {
> >       escape_entry ee;
> > -      ee.parm_index = bp_unpack_var_len_unsigned (bp);
> > +      ee.parm_index = bp_unpack_var_len_int (bp);
> >       ee.arg = bp_unpack_var_len_unsigned (bp);
> >       ee.min_flags = bp_unpack_var_len_unsigned (bp);
> >       ee.direct = bp_unpack_value (bp, 1);
> > @@ -2628,6 +2684,7 @@ modref_write ()
> > 	  streamer_write_uhwi (ob, r->arg_flags.length ());
> > 	  for (unsigned int i = 0; i < r->arg_flags.length (); i++)
> > 	    streamer_write_uhwi (ob, r->arg_flags[i]);
> > +	  streamer_write_uhwi (ob, r->retslot_flags);
> > 
> > 	  write_modref_records (r->loads, ob);
> > 	  write_modref_records (r->stores, ob);
> > @@ -2724,6 +2781,11 @@ read_section (struct lto_file_decl_data *file_data, const char *data,
> > 	  if (modref_sum_lto)
> > 	    modref_sum_lto->arg_flags.quick_push (flags);
> > 	}
> > +      eaf_flags_t flags = streamer_read_uhwi (&ib);
> > +      if (modref_sum)
> > +	modref_sum->retslot_flags = flags;
> > +      if (modref_sum_lto)
> > +	modref_sum_lto->retslot_flags = flags;
> >       read_modref_records (&ib, data_in,
> > 			   modref_sum ? &modref_sum->loads : NULL,
> > 			   modref_sum_lto ? &modref_sum_lto->loads : NULL);
> > @@ -3098,7 +3160,7 @@ struct escape_map
> >   bool direct;
> > };
> > 
> > -/* Update escape map fo E.  */
> > +/* Update escape map for E.  */
> > 
> > static void
> > update_escape_summary_1 (cgraph_edge *e,
> > @@ -3117,7 +3179,10 @@ update_escape_summary_1 (cgraph_edge *e,
> >     {
> >       unsigned int j;
> >       struct escape_map *em;
> > -      if (ee->parm_index >= map.length ())
> > +      /* TODO: We do not have jump functions for return slots, so we
> > +	 never propagate them to outer function.  */
> > +      if (ee->parm_index >= (int)map.length ()
> > +	  || ee->parm_index < 0)
> > 	continue;
> >       FOR_EACH_VEC_ELT (map[ee->parm_index], j, em)
> > 	{
> > @@ -3125,7 +3190,7 @@ update_escape_summary_1 (cgraph_edge *e,
> > 	  if (ee->direct && !em->direct)
> > 	    min_flags = deref_flags (min_flags, ignore_stores);
> > 	  struct escape_entry entry = {em->parm_index, ee->arg,
> > -	    			       ee->min_flags,
> > +				       ee->min_flags,
> > 				       ee->direct & em->direct};
> > 	  sum->esc.safe_push (entry);
> > 	}
> > @@ -3245,7 +3310,11 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
> >     FOR_EACH_VEC_ELT (sum->esc, i, ee)
> >       {
> > 	bool needed = false;
> > -	if (to_info && to_info->arg_flags.length () > ee->parm_index)
> > +	/* TODO: We do not have jump functions for return slots, so we
> > +	   never propagate them to outer function.  */
> > +	if (ee->parm_index < 0)
> > +	  continue;
> > +	if (to_info && (int)to_info->arg_flags.length () > ee->parm_index)
> > 	  {
> > 	    int flags = callee_info
> > 			&& callee_info->arg_flags.length () > ee->arg
> > @@ -3259,7 +3328,7 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
> > 	    if (to_info->arg_flags[ee->parm_index])
> > 	      needed = true;
> > 	  }
> > -	if (to_info_lto && to_info_lto->arg_flags.length () > ee->parm_index)
> > +	if (to_info_lto && (int)to_info_lto->arg_flags.length () > ee->parm_index)
> > 	  {
> > 	    int flags = callee_info_lto
> > 			&& callee_info_lto->arg_flags.length () > ee->arg
> > @@ -3798,29 +3867,31 @@ modref_merge_call_site_flags (escape_summary *sum,
> >       if (flags_lto & EAF_NOESCAPE)
> > 	flags_lto |= EAF_NODIRECTESCAPE;
> >       if (!(flags & EAF_UNUSED)
> > -	  && cur_summary && ee->parm_index < cur_summary->arg_flags.length ())
> > +	  && cur_summary && ee->parm_index < (int)cur_summary->arg_flags.length ())
> > 	{
> > -	  int f = cur_summary->arg_flags[ee->parm_index];
> > +	  eaf_flags_t &f = ee->parm_index == escape_point::retslot_arg
> > +			   ? cur_summary->retslot_flags
> > +			   : cur_summary->arg_flags[ee->parm_index];
> > 	  if ((f & flags) != f)
> > 	    {
> > 	      f = remove_useless_eaf_flags
> > 			 (f & flags, ecf_flags,
> > 			  VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
> > -	      cur_summary->arg_flags[ee->parm_index] = f;
> > 	      changed = true;
> > 	    }
> > 	}
> >       if (!(flags_lto & EAF_UNUSED)
> > 	  && cur_summary_lto
> > -	  && ee->parm_index < cur_summary_lto->arg_flags.length ())
> > +	  && ee->parm_index < (int)cur_summary_lto->arg_flags.length ())
> > 	{
> > -	  int f = cur_summary_lto->arg_flags[ee->parm_index];
> > +	  eaf_flags_t &f = ee->parm_index == escape_point::retslot_arg
> > +			   ? cur_summary_lto->retslot_flags
> > +			   : cur_summary_lto->arg_flags[ee->parm_index];
> > 	  if ((f & flags_lto) != f)
> > 	    {
> > 	      f = remove_useless_eaf_flags
> > 			 (f & flags_lto, ecf_flags,
> > 			  VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
> > -	      cur_summary_lto->arg_flags[ee->parm_index] = f;
> > 	      changed = true;
> > 	    }
> > 	}
> > diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
> > index 5afa3aa439f..a4db27471eb 100644
> > --- a/gcc/ipa-modref.h
> > +++ b/gcc/ipa-modref.h
> > @@ -31,6 +31,7 @@ struct GTY(()) modref_summary
> >   modref_records *loads;
> >   modref_records *stores;
> >   auto_vec<eaf_flags_t> GTY((skip)) arg_flags;
> > +  eaf_flags_t retslot_flags;
> >   bool writes_errno;
> > 
> >   modref_summary ();
> > diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
> > index 35971a54e02..99072df0768 100644
> > --- a/gcc/tree-ssa-structalias.c
> > +++ b/gcc/tree-ssa-structalias.c
> > @@ -4254,17 +4254,28 @@ handle_rhs_call (gcall *stmt, vec<ce_s> *results,
> >       && gimple_call_lhs (stmt) != NULL_TREE
> >       && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (stmt))))
> >     {
> > -      auto_vec<ce_s> tmpc;
> > -      struct constraint_expr *c;
> > -      unsigned i;
> > +      int flags = gimple_call_retslot_flags (stmt);
> > +      if ((flags & (EAF_NOESCAPE | EAF_NOT_RETURNED))
> > +	  != (EAF_NOESCAPE | EAF_NOT_RETURNED))
> > +	{
> > +	  auto_vec<ce_s> tmpc;
> > 
> > -      get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
> > +	  get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
> > 
> > -      make_constraints_to (callescape->id, tmpc);
> > -      if (writes_global_memory)
> > -	make_constraints_to (escaped_id, tmpc);
> > -      FOR_EACH_VEC_ELT (tmpc, i, c)
> > -	results->safe_push (*c);
> > +	  if (!(flags & (EAF_NOESCAPE | EAF_NODIRECTESCAPE)))
> > +	    {
> > +	      make_constraints_to (callescape->id, tmpc);
> > +	      if (writes_global_memory)
> > +		make_constraints_to (escaped_id, tmpc);
> > +	    }
> > +	  if (!(flags & EAF_NOT_RETURNED))
> > +	    {
> > +	      struct constraint_expr *c;
> > +	      unsigned i;
> > +	      FOR_EACH_VEC_ELT (tmpc, i, c)
> > +		results->safe_push (*c);
> > +	    }
> > +	}
> >     }
> > }
> > 
> > </cut>
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [TCWG CI] Regression caused by gcc: handle retslot in modref
       [not found]   ` <20211103151227.GA60321@kam.mff.cuni.cz>
@ 2021-11-03 16:37     ` Maxim Kuvyrkov
  2021-11-03 23:46       ` Jan Hubicka
  0 siblings, 1 reply; 5+ messages in thread
From: Maxim Kuvyrkov @ 2021-11-03 16:37 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-regression

Hi Jan,

Indeed, this seems to be fixed.  Yesterday’s build scored “1” (built binutils only), but build I triggered just now scored “2” (built binutils and bootstrap_lto) [*] .

Thanks!

[*] https://ci.linaro.org/job/tcwg_gcc_bootstrap-build-master-aarch64-bootstrap_lto/

--
Maxim Kuvyrkov
https://www.linaro.org

> On 3 Nov 2021, at 18:12, Jan Hubicka <hubicka@kam.mff.cuni.cz> wrote:
> 
>> Hi Jan,
>> 
>> Just wanted to make sure this is on your radar.
>> 
>> Your "handle retslot in modref” patch broken LTO bootstrap on aarch64 in all configurations that we test:
>> 
>> - tcwg_gcc_bootstrap/master-aarch64-bootstrap_lto
>> - tcwg_gcc_bootstrap/master-aarch64-bootstrap_profiled_lto
>> - tcwg_gcc_bootstrap/master-aarch64-bootstrap_profiled_lto_lean
> Maxim,
> I am trying lto build on other machine and lets see.  I wonder - I have
> fixed a wrong code bug related to that patch today (causing
> misopitmization of omnetpp). Is there any chance that this bug got
> fixed?
> 
> Honza
>> 
>> Regards,
>> 
>> --
>> Maxim Kuvyrkov
>> https://www.linaro.org
>> 
>>> On 30 Oct 2021, at 05:32, ci_notify@linaro.org wrote:
>>> 
>>> [TCWG CI] Regression caused by gcc: handle retslot in modref:
>>> commit b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
>>> Author: Jan Hubicka <hubicka@ucw.cz>
>>> 
>>>   handle retslot in modref
>>> 
>>> Results regressed to
>>> # reset_artifacts:
>>> -10
>>> # true:
>>> 0
>>> # build_abe binutils:
>>> 1
>>> # First few build errors in logs:
>>> # 00:04:02 make[3]: [Makefile:1772: aarch64-unknown-linux-gnu/bits/largefile-config.h] Error 1 (ignored)
>>> # 00:04:02 make[3]: [Makefile:1773: aarch64-unknown-linux-gnu/bits/largefile-config.h] Error 1 (ignored)
>>> # 00:19:19 checking for suffix of object files... configure: error: in `/home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gcc-gcc.git~master-stage2/aarch64-unknown-linux-gnu/libgcc':
>>> # 00:19:19 configure: error: cannot compute suffix of object files: cannot compile
>>> # 00:19:19 make[2]: *** [Makefile:19796: configure-stage2-target-libgcc] Error 1
>>> # 00:19:19 make[1]: *** [Makefile:25351: stage2-bubble] Error 2
>>> # 00:19:19 make: *** [Makefile:1019: all] Error 2
>>> 
>>> from
>>> # reset_artifacts:
>>> -10
>>> # true:
>>> 0
>>> # build_abe binutils:
>>> 1
>>> # build_abe bootstrap_lto:
>>> 2
>>> 
>>> THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.
>>> 
>>> This commit has regressed these CI configurations:
>>> - tcwg_gcc_bootstrap/master-aarch64-bootstrap_lto
>>> 
>>> First_bad build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9/
>>> Last_good build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-4045d5fa42f2ee7b284977c8f2f0edc300a63e43/
>>> Baseline build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-baseline/
>>> Even more details: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/
>>> 
>>> Reproduce builds:
>>> <cut>
>>> mkdir investigate-gcc-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
>>> cd investigate-gcc-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
>>> 
>>> # Fetch scripts
>>> git clone https://git.linaro.org/toolchain/jenkins-scripts
>>> 
>>> # Fetch manifests and test.sh script
>>> mkdir -p artifacts/manifests
>>> curl -o artifacts/manifests/build-baseline.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/manifests/build-baseline.sh --fail
>>> curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/manifests/build-parameters.sh --fail
>>> curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/test.sh --fail
>>> chmod +x artifacts/test.sh
>>> 
>>> # Reproduce the baseline build (build all pre-requisites)
>>> ./jenkins-scripts/tcwg_gnu-build.sh @@ artifacts/manifests/build-baseline.sh
>>> 
>>> # Save baseline build state (which is then restored in artifacts/test.sh)
>>> mkdir -p ./bisect
>>> rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ --exclude /gcc/ ./ ./bisect/baseline/
>>> 
>>> cd gcc
>>> 
>>> # Reproduce first_bad build
>>> git checkout --detach b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
>>> ../artifacts/test.sh
>>> 
>>> # Reproduce last_good build
>>> git checkout --detach 4045d5fa42f2ee7b284977c8f2f0edc300a63e43
>>> ../artifacts/test.sh
>>> 
>>> cd ..
>>> </cut>
>>> 
>>> Full commit (up to 1000 lines):
>>> <cut>
>>> commit b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
>>> Author: Jan Hubicka <hubicka@ucw.cz>
>>> Date:   Fri Oct 29 16:01:51 2021 +0200
>>> 
>>>   handle retslot in modref
>>> 
>>>   Extend modref and tree-ssa-structalias to handle retslot flags.
>>>   Since retslot it essentially a hidden argument that is known to be write-only
>>>   we can do pretty much the same stuff as we do for regular parameters.
>>>   I plan to add static chain handling similar way.
>>> 
>>>   We do not handle IPA propagation of retslot flags (where return slot is
>>>   initialized via return slot of other function). For this ipa-prop needs
>>>   to be extended to understand retslot as well.
>>> 
>>>   Bootstrapped/regtested x86_64-linux, OK for the gimple bits?
>>> 
>>>   Honza
>>> 
>>>   gcc/ChangeLog:
>>> 
>>>           * gimple.c (gimple_call_retslot_flags): New function.
>>>           * gimple.h (gimple_call_retslot_flags): Declare.
>>>           * ipa-modref.c: Include tree-cfg.h.
>>>           (struct escape_entry): Turn parm_index to signed.
>>>           (modref_summary_lto::modref_summary_lto): Add retslot_flags.
>>>           (modref_summary::modref_summary): Initialize retslot_flags.
>>>           (struct modref_summary_lto): Likewise.
>>>           (modref_summary::useful_p): Check retslot_flags.
>>>           (modref_summary_lto::useful_p): Likewise.
>>>           (modref_summary::dump): Dump retslot_flags.
>>>           (modref_summary_lto::dump): Likewise.
>>>           (struct escape_point): Add hidden_args enum.
>>>           (analyze_ssa_name_flags): Ignore return slot return;
>>>           use gimple_call_retslot_flags.
>>>           (record_escape_points): Break out from ...
>>>           (analyze_parms): ... here; handle retslot_flags.
>>>           (modref_summaries::duplicate): Duplicate retslot_flags.
>>>           (modref_summaries_lto::duplicate): Likewise.
>>>           (modref_write_escape_summary): Stream parm_index as signed.
>>>           (modref_read_escape_summary): Likewise.
>>>           (modref_write): Stream retslot_flags.
>>>           (read_section): Likewise.
>>>           (struct escape_map): Fix typo in comment.
>>>           (update_escape_summary_1): Fix whitespace.
>>>           (ipa_merge_modref_summary_after_inlining): Drop retslot_flags.
>>>           (modref_merge_call_site_flags): Merge retslot_flags.
>>>           * ipa-modref.h (struct modref_summary): Add retslot_flags.
>>>           * tree-ssa-structalias.c (handle_rhs_call): Handle retslot_flags.
>>> ---
>>> gcc/gimple.c               |  41 ++++++++++++-
>>> gcc/gimple.h               |   1 +
>>> gcc/ipa-modref.c           | 145 +++++++++++++++++++++++++++++++++------------
>>> gcc/ipa-modref.h           |   1 +
>>> gcc/tree-ssa-structalias.c |  29 ++++++---
>>> 5 files changed, 170 insertions(+), 47 deletions(-)
>>> 
>>> diff --git a/gcc/gimple.c b/gcc/gimple.c
>>> index cc7a88e822b..22dd6417d19 100644
>>> --- a/gcc/gimple.c
>>> +++ b/gcc/gimple.c
>>> @@ -1597,7 +1597,12 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
>>> 	  if (!node->binds_to_current_def_p ())
>>> 	    {
>>> 	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
>>> -		modref_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;
>>> 	    }
>>> @@ -1608,6 +1613,40 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
>>>  return flags;
>>> }
>>> 
>>> +/* Detects argument flags for return slot on call STMT.  */
>>> +
>>> +int
>>> +gimple_call_retslot_flags (const gcall *stmt)
>>> +{
>>> +  int flags = EAF_DIRECT | EAF_NOREAD;
>>> +
>>> +  tree callee = gimple_call_fndecl (stmt);
>>> +  if (callee)
>>> +    {
>>> +      cgraph_node *node = cgraph_node::get (callee);
>>> +      modref_summary *summary = node ? get_modref_function_summary (node)
>>> +				: NULL;
>>> +
>>> +      if (summary)
>>> +	{
>>> +	  int modref_flags = summary->retslot_flags;
>>> +
>>> +	  /* 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 (dbg_cnt (ipa_mod_ref_pta))
>>> +	    flags |= modref_flags;
>>> +	}
>>> +    }
>>> +  return flags;
>>> +}
>>> +
>>> /* Detects return flags for the call STMT.  */
>>> 
>>> int
>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>> index 303623b3ced..23a124ec769 100644
>>> --- a/gcc/gimple.h
>>> +++ b/gcc/gimple.h
>>> @@ -1589,6 +1589,7 @@ gimple_seq gimple_seq_copy (gimple_seq);
>>> bool gimple_call_same_target_p (const gimple *, const gimple *);
>>> int gimple_call_flags (const gimple *);
>>> int gimple_call_arg_flags (const gcall *, unsigned);
>>> +int gimple_call_retslot_flags (const gcall *);
>>> int gimple_call_return_flags (const gcall *);
>>> bool gimple_call_nonnull_result_p (gcall *);
>>> tree gimple_call_nonnull_arg (gcall *);
>>> diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
>>> index 0bbec8df0a2..3539cb43d11 100644
>>> --- a/gcc/ipa-modref.c
>>> +++ b/gcc/ipa-modref.c
>>> @@ -86,6 +86,7 @@ along with GCC; see the file COPYING3.  If not see
>>> #include "stringpool.h"
>>> #include "tree-ssanames.h"
>>> #include "attribs.h"
>>> +#include "tree-cfg.h"
>>> 
>>> 
>>> namespace {
>>> @@ -133,7 +134,7 @@ static fnspec_summaries_t *fnspec_summaries = NULL;
>>> struct escape_entry
>>> {
>>>  /* Parameter that escapes at a given call.  */
>>> -  unsigned int parm_index;
>>> +  int parm_index;
>>>  /* Argument it escapes to.  */
>>>  unsigned int arg;
>>>  /* Minimal flags known about the argument.  */
>>> @@ -269,7 +270,7 @@ static GTY(()) fast_function_summary <modref_summary_lto *, va_gc>
>>> /* Summary for a single function which this pass produces.  */
>>> 
>>> modref_summary::modref_summary ()
>>> -  : loads (NULL), stores (NULL), writes_errno (NULL)
>>> +  : loads (NULL), stores (NULL), retslot_flags (0), writes_errno (false)
>>> {
>>> }
>>> 
>>> @@ -322,6 +323,8 @@ modref_summary::useful_p (int ecf_flags, bool check_flags)
>>>  if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
>>>    return true;
>>>  arg_flags.release ();
>>> +  if (check_flags && remove_useless_eaf_flags (retslot_flags, ecf_flags, false))
>>> +    return true;
>>>  if (ecf_flags & ECF_CONST)
>>>    return false;
>>>  if (loads && !loads->every_base)
>>> @@ -363,6 +366,7 @@ struct GTY(()) modref_summary_lto
>>>  modref_records_lto *loads;
>>>  modref_records_lto *stores;
>>>  auto_vec<eaf_flags_t> GTY((skip)) arg_flags;
>>> +  eaf_flags_t retslot_flags;
>>>  bool writes_errno;
>>> 
>>>  modref_summary_lto ();
>>> @@ -374,7 +378,7 @@ struct GTY(()) modref_summary_lto
>>> /* Summary for a single function which this pass produces.  */
>>> 
>>> modref_summary_lto::modref_summary_lto ()
>>> -  : loads (NULL), stores (NULL), writes_errno (NULL)
>>> +  : loads (NULL), stores (NULL), retslot_flags (0), writes_errno (false)
>>> {
>>> }
>>> 
>>> @@ -400,6 +404,8 @@ modref_summary_lto::useful_p (int ecf_flags, bool check_flags)
>>>  if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
>>>    return true;
>>>  arg_flags.release ();
>>> +  if (check_flags && remove_useless_eaf_flags (retslot_flags, ecf_flags, false))
>>> +    return true;
>>>  if (ecf_flags & ECF_CONST)
>>>    return false;
>>>  if (loads && !loads->every_base)
>>> @@ -608,6 +614,11 @@ modref_summary::dump (FILE *out)
>>> 	    dump_eaf_flags (out, arg_flags[i]);
>>> 	  }
>>>    }
>>> +  if (retslot_flags)
>>> +    {
>>> +      fprintf (out, "  Retslot flags:");
>>> +      dump_eaf_flags (out, retslot_flags);
>>> +    }
>>> }
>>> 
>>> /* Dump summary.  */
>>> @@ -630,6 +641,11 @@ modref_summary_lto::dump (FILE *out)
>>> 	    dump_eaf_flags (out, arg_flags[i]);
>>> 	  }
>>>    }
>>> +  if (retslot_flags)
>>> +    {
>>> +      fprintf (out, "  Retslot flags:");
>>> +      dump_eaf_flags (out, retslot_flags);
>>> +    }
>>> }
>>> 
>>> /* Get function summary for FUNC if it exists, return NULL otherwise.  */
>>> @@ -1396,6 +1412,11 @@ namespace {
>>> 
>>> struct escape_point
>>> {
>>> +  /* Extra hidden args we keep track of.  */
>>> +  enum hidden_args
>>> +  {
>>> +    retslot_arg = -1
>>> +  };
>>>  /* Value escapes to this call.  */
>>>  gcall *call;
>>>  /* Argument it escapes to.  */
>>> @@ -1705,7 +1726,11 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
>>> 	 Returning name counts as an use by tree-ssa-structalias.c  */
>>>      if (greturn *ret = dyn_cast <greturn *> (use_stmt))
>>> 	{
>>> -	  if (gimple_return_retval (ret) == name)
>>> +	  /* Returning through return slot is seen as memory write earlier.  */
>>> +	  if (DECL_RESULT (current_function_decl)
>>> +	      && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
>>> +	    ;
>>> +	  else if (gimple_return_retval (ret) == name)
>>> 	    lattice[index].merge (~(EAF_UNUSED | EAF_NOT_RETURNED));
>>> 	  else if (memory_access_to (gimple_return_retval (ret), name))
>>> 	    {
>>> @@ -1748,7 +1773,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
>>> 		     may make LHS to escape.  See PR 98499.  */
>>> 		  if (gimple_call_return_slot_opt_p (call)
>>> 		      && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (call))))
>>> -		    lattice[index].merge (EAF_NOREAD | EAF_DIRECT);
>>> +		    lattice[index].merge (gimple_call_retslot_flags (call));
>>> 		}
>>> 
>>> 	      /* We do not track accesses to the static chain (we could)
>>> @@ -1777,7 +1802,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
>>> 			  lattice[index].merge (call_flags);
>>> 			else
>>> 			  lattice[index].add_escape_point (call, i,
>>> -			     				   call_flags, true);
>>> +							   call_flags, true);
>>> 		      }
>>> 		    if (!ignore_retval)
>>> 		      merge_call_lhs_flags (call, i, index, false,
>>> @@ -1912,6 +1937,29 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
>>>  lattice[index].known = true;
>>> }
>>> 
>>> +/* Record escape points of PARM_INDEX according to LATTICE.  */
>>> +
>>> +static void
>>> +record_escape_points (modref_lattice &lattice, int parm_index, int flags)
>>> +{
>>> +  if (lattice.escape_points.length ())
>>> +    {
>>> +      escape_point *ep;
>>> +      unsigned int ip;
>>> +      cgraph_node *node = cgraph_node::get (current_function_decl);
>>> +
>>> +      FOR_EACH_VEC_ELT (lattice.escape_points, ip, ep)
>>> +	if ((ep->min_flags & flags) != flags)
>>> +	  {
>>> +	    cgraph_edge *e = node->get_edge (ep->call);
>>> +	    struct escape_entry ee = {parm_index, ep->arg,
>>> +				      ep->min_flags, ep->direct};
>>> +
>>> +	    escape_summaries->get_create (e)->esc.safe_push (ee);
>>> +	  }
>>> +    }
>>> +}
>>> +
>>> /* Determine EAF flags for function parameters.  */
>>> 
>>> static void
>>> @@ -1921,16 +1969,22 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto,
>>>  unsigned int parm_index = 0;
>>>  unsigned int count = 0;
>>>  int ecf_flags = flags_from_decl_or_type (current_function_decl);
>>> +  tree retslot = 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)))
>>> +    retslot = ssa_default_def (cfun, DECL_RESULT (current_function_decl));
>>> +
>>>  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
>>>       parm = TREE_CHAIN (parm))
>>>    count++;
>>> 
>>> -  if (!count)
>>> +  if (!count && !retslot)
>>>    return;
>>> 
>>>  auto_vec<modref_lattice> lattice;
>>> @@ -1984,24 +2038,24 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto,
>>> 		summary_lto->arg_flags.safe_grow_cleared (count, true);
>>> 	      summary_lto->arg_flags[parm_index] = flags;
>>> 	    }
>>> -	  if (lattice[SSA_NAME_VERSION (name)].escape_points.length ())
>>> -	    {
>>> -	      escape_point *ep;
>>> -	      unsigned int ip;
>>> -	      cgraph_node *node = cgraph_node::get (current_function_decl);
>>> -
>>> -	      gcc_checking_assert (ipa);
>>> -	      FOR_EACH_VEC_ELT
>>> -		  (lattice[SSA_NAME_VERSION (name)].escape_points, ip, ep)
>>> -		if ((ep->min_flags & flags) != flags)
>>> -		  {
>>> -		    cgraph_edge *e = node->get_edge (ep->call);
>>> -		    struct escape_entry ee = {parm_index, ep->arg,
>>> -		     			      ep->min_flags, ep->direct};
>>> +	  record_escape_points (lattice[SSA_NAME_VERSION (name)],
>>> +				parm_index, flags);
>>> +	}
>>> +    }
>>> +  if (retslot)
>>> +    {
>>> +      analyze_ssa_name_flags (retslot, lattice, 0, ipa);
>>> +      int flags = lattice[SSA_NAME_VERSION (retslot)].flags;
>>> 
>>> -		    escape_summaries->get_create (e)->esc.safe_push (ee);
>>> -		  }
>>> -	    }
>>> +      flags = remove_useless_eaf_flags (flags, ecf_flags, false);
>>> +      if (flags)
>>> +	{
>>> +	  if (summary)
>>> +	    summary->retslot_flags = flags;
>>> +	  if (summary_lto)
>>> +	    summary_lto->retslot_flags = flags;
>>> +	  record_escape_points (lattice[SSA_NAME_VERSION (retslot)],
>>> +				escape_point::retslot_arg, flags);
>>> 	}
>>>    }
>>>  if (ipa)
>>> @@ -2287,6 +2341,7 @@ modref_summaries::duplicate (cgraph_node *, cgraph_node *dst,
>>>  dst_data->writes_errno = src_data->writes_errno;
>>>  if (src_data->arg_flags.length ())
>>>    dst_data->arg_flags = src_data->arg_flags.copy ();
>>> +  dst_data->retslot_flags = src_data->retslot_flags;
>>> }
>>> 
>>> /* Called when new clone is inserted to callgraph late.  */
>>> @@ -2312,6 +2367,7 @@ modref_summaries_lto::duplicate (cgraph_node *, cgraph_node *,
>>>  dst_data->writes_errno = src_data->writes_errno;
>>>  if (src_data->arg_flags.length ())
>>>    dst_data->arg_flags = src_data->arg_flags.copy ();
>>> +  dst_data->retslot_flags = src_data->retslot_flags;
>>> }
>>> 
>>> namespace
>>> @@ -2551,7 +2607,7 @@ modref_write_escape_summary (struct bitpack_d *bp, escape_summary *esum)
>>>  escape_entry *ee;
>>>  FOR_EACH_VEC_ELT (esum->esc, i, ee)
>>>    {
>>> -      bp_pack_var_len_unsigned (bp, ee->parm_index);
>>> +      bp_pack_var_len_int (bp, ee->parm_index);
>>>      bp_pack_var_len_unsigned (bp, ee->arg);
>>>      bp_pack_var_len_unsigned (bp, ee->min_flags);
>>>      bp_pack_value (bp, ee->direct, 1);
>>> @@ -2571,7 +2627,7 @@ modref_read_escape_summary (struct bitpack_d *bp, cgraph_edge *e)
>>>  for (unsigned int i = 0; i < n; i++)
>>>    {
>>>      escape_entry ee;
>>> -      ee.parm_index = bp_unpack_var_len_unsigned (bp);
>>> +      ee.parm_index = bp_unpack_var_len_int (bp);
>>>      ee.arg = bp_unpack_var_len_unsigned (bp);
>>>      ee.min_flags = bp_unpack_var_len_unsigned (bp);
>>>      ee.direct = bp_unpack_value (bp, 1);
>>> @@ -2628,6 +2684,7 @@ modref_write ()
>>> 	  streamer_write_uhwi (ob, r->arg_flags.length ());
>>> 	  for (unsigned int i = 0; i < r->arg_flags.length (); i++)
>>> 	    streamer_write_uhwi (ob, r->arg_flags[i]);
>>> +	  streamer_write_uhwi (ob, r->retslot_flags);
>>> 
>>> 	  write_modref_records (r->loads, ob);
>>> 	  write_modref_records (r->stores, ob);
>>> @@ -2724,6 +2781,11 @@ read_section (struct lto_file_decl_data *file_data, const char *data,
>>> 	  if (modref_sum_lto)
>>> 	    modref_sum_lto->arg_flags.quick_push (flags);
>>> 	}
>>> +      eaf_flags_t flags = streamer_read_uhwi (&ib);
>>> +      if (modref_sum)
>>> +	modref_sum->retslot_flags = flags;
>>> +      if (modref_sum_lto)
>>> +	modref_sum_lto->retslot_flags = flags;
>>>      read_modref_records (&ib, data_in,
>>> 			   modref_sum ? &modref_sum->loads : NULL,
>>> 			   modref_sum_lto ? &modref_sum_lto->loads : NULL);
>>> @@ -3098,7 +3160,7 @@ struct escape_map
>>>  bool direct;
>>> };
>>> 
>>> -/* Update escape map fo E.  */
>>> +/* Update escape map for E.  */
>>> 
>>> static void
>>> update_escape_summary_1 (cgraph_edge *e,
>>> @@ -3117,7 +3179,10 @@ update_escape_summary_1 (cgraph_edge *e,
>>>    {
>>>      unsigned int j;
>>>      struct escape_map *em;
>>> -      if (ee->parm_index >= map.length ())
>>> +      /* TODO: We do not have jump functions for return slots, so we
>>> +	 never propagate them to outer function.  */
>>> +      if (ee->parm_index >= (int)map.length ()
>>> +	  || ee->parm_index < 0)
>>> 	continue;
>>>      FOR_EACH_VEC_ELT (map[ee->parm_index], j, em)
>>> 	{
>>> @@ -3125,7 +3190,7 @@ update_escape_summary_1 (cgraph_edge *e,
>>> 	  if (ee->direct && !em->direct)
>>> 	    min_flags = deref_flags (min_flags, ignore_stores);
>>> 	  struct escape_entry entry = {em->parm_index, ee->arg,
>>> -	    			       ee->min_flags,
>>> +				       ee->min_flags,
>>> 				       ee->direct & em->direct};
>>> 	  sum->esc.safe_push (entry);
>>> 	}
>>> @@ -3245,7 +3310,11 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
>>>    FOR_EACH_VEC_ELT (sum->esc, i, ee)
>>>      {
>>> 	bool needed = false;
>>> -	if (to_info && to_info->arg_flags.length () > ee->parm_index)
>>> +	/* TODO: We do not have jump functions for return slots, so we
>>> +	   never propagate them to outer function.  */
>>> +	if (ee->parm_index < 0)
>>> +	  continue;
>>> +	if (to_info && (int)to_info->arg_flags.length () > ee->parm_index)
>>> 	  {
>>> 	    int flags = callee_info
>>> 			&& callee_info->arg_flags.length () > ee->arg
>>> @@ -3259,7 +3328,7 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
>>> 	    if (to_info->arg_flags[ee->parm_index])
>>> 	      needed = true;
>>> 	  }
>>> -	if (to_info_lto && to_info_lto->arg_flags.length () > ee->parm_index)
>>> +	if (to_info_lto && (int)to_info_lto->arg_flags.length () > ee->parm_index)
>>> 	  {
>>> 	    int flags = callee_info_lto
>>> 			&& callee_info_lto->arg_flags.length () > ee->arg
>>> @@ -3798,29 +3867,31 @@ modref_merge_call_site_flags (escape_summary *sum,
>>>      if (flags_lto & EAF_NOESCAPE)
>>> 	flags_lto |= EAF_NODIRECTESCAPE;
>>>      if (!(flags & EAF_UNUSED)
>>> -	  && cur_summary && ee->parm_index < cur_summary->arg_flags.length ())
>>> +	  && cur_summary && ee->parm_index < (int)cur_summary->arg_flags.length ())
>>> 	{
>>> -	  int f = cur_summary->arg_flags[ee->parm_index];
>>> +	  eaf_flags_t &f = ee->parm_index == escape_point::retslot_arg
>>> +			   ? cur_summary->retslot_flags
>>> +			   : cur_summary->arg_flags[ee->parm_index];
>>> 	  if ((f & flags) != f)
>>> 	    {
>>> 	      f = remove_useless_eaf_flags
>>> 			 (f & flags, ecf_flags,
>>> 			  VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
>>> -	      cur_summary->arg_flags[ee->parm_index] = f;
>>> 	      changed = true;
>>> 	    }
>>> 	}
>>>      if (!(flags_lto & EAF_UNUSED)
>>> 	  && cur_summary_lto
>>> -	  && ee->parm_index < cur_summary_lto->arg_flags.length ())
>>> +	  && ee->parm_index < (int)cur_summary_lto->arg_flags.length ())
>>> 	{
>>> -	  int f = cur_summary_lto->arg_flags[ee->parm_index];
>>> +	  eaf_flags_t &f = ee->parm_index == escape_point::retslot_arg
>>> +			   ? cur_summary_lto->retslot_flags
>>> +			   : cur_summary_lto->arg_flags[ee->parm_index];
>>> 	  if ((f & flags_lto) != f)
>>> 	    {
>>> 	      f = remove_useless_eaf_flags
>>> 			 (f & flags_lto, ecf_flags,
>>> 			  VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
>>> -	      cur_summary_lto->arg_flags[ee->parm_index] = f;
>>> 	      changed = true;
>>> 	    }
>>> 	}
>>> diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
>>> index 5afa3aa439f..a4db27471eb 100644
>>> --- a/gcc/ipa-modref.h
>>> +++ b/gcc/ipa-modref.h
>>> @@ -31,6 +31,7 @@ struct GTY(()) modref_summary
>>>  modref_records *loads;
>>>  modref_records *stores;
>>>  auto_vec<eaf_flags_t> GTY((skip)) arg_flags;
>>> +  eaf_flags_t retslot_flags;
>>>  bool writes_errno;
>>> 
>>>  modref_summary ();
>>> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
>>> index 35971a54e02..99072df0768 100644
>>> --- a/gcc/tree-ssa-structalias.c
>>> +++ b/gcc/tree-ssa-structalias.c
>>> @@ -4254,17 +4254,28 @@ handle_rhs_call (gcall *stmt, vec<ce_s> *results,
>>>      && gimple_call_lhs (stmt) != NULL_TREE
>>>      && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (stmt))))
>>>    {
>>> -      auto_vec<ce_s> tmpc;
>>> -      struct constraint_expr *c;
>>> -      unsigned i;
>>> +      int flags = gimple_call_retslot_flags (stmt);
>>> +      if ((flags & (EAF_NOESCAPE | EAF_NOT_RETURNED))
>>> +	  != (EAF_NOESCAPE | EAF_NOT_RETURNED))
>>> +	{
>>> +	  auto_vec<ce_s> tmpc;
>>> 
>>> -      get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
>>> +	  get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
>>> 
>>> -      make_constraints_to (callescape->id, tmpc);
>>> -      if (writes_global_memory)
>>> -	make_constraints_to (escaped_id, tmpc);
>>> -      FOR_EACH_VEC_ELT (tmpc, i, c)
>>> -	results->safe_push (*c);
>>> +	  if (!(flags & (EAF_NOESCAPE | EAF_NODIRECTESCAPE)))
>>> +	    {
>>> +	      make_constraints_to (callescape->id, tmpc);
>>> +	      if (writes_global_memory)
>>> +		make_constraints_to (escaped_id, tmpc);
>>> +	    }
>>> +	  if (!(flags & EAF_NOT_RETURNED))
>>> +	    {
>>> +	      struct constraint_expr *c;
>>> +	      unsigned i;
>>> +	      FOR_EACH_VEC_ELT (tmpc, i, c)
>>> +		results->safe_push (*c);
>>> +	    }
>>> +	}
>>>    }
>>> }
>>> 
>>> </cut>
>> 



^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [TCWG CI] Regression caused by gcc: handle retslot in modref
  2021-11-03 16:37     ` Maxim Kuvyrkov
@ 2021-11-03 23:46       ` Jan Hubicka
  0 siblings, 0 replies; 5+ messages in thread
From: Jan Hubicka @ 2021-11-03 23:46 UTC (permalink / raw)
  To: Maxim Kuvyrkov; +Cc: gcc-regression

> Hi Jan,
> 
> Indeed, this seems to be fixed.  Yesterday’s build scored “1” (built binutils only), but build I triggered just now scored “2” (built binutils and bootstrap_lto) [*] .
> 
> Thanks!
Thanks too, that is good news :)

Honza
> 
> [*] https://ci.linaro.org/job/tcwg_gcc_bootstrap-build-master-aarch64-bootstrap_lto/
> 
> --
> Maxim Kuvyrkov
> https://www.linaro.org
> 
> > On 3 Nov 2021, at 18:12, Jan Hubicka <hubicka@kam.mff.cuni.cz> wrote:
> > 
> >> Hi Jan,
> >> 
> >> Just wanted to make sure this is on your radar.
> >> 
> >> Your "handle retslot in modref” patch broken LTO bootstrap on aarch64 in all configurations that we test:
> >> 
> >> - tcwg_gcc_bootstrap/master-aarch64-bootstrap_lto
> >> - tcwg_gcc_bootstrap/master-aarch64-bootstrap_profiled_lto
> >> - tcwg_gcc_bootstrap/master-aarch64-bootstrap_profiled_lto_lean
> > Maxim,
> > I am trying lto build on other machine and lets see.  I wonder - I have
> > fixed a wrong code bug related to that patch today (causing
> > misopitmization of omnetpp). Is there any chance that this bug got
> > fixed?
> > 
> > Honza
> >> 
> >> Regards,
> >> 
> >> --
> >> Maxim Kuvyrkov
> >> https://www.linaro.org
> >> 
> >>> On 30 Oct 2021, at 05:32, ci_notify@linaro.org wrote:
> >>> 
> >>> [TCWG CI] Regression caused by gcc: handle retslot in modref:
> >>> commit b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> >>> Author: Jan Hubicka <hubicka@ucw.cz>
> >>> 
> >>>   handle retslot in modref
> >>> 
> >>> Results regressed to
> >>> # reset_artifacts:
> >>> -10
> >>> # true:
> >>> 0
> >>> # build_abe binutils:
> >>> 1
> >>> # First few build errors in logs:
> >>> # 00:04:02 make[3]: [Makefile:1772: aarch64-unknown-linux-gnu/bits/largefile-config.h] Error 1 (ignored)
> >>> # 00:04:02 make[3]: [Makefile:1773: aarch64-unknown-linux-gnu/bits/largefile-config.h] Error 1 (ignored)
> >>> # 00:19:19 checking for suffix of object files... configure: error: in `/home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gcc-gcc.git~master-stage2/aarch64-unknown-linux-gnu/libgcc':
> >>> # 00:19:19 configure: error: cannot compute suffix of object files: cannot compile
> >>> # 00:19:19 make[2]: *** [Makefile:19796: configure-stage2-target-libgcc] Error 1
> >>> # 00:19:19 make[1]: *** [Makefile:25351: stage2-bubble] Error 2
> >>> # 00:19:19 make: *** [Makefile:1019: all] Error 2
> >>> 
> >>> from
> >>> # reset_artifacts:
> >>> -10
> >>> # true:
> >>> 0
> >>> # build_abe binutils:
> >>> 1
> >>> # build_abe bootstrap_lto:
> >>> 2
> >>> 
> >>> THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.
> >>> 
> >>> This commit has regressed these CI configurations:
> >>> - tcwg_gcc_bootstrap/master-aarch64-bootstrap_lto
> >>> 
> >>> First_bad build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9/
> >>> Last_good build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-4045d5fa42f2ee7b284977c8f2f0edc300a63e43/
> >>> Baseline build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/build-baseline/
> >>> Even more details: https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/
> >>> 
> >>> Reproduce builds:
> >>> <cut>
> >>> mkdir investigate-gcc-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> >>> cd investigate-gcc-b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> >>> 
> >>> # Fetch scripts
> >>> git clone https://git.linaro.org/toolchain/jenkins-scripts
> >>> 
> >>> # Fetch manifests and test.sh script
> >>> mkdir -p artifacts/manifests
> >>> curl -o artifacts/manifests/build-baseline.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/manifests/build-baseline.sh --fail
> >>> curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/manifests/build-parameters.sh --fail
> >>> curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-bisect-master-aarch64-bootstrap_lto/3/artifact/artifacts/test.sh --fail
> >>> chmod +x artifacts/test.sh
> >>> 
> >>> # Reproduce the baseline build (build all pre-requisites)
> >>> ./jenkins-scripts/tcwg_gnu-build.sh @@ artifacts/manifests/build-baseline.sh
> >>> 
> >>> # Save baseline build state (which is then restored in artifacts/test.sh)
> >>> mkdir -p ./bisect
> >>> rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ --exclude /gcc/ ./ ./bisect/baseline/
> >>> 
> >>> cd gcc
> >>> 
> >>> # Reproduce first_bad build
> >>> git checkout --detach b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> >>> ../artifacts/test.sh
> >>> 
> >>> # Reproduce last_good build
> >>> git checkout --detach 4045d5fa42f2ee7b284977c8f2f0edc300a63e43
> >>> ../artifacts/test.sh
> >>> 
> >>> cd ..
> >>> </cut>
> >>> 
> >>> Full commit (up to 1000 lines):
> >>> <cut>
> >>> commit b8ef019ab938471f7f877a1eee3a6374fd8a6ae9
> >>> Author: Jan Hubicka <hubicka@ucw.cz>
> >>> Date:   Fri Oct 29 16:01:51 2021 +0200
> >>> 
> >>>   handle retslot in modref
> >>> 
> >>>   Extend modref and tree-ssa-structalias to handle retslot flags.
> >>>   Since retslot it essentially a hidden argument that is known to be write-only
> >>>   we can do pretty much the same stuff as we do for regular parameters.
> >>>   I plan to add static chain handling similar way.
> >>> 
> >>>   We do not handle IPA propagation of retslot flags (where return slot is
> >>>   initialized via return slot of other function). For this ipa-prop needs
> >>>   to be extended to understand retslot as well.
> >>> 
> >>>   Bootstrapped/regtested x86_64-linux, OK for the gimple bits?
> >>> 
> >>>   Honza
> >>> 
> >>>   gcc/ChangeLog:
> >>> 
> >>>           * gimple.c (gimple_call_retslot_flags): New function.
> >>>           * gimple.h (gimple_call_retslot_flags): Declare.
> >>>           * ipa-modref.c: Include tree-cfg.h.
> >>>           (struct escape_entry): Turn parm_index to signed.
> >>>           (modref_summary_lto::modref_summary_lto): Add retslot_flags.
> >>>           (modref_summary::modref_summary): Initialize retslot_flags.
> >>>           (struct modref_summary_lto): Likewise.
> >>>           (modref_summary::useful_p): Check retslot_flags.
> >>>           (modref_summary_lto::useful_p): Likewise.
> >>>           (modref_summary::dump): Dump retslot_flags.
> >>>           (modref_summary_lto::dump): Likewise.
> >>>           (struct escape_point): Add hidden_args enum.
> >>>           (analyze_ssa_name_flags): Ignore return slot return;
> >>>           use gimple_call_retslot_flags.
> >>>           (record_escape_points): Break out from ...
> >>>           (analyze_parms): ... here; handle retslot_flags.
> >>>           (modref_summaries::duplicate): Duplicate retslot_flags.
> >>>           (modref_summaries_lto::duplicate): Likewise.
> >>>           (modref_write_escape_summary): Stream parm_index as signed.
> >>>           (modref_read_escape_summary): Likewise.
> >>>           (modref_write): Stream retslot_flags.
> >>>           (read_section): Likewise.
> >>>           (struct escape_map): Fix typo in comment.
> >>>           (update_escape_summary_1): Fix whitespace.
> >>>           (ipa_merge_modref_summary_after_inlining): Drop retslot_flags.
> >>>           (modref_merge_call_site_flags): Merge retslot_flags.
> >>>           * ipa-modref.h (struct modref_summary): Add retslot_flags.
> >>>           * tree-ssa-structalias.c (handle_rhs_call): Handle retslot_flags.
> >>> ---
> >>> gcc/gimple.c               |  41 ++++++++++++-
> >>> gcc/gimple.h               |   1 +
> >>> gcc/ipa-modref.c           | 145 +++++++++++++++++++++++++++++++++------------
> >>> gcc/ipa-modref.h           |   1 +
> >>> gcc/tree-ssa-structalias.c |  29 ++++++---
> >>> 5 files changed, 170 insertions(+), 47 deletions(-)
> >>> 
> >>> diff --git a/gcc/gimple.c b/gcc/gimple.c
> >>> index cc7a88e822b..22dd6417d19 100644
> >>> --- a/gcc/gimple.c
> >>> +++ b/gcc/gimple.c
> >>> @@ -1597,7 +1597,12 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
> >>> 	  if (!node->binds_to_current_def_p ())
> >>> 	    {
> >>> 	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
> >>> -		modref_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;
> >>> 	    }
> >>> @@ -1608,6 +1613,40 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
> >>>  return flags;
> >>> }
> >>> 
> >>> +/* Detects argument flags for return slot on call STMT.  */
> >>> +
> >>> +int
> >>> +gimple_call_retslot_flags (const gcall *stmt)
> >>> +{
> >>> +  int flags = EAF_DIRECT | EAF_NOREAD;
> >>> +
> >>> +  tree callee = gimple_call_fndecl (stmt);
> >>> +  if (callee)
> >>> +    {
> >>> +      cgraph_node *node = cgraph_node::get (callee);
> >>> +      modref_summary *summary = node ? get_modref_function_summary (node)
> >>> +				: NULL;
> >>> +
> >>> +      if (summary)
> >>> +	{
> >>> +	  int modref_flags = summary->retslot_flags;
> >>> +
> >>> +	  /* 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 (dbg_cnt (ipa_mod_ref_pta))
> >>> +	    flags |= modref_flags;
> >>> +	}
> >>> +    }
> >>> +  return flags;
> >>> +}
> >>> +
> >>> /* Detects return flags for the call STMT.  */
> >>> 
> >>> int
> >>> diff --git a/gcc/gimple.h b/gcc/gimple.h
> >>> index 303623b3ced..23a124ec769 100644
> >>> --- a/gcc/gimple.h
> >>> +++ b/gcc/gimple.h
> >>> @@ -1589,6 +1589,7 @@ gimple_seq gimple_seq_copy (gimple_seq);
> >>> bool gimple_call_same_target_p (const gimple *, const gimple *);
> >>> int gimple_call_flags (const gimple *);
> >>> int gimple_call_arg_flags (const gcall *, unsigned);
> >>> +int gimple_call_retslot_flags (const gcall *);
> >>> int gimple_call_return_flags (const gcall *);
> >>> bool gimple_call_nonnull_result_p (gcall *);
> >>> tree gimple_call_nonnull_arg (gcall *);
> >>> diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> >>> index 0bbec8df0a2..3539cb43d11 100644
> >>> --- a/gcc/ipa-modref.c
> >>> +++ b/gcc/ipa-modref.c
> >>> @@ -86,6 +86,7 @@ along with GCC; see the file COPYING3.  If not see
> >>> #include "stringpool.h"
> >>> #include "tree-ssanames.h"
> >>> #include "attribs.h"
> >>> +#include "tree-cfg.h"
> >>> 
> >>> 
> >>> namespace {
> >>> @@ -133,7 +134,7 @@ static fnspec_summaries_t *fnspec_summaries = NULL;
> >>> struct escape_entry
> >>> {
> >>>  /* Parameter that escapes at a given call.  */
> >>> -  unsigned int parm_index;
> >>> +  int parm_index;
> >>>  /* Argument it escapes to.  */
> >>>  unsigned int arg;
> >>>  /* Minimal flags known about the argument.  */
> >>> @@ -269,7 +270,7 @@ static GTY(()) fast_function_summary <modref_summary_lto *, va_gc>
> >>> /* Summary for a single function which this pass produces.  */
> >>> 
> >>> modref_summary::modref_summary ()
> >>> -  : loads (NULL), stores (NULL), writes_errno (NULL)
> >>> +  : loads (NULL), stores (NULL), retslot_flags (0), writes_errno (false)
> >>> {
> >>> }
> >>> 
> >>> @@ -322,6 +323,8 @@ modref_summary::useful_p (int ecf_flags, bool check_flags)
> >>>  if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
> >>>    return true;
> >>>  arg_flags.release ();
> >>> +  if (check_flags && remove_useless_eaf_flags (retslot_flags, ecf_flags, false))
> >>> +    return true;
> >>>  if (ecf_flags & ECF_CONST)
> >>>    return false;
> >>>  if (loads && !loads->every_base)
> >>> @@ -363,6 +366,7 @@ struct GTY(()) modref_summary_lto
> >>>  modref_records_lto *loads;
> >>>  modref_records_lto *stores;
> >>>  auto_vec<eaf_flags_t> GTY((skip)) arg_flags;
> >>> +  eaf_flags_t retslot_flags;
> >>>  bool writes_errno;
> >>> 
> >>>  modref_summary_lto ();
> >>> @@ -374,7 +378,7 @@ struct GTY(()) modref_summary_lto
> >>> /* Summary for a single function which this pass produces.  */
> >>> 
> >>> modref_summary_lto::modref_summary_lto ()
> >>> -  : loads (NULL), stores (NULL), writes_errno (NULL)
> >>> +  : loads (NULL), stores (NULL), retslot_flags (0), writes_errno (false)
> >>> {
> >>> }
> >>> 
> >>> @@ -400,6 +404,8 @@ modref_summary_lto::useful_p (int ecf_flags, bool check_flags)
> >>>  if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
> >>>    return true;
> >>>  arg_flags.release ();
> >>> +  if (check_flags && remove_useless_eaf_flags (retslot_flags, ecf_flags, false))
> >>> +    return true;
> >>>  if (ecf_flags & ECF_CONST)
> >>>    return false;
> >>>  if (loads && !loads->every_base)
> >>> @@ -608,6 +614,11 @@ modref_summary::dump (FILE *out)
> >>> 	    dump_eaf_flags (out, arg_flags[i]);
> >>> 	  }
> >>>    }
> >>> +  if (retslot_flags)
> >>> +    {
> >>> +      fprintf (out, "  Retslot flags:");
> >>> +      dump_eaf_flags (out, retslot_flags);
> >>> +    }
> >>> }
> >>> 
> >>> /* Dump summary.  */
> >>> @@ -630,6 +641,11 @@ modref_summary_lto::dump (FILE *out)
> >>> 	    dump_eaf_flags (out, arg_flags[i]);
> >>> 	  }
> >>>    }
> >>> +  if (retslot_flags)
> >>> +    {
> >>> +      fprintf (out, "  Retslot flags:");
> >>> +      dump_eaf_flags (out, retslot_flags);
> >>> +    }
> >>> }
> >>> 
> >>> /* Get function summary for FUNC if it exists, return NULL otherwise.  */
> >>> @@ -1396,6 +1412,11 @@ namespace {
> >>> 
> >>> struct escape_point
> >>> {
> >>> +  /* Extra hidden args we keep track of.  */
> >>> +  enum hidden_args
> >>> +  {
> >>> +    retslot_arg = -1
> >>> +  };
> >>>  /* Value escapes to this call.  */
> >>>  gcall *call;
> >>>  /* Argument it escapes to.  */
> >>> @@ -1705,7 +1726,11 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
> >>> 	 Returning name counts as an use by tree-ssa-structalias.c  */
> >>>      if (greturn *ret = dyn_cast <greturn *> (use_stmt))
> >>> 	{
> >>> -	  if (gimple_return_retval (ret) == name)
> >>> +	  /* Returning through return slot is seen as memory write earlier.  */
> >>> +	  if (DECL_RESULT (current_function_decl)
> >>> +	      && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
> >>> +	    ;
> >>> +	  else if (gimple_return_retval (ret) == name)
> >>> 	    lattice[index].merge (~(EAF_UNUSED | EAF_NOT_RETURNED));
> >>> 	  else if (memory_access_to (gimple_return_retval (ret), name))
> >>> 	    {
> >>> @@ -1748,7 +1773,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
> >>> 		     may make LHS to escape.  See PR 98499.  */
> >>> 		  if (gimple_call_return_slot_opt_p (call)
> >>> 		      && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (call))))
> >>> -		    lattice[index].merge (EAF_NOREAD | EAF_DIRECT);
> >>> +		    lattice[index].merge (gimple_call_retslot_flags (call));
> >>> 		}
> >>> 
> >>> 	      /* We do not track accesses to the static chain (we could)
> >>> @@ -1777,7 +1802,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
> >>> 			  lattice[index].merge (call_flags);
> >>> 			else
> >>> 			  lattice[index].add_escape_point (call, i,
> >>> -			     				   call_flags, true);
> >>> +							   call_flags, true);
> >>> 		      }
> >>> 		    if (!ignore_retval)
> >>> 		      merge_call_lhs_flags (call, i, index, false,
> >>> @@ -1912,6 +1937,29 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth,
> >>>  lattice[index].known = true;
> >>> }
> >>> 
> >>> +/* Record escape points of PARM_INDEX according to LATTICE.  */
> >>> +
> >>> +static void
> >>> +record_escape_points (modref_lattice &lattice, int parm_index, int flags)
> >>> +{
> >>> +  if (lattice.escape_points.length ())
> >>> +    {
> >>> +      escape_point *ep;
> >>> +      unsigned int ip;
> >>> +      cgraph_node *node = cgraph_node::get (current_function_decl);
> >>> +
> >>> +      FOR_EACH_VEC_ELT (lattice.escape_points, ip, ep)
> >>> +	if ((ep->min_flags & flags) != flags)
> >>> +	  {
> >>> +	    cgraph_edge *e = node->get_edge (ep->call);
> >>> +	    struct escape_entry ee = {parm_index, ep->arg,
> >>> +				      ep->min_flags, ep->direct};
> >>> +
> >>> +	    escape_summaries->get_create (e)->esc.safe_push (ee);
> >>> +	  }
> >>> +    }
> >>> +}
> >>> +
> >>> /* Determine EAF flags for function parameters.  */
> >>> 
> >>> static void
> >>> @@ -1921,16 +1969,22 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto,
> >>>  unsigned int parm_index = 0;
> >>>  unsigned int count = 0;
> >>>  int ecf_flags = flags_from_decl_or_type (current_function_decl);
> >>> +  tree retslot = 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)))
> >>> +    retslot = ssa_default_def (cfun, DECL_RESULT (current_function_decl));
> >>> +
> >>>  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
> >>>       parm = TREE_CHAIN (parm))
> >>>    count++;
> >>> 
> >>> -  if (!count)
> >>> +  if (!count && !retslot)
> >>>    return;
> >>> 
> >>>  auto_vec<modref_lattice> lattice;
> >>> @@ -1984,24 +2038,24 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto,
> >>> 		summary_lto->arg_flags.safe_grow_cleared (count, true);
> >>> 	      summary_lto->arg_flags[parm_index] = flags;
> >>> 	    }
> >>> -	  if (lattice[SSA_NAME_VERSION (name)].escape_points.length ())
> >>> -	    {
> >>> -	      escape_point *ep;
> >>> -	      unsigned int ip;
> >>> -	      cgraph_node *node = cgraph_node::get (current_function_decl);
> >>> -
> >>> -	      gcc_checking_assert (ipa);
> >>> -	      FOR_EACH_VEC_ELT
> >>> -		  (lattice[SSA_NAME_VERSION (name)].escape_points, ip, ep)
> >>> -		if ((ep->min_flags & flags) != flags)
> >>> -		  {
> >>> -		    cgraph_edge *e = node->get_edge (ep->call);
> >>> -		    struct escape_entry ee = {parm_index, ep->arg,
> >>> -		     			      ep->min_flags, ep->direct};
> >>> +	  record_escape_points (lattice[SSA_NAME_VERSION (name)],
> >>> +				parm_index, flags);
> >>> +	}
> >>> +    }
> >>> +  if (retslot)
> >>> +    {
> >>> +      analyze_ssa_name_flags (retslot, lattice, 0, ipa);
> >>> +      int flags = lattice[SSA_NAME_VERSION (retslot)].flags;
> >>> 
> >>> -		    escape_summaries->get_create (e)->esc.safe_push (ee);
> >>> -		  }
> >>> -	    }
> >>> +      flags = remove_useless_eaf_flags (flags, ecf_flags, false);
> >>> +      if (flags)
> >>> +	{
> >>> +	  if (summary)
> >>> +	    summary->retslot_flags = flags;
> >>> +	  if (summary_lto)
> >>> +	    summary_lto->retslot_flags = flags;
> >>> +	  record_escape_points (lattice[SSA_NAME_VERSION (retslot)],
> >>> +				escape_point::retslot_arg, flags);
> >>> 	}
> >>>    }
> >>>  if (ipa)
> >>> @@ -2287,6 +2341,7 @@ modref_summaries::duplicate (cgraph_node *, cgraph_node *dst,
> >>>  dst_data->writes_errno = src_data->writes_errno;
> >>>  if (src_data->arg_flags.length ())
> >>>    dst_data->arg_flags = src_data->arg_flags.copy ();
> >>> +  dst_data->retslot_flags = src_data->retslot_flags;
> >>> }
> >>> 
> >>> /* Called when new clone is inserted to callgraph late.  */
> >>> @@ -2312,6 +2367,7 @@ modref_summaries_lto::duplicate (cgraph_node *, cgraph_node *,
> >>>  dst_data->writes_errno = src_data->writes_errno;
> >>>  if (src_data->arg_flags.length ())
> >>>    dst_data->arg_flags = src_data->arg_flags.copy ();
> >>> +  dst_data->retslot_flags = src_data->retslot_flags;
> >>> }
> >>> 
> >>> namespace
> >>> @@ -2551,7 +2607,7 @@ modref_write_escape_summary (struct bitpack_d *bp, escape_summary *esum)
> >>>  escape_entry *ee;
> >>>  FOR_EACH_VEC_ELT (esum->esc, i, ee)
> >>>    {
> >>> -      bp_pack_var_len_unsigned (bp, ee->parm_index);
> >>> +      bp_pack_var_len_int (bp, ee->parm_index);
> >>>      bp_pack_var_len_unsigned (bp, ee->arg);
> >>>      bp_pack_var_len_unsigned (bp, ee->min_flags);
> >>>      bp_pack_value (bp, ee->direct, 1);
> >>> @@ -2571,7 +2627,7 @@ modref_read_escape_summary (struct bitpack_d *bp, cgraph_edge *e)
> >>>  for (unsigned int i = 0; i < n; i++)
> >>>    {
> >>>      escape_entry ee;
> >>> -      ee.parm_index = bp_unpack_var_len_unsigned (bp);
> >>> +      ee.parm_index = bp_unpack_var_len_int (bp);
> >>>      ee.arg = bp_unpack_var_len_unsigned (bp);
> >>>      ee.min_flags = bp_unpack_var_len_unsigned (bp);
> >>>      ee.direct = bp_unpack_value (bp, 1);
> >>> @@ -2628,6 +2684,7 @@ modref_write ()
> >>> 	  streamer_write_uhwi (ob, r->arg_flags.length ());
> >>> 	  for (unsigned int i = 0; i < r->arg_flags.length (); i++)
> >>> 	    streamer_write_uhwi (ob, r->arg_flags[i]);
> >>> +	  streamer_write_uhwi (ob, r->retslot_flags);
> >>> 
> >>> 	  write_modref_records (r->loads, ob);
> >>> 	  write_modref_records (r->stores, ob);
> >>> @@ -2724,6 +2781,11 @@ read_section (struct lto_file_decl_data *file_data, const char *data,
> >>> 	  if (modref_sum_lto)
> >>> 	    modref_sum_lto->arg_flags.quick_push (flags);
> >>> 	}
> >>> +      eaf_flags_t flags = streamer_read_uhwi (&ib);
> >>> +      if (modref_sum)
> >>> +	modref_sum->retslot_flags = flags;
> >>> +      if (modref_sum_lto)
> >>> +	modref_sum_lto->retslot_flags = flags;
> >>>      read_modref_records (&ib, data_in,
> >>> 			   modref_sum ? &modref_sum->loads : NULL,
> >>> 			   modref_sum_lto ? &modref_sum_lto->loads : NULL);
> >>> @@ -3098,7 +3160,7 @@ struct escape_map
> >>>  bool direct;
> >>> };
> >>> 
> >>> -/* Update escape map fo E.  */
> >>> +/* Update escape map for E.  */
> >>> 
> >>> static void
> >>> update_escape_summary_1 (cgraph_edge *e,
> >>> @@ -3117,7 +3179,10 @@ update_escape_summary_1 (cgraph_edge *e,
> >>>    {
> >>>      unsigned int j;
> >>>      struct escape_map *em;
> >>> -      if (ee->parm_index >= map.length ())
> >>> +      /* TODO: We do not have jump functions for return slots, so we
> >>> +	 never propagate them to outer function.  */
> >>> +      if (ee->parm_index >= (int)map.length ()
> >>> +	  || ee->parm_index < 0)
> >>> 	continue;
> >>>      FOR_EACH_VEC_ELT (map[ee->parm_index], j, em)
> >>> 	{
> >>> @@ -3125,7 +3190,7 @@ update_escape_summary_1 (cgraph_edge *e,
> >>> 	  if (ee->direct && !em->direct)
> >>> 	    min_flags = deref_flags (min_flags, ignore_stores);
> >>> 	  struct escape_entry entry = {em->parm_index, ee->arg,
> >>> -	    			       ee->min_flags,
> >>> +				       ee->min_flags,
> >>> 				       ee->direct & em->direct};
> >>> 	  sum->esc.safe_push (entry);
> >>> 	}
> >>> @@ -3245,7 +3310,11 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
> >>>    FOR_EACH_VEC_ELT (sum->esc, i, ee)
> >>>      {
> >>> 	bool needed = false;
> >>> -	if (to_info && to_info->arg_flags.length () > ee->parm_index)
> >>> +	/* TODO: We do not have jump functions for return slots, so we
> >>> +	   never propagate them to outer function.  */
> >>> +	if (ee->parm_index < 0)
> >>> +	  continue;
> >>> +	if (to_info && (int)to_info->arg_flags.length () > ee->parm_index)
> >>> 	  {
> >>> 	    int flags = callee_info
> >>> 			&& callee_info->arg_flags.length () > ee->arg
> >>> @@ -3259,7 +3328,7 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
> >>> 	    if (to_info->arg_flags[ee->parm_index])
> >>> 	      needed = true;
> >>> 	  }
> >>> -	if (to_info_lto && to_info_lto->arg_flags.length () > ee->parm_index)
> >>> +	if (to_info_lto && (int)to_info_lto->arg_flags.length () > ee->parm_index)
> >>> 	  {
> >>> 	    int flags = callee_info_lto
> >>> 			&& callee_info_lto->arg_flags.length () > ee->arg
> >>> @@ -3798,29 +3867,31 @@ modref_merge_call_site_flags (escape_summary *sum,
> >>>      if (flags_lto & EAF_NOESCAPE)
> >>> 	flags_lto |= EAF_NODIRECTESCAPE;
> >>>      if (!(flags & EAF_UNUSED)
> >>> -	  && cur_summary && ee->parm_index < cur_summary->arg_flags.length ())
> >>> +	  && cur_summary && ee->parm_index < (int)cur_summary->arg_flags.length ())
> >>> 	{
> >>> -	  int f = cur_summary->arg_flags[ee->parm_index];
> >>> +	  eaf_flags_t &f = ee->parm_index == escape_point::retslot_arg
> >>> +			   ? cur_summary->retslot_flags
> >>> +			   : cur_summary->arg_flags[ee->parm_index];
> >>> 	  if ((f & flags) != f)
> >>> 	    {
> >>> 	      f = remove_useless_eaf_flags
> >>> 			 (f & flags, ecf_flags,
> >>> 			  VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
> >>> -	      cur_summary->arg_flags[ee->parm_index] = f;
> >>> 	      changed = true;
> >>> 	    }
> >>> 	}
> >>>      if (!(flags_lto & EAF_UNUSED)
> >>> 	  && cur_summary_lto
> >>> -	  && ee->parm_index < cur_summary_lto->arg_flags.length ())
> >>> +	  && ee->parm_index < (int)cur_summary_lto->arg_flags.length ())
> >>> 	{
> >>> -	  int f = cur_summary_lto->arg_flags[ee->parm_index];
> >>> +	  eaf_flags_t &f = ee->parm_index == escape_point::retslot_arg
> >>> +			   ? cur_summary_lto->retslot_flags
> >>> +			   : cur_summary_lto->arg_flags[ee->parm_index];
> >>> 	  if ((f & flags_lto) != f)
> >>> 	    {
> >>> 	      f = remove_useless_eaf_flags
> >>> 			 (f & flags_lto, ecf_flags,
> >>> 			  VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
> >>> -	      cur_summary_lto->arg_flags[ee->parm_index] = f;
> >>> 	      changed = true;
> >>> 	    }
> >>> 	}
> >>> diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
> >>> index 5afa3aa439f..a4db27471eb 100644
> >>> --- a/gcc/ipa-modref.h
> >>> +++ b/gcc/ipa-modref.h
> >>> @@ -31,6 +31,7 @@ struct GTY(()) modref_summary
> >>>  modref_records *loads;
> >>>  modref_records *stores;
> >>>  auto_vec<eaf_flags_t> GTY((skip)) arg_flags;
> >>> +  eaf_flags_t retslot_flags;
> >>>  bool writes_errno;
> >>> 
> >>>  modref_summary ();
> >>> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
> >>> index 35971a54e02..99072df0768 100644
> >>> --- a/gcc/tree-ssa-structalias.c
> >>> +++ b/gcc/tree-ssa-structalias.c
> >>> @@ -4254,17 +4254,28 @@ handle_rhs_call (gcall *stmt, vec<ce_s> *results,
> >>>      && gimple_call_lhs (stmt) != NULL_TREE
> >>>      && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (stmt))))
> >>>    {
> >>> -      auto_vec<ce_s> tmpc;
> >>> -      struct constraint_expr *c;
> >>> -      unsigned i;
> >>> +      int flags = gimple_call_retslot_flags (stmt);
> >>> +      if ((flags & (EAF_NOESCAPE | EAF_NOT_RETURNED))
> >>> +	  != (EAF_NOESCAPE | EAF_NOT_RETURNED))
> >>> +	{
> >>> +	  auto_vec<ce_s> tmpc;
> >>> 
> >>> -      get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
> >>> +	  get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
> >>> 
> >>> -      make_constraints_to (callescape->id, tmpc);
> >>> -      if (writes_global_memory)
> >>> -	make_constraints_to (escaped_id, tmpc);
> >>> -      FOR_EACH_VEC_ELT (tmpc, i, c)
> >>> -	results->safe_push (*c);
> >>> +	  if (!(flags & (EAF_NOESCAPE | EAF_NODIRECTESCAPE)))
> >>> +	    {
> >>> +	      make_constraints_to (callescape->id, tmpc);
> >>> +	      if (writes_global_memory)
> >>> +		make_constraints_to (escaped_id, tmpc);
> >>> +	    }
> >>> +	  if (!(flags & EAF_NOT_RETURNED))
> >>> +	    {
> >>> +	      struct constraint_expr *c;
> >>> +	      unsigned i;
> >>> +	      FOR_EACH_VEC_ELT (tmpc, i, c)
> >>> +		results->safe_push (*c);
> >>> +	    }
> >>> +	}
> >>>    }
> >>> }
> >>> 
> >>> </cut>
> >> 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-03 23:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-30  2:32 [TCWG CI] Regression caused by gcc: handle retslot in modref ci_notify
2021-11-03 11:33 ` Maxim Kuvyrkov
2021-11-03 13:00   ` Jan Hubicka
     [not found]   ` <20211103151227.GA60321@kam.mff.cuni.cz>
2021-11-03 16:37     ` Maxim Kuvyrkov
2021-11-03 23:46       ` Jan Hubicka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).