Fix tramp3d PGO misoptimization

this patch fixes tramp3d ICE with PGO.  It has turned out to be by a misupdate
in ignore_edge I introduced in previous patch that made us to not compute
SCCs correctly with -fno-lto.

While looking for problem I proofread the sources and also fortified the
srouces for situation where we insert a summary for no good reason and noticed
a problem that early ipa-modref disabled itself in some cases.
I also noticed that param_index is treamed as uhwi while it is signed (that
wastes file space).

Bootstrapping/regtesting x86_64-linux, will commit it tomorrow if that passes.

gcc/ChangeLog:

2020-10-13  Jan Hubicka  <hubicka@ucw.cz>

	PR ipa/97389
	* ipa-modref.c (dump_lto_records): Fix formating of dump file.
	(modref_summary::dump): Do not check loads to be non-null.
	(modref_summary_lto::dump): Do not check loads to be non-null.
	(merge_call_side_effects): Improve debug output.
	(analyze_call): Crash when cur_summary->loads is NULL.
	(analyze_function): Update.
	(modref_summaries::insert): Insert only into summaries, not
	optimization_summaries.
	(modref_summaries::duplicate): Likewise; crash when load or sotres
	are NULL.
	(modref_summaries_lto::duplicate): Crash when loads or stores are NULL.
	(write_modref_records): param_index is signed.
	(read_modref_records): param_index is signed.
	(modref_write): Crash when loads or stores are NULL.
	(read_section): Compensate previous change.
	(pass_modref::execute): Do not check optimization_summaries t be
	non-NULL.
	(ignore_edge): Fix.
	(compute_parm_map): Fix formating.
	(modref_propagate_in_scc): Do not expect loads/stores to be NULL.

1 files changed, 84 insertions, 104 deletions

diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 1d4eaf8..4f86b9c 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -298,7 +298,7 @@ dump_lto_records (modref_records_lto *tt, FILE *out)
 		   r->ref ? get_alias_set (r->ref) : 0);
 	  if (r->every_access)
 	    {
-	      fprintf (out, "      Every access\n");
+	      fprintf (out, "          Every access\n");
 	      continue;
 	    }
 	  size_t k;
@@ -314,16 +314,10 @@ dump_lto_records (modref_records_lto *tt, FILE *out)
 void
 modref_summary::dump (FILE *out)
 {
-  if (loads)
-    {
-      fprintf (out, "  loads:\n");
-      dump_records (loads, out);
-    }
-  if (stores)
-    {
-      fprintf (out, "  stores:\n");
-      dump_records (stores, out);
-    }
+  fprintf (out, "  loads:\n");
+  dump_records (loads, out);
+  fprintf (out, "  stores:\n");
+  dump_records (stores, out);
 }
 
 /* Dump summary.  */
@@ -331,16 +325,10 @@ modref_summary::dump (FILE *out)
 void
 modref_summary_lto::dump (FILE *out)
 {
-  if (loads)
-    {
-      fprintf (out, "  loads:\n");
-      dump_lto_records (loads, out);
-    }
-  if (stores)
-    {
-      fprintf (out, "  stores:\n");
-      dump_lto_records (stores, out);
-    }
+  fprintf (out, "  loads:\n");
+  dump_lto_records (loads, out);
+  fprintf (out, "  stores:\n");
+  dump_lto_records (stores, out);
 }
 
 /* Get function summary for FUNC if it exists, return NULL otherwise.  */
@@ -530,16 +518,19 @@ ignore_stores_p (tree caller, int flags)
 bool
 merge_call_side_effects (modref_summary *cur_summary,
 			 gimple *stmt, modref_summary *callee_summary,
-			 bool ignore_stores)
+			 bool ignore_stores, cgraph_node *callee_node)
 {
   auto_vec <modref_parm_map, 32> parm_map;
   bool changed = false;
 
+  if (dump_file)
+    fprintf (dump_file, " - Merging side effects of %s with parm map:",
+	     callee_node->dump_name ());
+
   parm_map.safe_grow_cleared (gimple_call_num_args (stmt));
   for (unsigned i = 0; i < gimple_call_num_args (stmt); i++)
     {
       tree op = gimple_call_arg (stmt, i);
-      STRIP_NOPS (op);
       if (TREE_CODE (op) == SSA_NAME
 	  && SSA_NAME_IS_DEFAULT_DEF (op)
 	  && TREE_CODE (SSA_NAME_VAR (op)) == PARM_DECL)
@@ -563,17 +554,17 @@ merge_call_side_effects (modref_summary *cur_summary,
 	parm_map[i].parm_index = -2;
       else
 	parm_map[i].parm_index = -1;
+      if (dump_file)
+	fprintf (dump_file, " %i", parm_map[i].parm_index);
     }
+  if (dump_file)
+    fprintf (dump_file, "\n");
 
   /* Merge with callee's summary.  */
-  if (cur_summary->loads)
-    changed |= cur_summary->loads->merge (callee_summary->loads, &parm_map);
+  changed |= cur_summary->loads->merge (callee_summary->loads, &parm_map);
   if (!ignore_stores)
-    {
-      if (cur_summary->stores)
-	changed |= cur_summary->stores->merge (callee_summary->stores,
-					       &parm_map);
-    }
+    changed |= cur_summary->stores->merge (callee_summary->stores,
+					   &parm_map);
   return changed;
 }
 
@@ -672,8 +663,7 @@ analyze_call (modref_summary *cur_summary,
     {
       if (ignore_stores)
 	{
-	  if (cur_summary->loads)
-	    cur_summary->loads->collapse ();
+	  cur_summary->loads->collapse ();
 	  return true;
 	}
       if (dump_file)
@@ -681,7 +671,8 @@ analyze_call (modref_summary *cur_summary,
       return false;
     }
 
-  merge_call_side_effects (cur_summary, stmt, callee_summary, ignore_stores);
+  merge_call_side_effects (cur_summary, stmt, callee_summary, ignore_stores,
+			   callee_node);
 
   return true;
 }
@@ -853,7 +844,10 @@ analyze_function (function *f, bool ipa)
       else /* Remove existing summary if we are re-running the pass.  */
 	{
 	  if (dump_file
-	      && optimization_summaries->get (cgraph_node::get (f->decl)))
+	      && (summary
+		  = optimization_summaries->get (cgraph_node::get (f->decl)))
+		 != NULL
+	      && summary->loads)
 	    {
 	      fprintf (dump_file, "Past summary:\n");
 	      optimization_summaries->get
@@ -950,7 +944,8 @@ analyze_function (function *f, bool ipa)
 			  (summary, recursive_calls[i], summary,
 			   ignore_stores_p (current_function_decl,
 					    gimple_call_flags
-						 (recursive_calls[i])));
+						 (recursive_calls[i])),
+			   fnode);
 	      if (!summary->useful_p (ecf_flags))
 		{
 		  remove_summary (lto, nolto, ipa);
@@ -1005,18 +1000,19 @@ modref_generate (void)
 void
 modref_summaries::insert (struct cgraph_node *node, modref_summary *)
 {
-  if (!DECL_STRUCT_FUNCTION (node->decl))
+  /* Local passes ought to be executed by the pass manager.  */
+  if (this == optimization_summaries)
     {
       optimization_summaries->remove (node);
+      return;
+    }
+  if (!DECL_STRUCT_FUNCTION (node->decl))
+    {
       summaries->remove (node);
+      return;
     }
   push_cfun (DECL_STRUCT_FUNCTION (node->decl));
-  /* This is not very pretty: We do not know if we insert into optimization
-     summary or summary.  Do both but check for duplicated effort.  */
-  if (optimization_summaries && !optimization_summaries->get (node)->loads)
-    analyze_function (DECL_STRUCT_FUNCTION (node->decl), false);
-  if (summaries && !summaries->get (node)->loads)
-    analyze_function (DECL_STRUCT_FUNCTION (node->decl), true);
+  analyze_function (DECL_STRUCT_FUNCTION (node->decl), true);
   pop_cfun ();
 }
 
@@ -1042,26 +1038,27 @@ modref_summaries_lto::insert (struct cgraph_node *node, modref_summary_lto *)
 /* Called when new clone is inserted to callgraph late.  */
 
 void
-modref_summaries::duplicate (cgraph_node *, cgraph_node *,
+modref_summaries::duplicate (cgraph_node *, cgraph_node *dst,
 			     modref_summary *src_data,
 			     modref_summary *dst_data)
 {
-  if (src_data->stores)
+  /* Do not duplicte optimization summaries; we do not handle parameter
+     transforms on them.  */
+  if (this == optimization_summaries)
     {
-      dst_data->stores = modref_records::create_ggc
-			    (src_data->stores->max_bases,
-			     src_data->stores->max_refs,
-			     src_data->stores->max_accesses);
-      dst_data->stores->copy_from (src_data->stores);
-    }
-  if (src_data->loads)
-    {
-      dst_data->loads = modref_records::create_ggc
-			    (src_data->loads->max_bases,
-			     src_data->loads->max_refs,
-			     src_data->loads->max_accesses);
-      dst_data->loads->copy_from (src_data->loads);
+      optimization_summaries->remove (dst);
+      return;
     }
+  dst_data->stores = modref_records::create_ggc
+			(src_data->stores->max_bases,
+			 src_data->stores->max_refs,
+			 src_data->stores->max_accesses);
+  dst_data->stores->copy_from (src_data->stores);
+  dst_data->loads = modref_records::create_ggc
+			(src_data->loads->max_bases,
+			 src_data->loads->max_refs,
+			 src_data->loads->max_accesses);
+  dst_data->loads->copy_from (src_data->loads);
 }
 
 /* Called when new clone is inserted to callgraph late.  */
@@ -1071,22 +1068,16 @@ modref_summaries_lto::duplicate (cgraph_node *, cgraph_node *,
 				 modref_summary_lto *src_data,
 				 modref_summary_lto *dst_data)
 {
-  if (src_data->stores)
-    {
-      dst_data->stores = modref_records_lto::create_ggc
-			    (src_data->stores->max_bases,
-			     src_data->stores->max_refs,
-			     src_data->stores->max_accesses);
-      dst_data->stores->copy_from (src_data->stores);
-    }
-  if (src_data->loads)
-    {
-      dst_data->loads = modref_records_lto::create_ggc
-			    (src_data->loads->max_bases,
-			     src_data->loads->max_refs,
-			     src_data->loads->max_accesses);
-      dst_data->loads->copy_from (src_data->loads);
-    }
+  dst_data->stores = modref_records_lto::create_ggc
+			(src_data->stores->max_bases,
+			 src_data->stores->max_refs,
+			 src_data->stores->max_accesses);
+  dst_data->stores->copy_from (src_data->stores);
+  dst_data->loads = modref_records_lto::create_ggc
+			(src_data->loads->max_bases,
+			 src_data->loads->max_refs,
+			 src_data->loads->max_accesses);
+  dst_data->loads->copy_from (src_data->loads);
 }
 
 namespace
@@ -1154,7 +1145,7 @@ write_modref_records (modref_records_lto *tt, struct output_block *ob)
 	  modref_access_node *access_node;
 	  FOR_EACH_VEC_SAFE_ELT (ref_node->accesses, k, access_node)
 	    {
-	      streamer_write_uhwi (ob, access_node->parm_index);
+	      streamer_write_hwi (ob, access_node->parm_index);
 	      if (access_node->parm_index != -1)
 		{
 		  streamer_write_uhwi (ob, access_node->parm_offset_known);
@@ -1278,7 +1269,7 @@ read_modref_records (lto_input_block *ib, struct data_in *data_in,
 
 	  for (size_t k = 0; k < naccesses; k++)
 	    {
-	      int parm_index = streamer_read_uhwi (ib);
+	      int parm_index = streamer_read_hwi (ib);
 	      bool parm_offset_known = false;
 	      poly_int64 parm_offset = 0;
 	      poly_int64 offset = 0;
@@ -1358,12 +1349,8 @@ modref_write ()
 
 	  streamer_write_uhwi (ob, lto_symtab_encoder_encode (encoder, cnode));
 
-	  streamer_write_uhwi (ob, r->loads ? 1 : 0);
-	  streamer_write_uhwi (ob, r->stores ? 1 : 0);
-	  if (r->loads)
-	    write_modref_records (r->loads, ob);
-	  if (r->stores)
-	    write_modref_records (r->stores, ob);
+	  write_modref_records (r->loads, ob);
+	  write_modref_records (r->stores, ob);
 	}
     }
   streamer_write_char_stream (ob->main_stream, 0);
@@ -1406,8 +1393,6 @@ read_section (struct lto_file_decl_data *file_data, const char *data,
       modref_summary_lto *modref_sum_lto = summaries_lto
 					   ? summaries_lto->get_create (node)
 					   : NULL;
-      int have_loads = streamer_read_uhwi (&ib);
-      int have_stores = streamer_read_uhwi (&ib);
 
       if (optimization_summaries)
 	modref_sum = optimization_summaries->get_create (node);
@@ -1416,14 +1401,12 @@ read_section (struct lto_file_decl_data *file_data, const char *data,
 				  && !modref_sum->stores));
       gcc_assert (!modref_sum_lto || (!modref_sum_lto->loads
 				      && !modref_sum_lto->stores));
-      if (have_loads)
-	 read_modref_records (&ib, data_in,
-			      modref_sum ? &modref_sum->loads : NULL,
-			      modref_sum_lto ? &modref_sum_lto->loads : NULL);
-      if (have_stores)
-	 read_modref_records (&ib, data_in,
-			      modref_sum ? &modref_sum->stores : NULL,
-			      modref_sum_lto ? &modref_sum_lto->stores : NULL);
+      read_modref_records (&ib, data_in,
+			   modref_sum ? &modref_sum->loads : NULL,
+			   modref_sum_lto ? &modref_sum_lto->loads : NULL);
+      read_modref_records (&ib, data_in,
+			   modref_sum ? &modref_sum->stores : NULL,
+			   modref_sum_lto ? &modref_sum_lto->stores : NULL);
       if (dump_file)
 	{
 	  fprintf (dump_file, "Read modref for %s\n",
@@ -1598,9 +1581,6 @@ public:
 
 unsigned int pass_modref::execute (function *f)
 {
-  /* If new function is being added during IPA, we can skip analysis.  */
-  if (!optimization_summaries)
-    return 0;
   analyze_function (f, false);
   return 0;
 }
@@ -1628,7 +1608,7 @@ ignore_edge (struct cgraph_edge *e)
 			  (&avail, e->caller);
 
   return (avail <= AVAIL_INTERPOSABLE
-	  || ((!summaries || !summaries->get (callee))
+	  || ((!optimization_summaries || !optimization_summaries->get (callee))
 	      && (!summaries_lto || !summaries_lto->get (callee)))
 	  || flags_from_decl_or_type (e->callee->decl)
 	     & (ECF_CONST | ECF_NOVOPS));
@@ -1684,7 +1664,7 @@ compute_parm_map (cgraph_edge *callee_edge, vec<modref_parm_map> *parm_map)
 	  if (jf && jf->type == IPA_JF_PASS_THROUGH)
 	    {
 	      (*parm_map)[i].parm_index
-		 = ipa_get_jf_pass_through_formal_id (jf);
+		= ipa_get_jf_pass_through_formal_id (jf);
 	      (*parm_map)[i].parm_offset_known
 		= ipa_get_jf_pass_through_operation (jf) == NOP_EXPR;
 	      (*parm_map)[i].parm_offset = 0;
@@ -1921,6 +1901,8 @@ modref_propagate_in_scc (cgraph_node *component_node)
 			optimization_summaries->remove (node);
 		      if (summaries_lto)
 			summaries_lto->remove (node);
+		      cur_summary = NULL;
+		      cur_summary_lto = NULL;
 		      changed = true;
 		      break;
 		    }
@@ -2009,19 +1991,17 @@ modref_propagate_in_scc (cgraph_node *component_node)
 	      /* Merge in callee's information.  */
 	      if (callee_summary)
 		{
-		  if (callee_summary->loads)
-		    changed |= cur_summary->loads->merge
-				    (callee_summary->loads, &parm_map);
-		  if (callee_summary->stores)
+		  changed |= cur_summary->loads->merge
+				  (callee_summary->loads, &parm_map);
+		  if (!ignore_stores)
 		    changed |= cur_summary->stores->merge
 				    (callee_summary->stores, &parm_map);
 		}
 	      if (callee_summary_lto)
 		{
-		  if (callee_summary_lto->loads)
-		    changed |= cur_summary_lto->loads->merge
-				    (callee_summary_lto->loads, &parm_map);
-		  if (callee_summary_lto->stores)
+		  changed |= cur_summary_lto->loads->merge
+				  (callee_summary_lto->loads, &parm_map);
+		  if (!ignore_stores)
 		    changed |= cur_summary_lto->stores->merge
 				    (callee_summary_lto->stores, &parm_map);
 		}