public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Fix ipa-pure-const and ipa-reference for LTO
@ 2009-10-22 10:42 Jan Hubicka
  0 siblings, 0 replies; only message in thread
From: Jan Hubicka @ 2009-10-22 10:42 UTC (permalink / raw)
  To: gcc-patches

Hi,
PR 40556 shows ICE caused by interesting misconception of IPA passes.  The
generate summary are written in a way they handle all indirect calls and calls
to other units, while propgate stages propagate only across direct calls.  With
LTO analysis happens at compile time, when calls to other LTO units seems like
external calls.

This patch reorganizes the passes to handle externall calls within propagation
module and make analysis always optimistics (if we LTO or not).  I also noticed
that ipa-reference is not streaming calls_read_all/calls_write_all data at all.
I am bit confused this did no lead to heavy misoptimizations.

Bootstrapped/regtested x86_64-linux, will commit it after re-testing on current
tree after comitting the summary fixing patch.

Honza

	PR tree-optimize/40556
	* ipa-reference.c (has_proper_scope_for_analysis): Add fixme about global vars.
	(check_call): Handle only indirect calls.
	(propagate_bits): Update comment.
	(write_node_summary_p): Turn bogus check to assert.
	(ipa_reference_write_summary): Stream calls_read_all properly.
	(ipa_reference_read_summary): Stream in calls_read_all properly.
	(read_write_all_from_decl): New function.
	(propagate): Handle OVERWRITABLE nodes and external calls here.
	* ipa-pre-const.c (check_call): In IPA mode handle indirect calls
	only.
	(analyze_function): Do not check visibility here.
	(add_new_function): We summary OVERWRITABLE too.
	(generate_summary): Stream OVERWRITABLE nodes too.
	(propagate): Handle external calls and OVERWRITABLE nodes here.
	(local_pure_const): Check visibility here.

	* testsuite/gcc.c-torture/compile/pr40556.c: New testcase.
Index: ipa-reference.c
===================================================================
*** ipa-reference.c	(revision 153449)
--- ipa-reference.c	(working copy)
*************** has_proper_scope_for_analysis (tree t)
*** 318,323 ****
--- 318,325 ----
    if (!TREE_STATIC (t) && !DECL_EXTERNAL (t))
      return false;
  
+   /* FIXME: for LTO we should include PUBLIC vars too.  This is bit difficult
+      as summarie would need unsharing.  */
    if (DECL_EXTERNAL (t) || TREE_PUBLIC (t))
      return false;
  
*************** check_call (ipa_reference_local_vars_inf
*** 413,443 ****
  {
    int flags = gimple_call_flags (stmt);
    tree callee_t = gimple_call_fndecl (stmt);
-   enum availability avail = AVAIL_NOT_AVAILABLE;
  
!   if (callee_t)
      {
!       struct cgraph_node* callee = cgraph_node(callee_t);
!       avail = cgraph_function_body_availability (callee);
!     }
! 
!   if (avail <= AVAIL_OVERWRITABLE)
!     if (local) 
!       {
! 	if (flags & ECF_CONST) 
! 	  ;
! 	else if (flags & ECF_PURE)
  	  local->calls_read_all = true;
! 	else 
! 	  {
! 	    local->calls_read_all = true;
! 	    local->calls_write_all = true;
! 	  }
!       }
!    /* TODO: To be able to produce sane results, we should also handle
!       common builtins, in particular throw.
!       Indirect calls hsould be only counted and as inliner is replacing them
!       by direct calls, we can conclude if any indirect calls are left in body */
  }
  
  /* TP is the part of the tree currently under the microscope.
--- 415,435 ----
  {
    int flags = gimple_call_flags (stmt);
    tree callee_t = gimple_call_fndecl (stmt);
  
!   /* Process indirect calls.  All direct calles are handled at propagation
!      time.  */
!   if (!callee_t)
      {
!       if (flags & ECF_CONST) 
! 	;
!       else if (flags & ECF_PURE)
! 	local->calls_read_all = true;
!       else 
! 	{
  	  local->calls_read_all = true;
! 	  local->calls_write_all = true;
! 	}
!     }
  }
  
  /* TP is the part of the tree currently under the microscope.
*************** propagate_bits (ipa_reference_global_var
*** 527,533 ****
      {
        struct cgraph_node *y = e->callee;
  
!       /* Only look at the master nodes and skip external nodes.  */
        if (cgraph_function_body_availability (e->callee) > AVAIL_OVERWRITABLE)
  	{
  	  if (get_reference_vars_info (y))
--- 519,525 ----
      {
        struct cgraph_node *y = e->callee;
  
!       /* Only look into nodes we can propagate something.  */
        if (cgraph_function_body_availability (e->callee) > AVAIL_OVERWRITABLE)
  	{
  	  if (get_reference_vars_info (y))
*************** generate_summary (void)
*** 1012,1019 ****
  static bool
  write_node_summary_p (struct cgraph_node *node)
  {
    return (node->analyzed 
- 	  && node->global.inlined_to == NULL
  	  && cgraph_function_body_availability (node) >= AVAIL_OVERWRITABLE
  	  && get_reference_vars_info (node) != NULL);
  }
--- 1004,1011 ----
  static bool
  write_node_summary_p (struct cgraph_node *node)
  {
+   gcc_assert (node->global.inlined_to == NULL);
    return (node->analyzed 
  	  && cgraph_function_body_availability (node) >= AVAIL_OVERWRITABLE
  	  && get_reference_vars_info (node) != NULL);
  }
*************** ipa_reference_write_summary (cgraph_node
*** 1053,1070 ****
  	  lto_output_uleb128_stream (ob->main_stream, node_ref);
  
  	  /* Stream out the statics read.  */
! 	  lto_output_uleb128_stream (ob->main_stream,
! 				     bitmap_count_bits (l->statics_read));
! 	  EXECUTE_IF_SET_IN_BITMAP (l->statics_read, 0, index, bi)
! 	    lto_output_var_decl_index(ob->decl_state, ob->main_stream,
! 				      get_static_decl (index));
  
  	  /* Stream out the statics written.  */
! 	  lto_output_uleb128_stream (ob->main_stream,
! 				     bitmap_count_bits (l->statics_written));
! 	  EXECUTE_IF_SET_IN_BITMAP (l->statics_written, 0, index, bi)
! 	    lto_output_var_decl_index(ob->decl_state, ob->main_stream,
! 				      get_static_decl (index));
  	}
      }
    lto_destroy_simple_output_block (ob);
--- 1045,1072 ----
  	  lto_output_uleb128_stream (ob->main_stream, node_ref);
  
  	  /* Stream out the statics read.  */
! 	  if (l->calls_read_all)
! 	    lto_output_sleb128_stream (ob->main_stream, -1);
! 	  else
! 	    {
! 	      lto_output_sleb128_stream (ob->main_stream,
! 					 bitmap_count_bits (l->statics_read));
! 	      EXECUTE_IF_SET_IN_BITMAP (l->statics_read, 0, index, bi)
! 		lto_output_var_decl_index(ob->decl_state, ob->main_stream,
! 					  get_static_decl (index));
! 	    }
  
  	  /* Stream out the statics written.  */
! 	  if (l->calls_write_all)
! 	    lto_output_sleb128_stream (ob->main_stream, -1);
! 	  else
! 	    {
! 	      lto_output_sleb128_stream (ob->main_stream,
! 					 bitmap_count_bits (l->statics_written));
! 	      EXECUTE_IF_SET_IN_BITMAP (l->statics_written, 0, index, bi)
! 		lto_output_var_decl_index(ob->decl_state, ob->main_stream,
! 					  get_static_decl (index));
! 	    }
  	}
      }
    lto_destroy_simple_output_block (ob);
*************** ipa_reference_read_summary (void)
*** 1101,1107 ****
  	      unsigned int j, index;
  	      struct cgraph_node *node;
  	      ipa_reference_local_vars_info_t l;
! 	      unsigned int v_count;
  	      lto_cgraph_encoder_t encoder;
  
  	      index = lto_input_uleb128 (ib);
--- 1103,1109 ----
  	      unsigned int j, index;
  	      struct cgraph_node *node;
  	      ipa_reference_local_vars_info_t l;
! 	      int v_count;
  	      lto_cgraph_encoder_t encoder;
  
  	      index = lto_input_uleb128 (ib);
*************** ipa_reference_read_summary (void)
*** 1110,1135 ****
  	      l = init_function_info (node);
  
  	      /* Set the statics read.  */
! 	      v_count = lto_input_uleb128 (ib);
! 	      for (j = 0; j < v_count; j++)
! 		{
! 		  unsigned int var_index = lto_input_uleb128 (ib);
! 		  tree v_decl = lto_file_decl_data_get_var_decl (file_data,
! 								 var_index);
! 		  add_static_var (v_decl);
! 		  bitmap_set_bit (l->statics_read, DECL_UID (v_decl));
! 		} 
  
  	      /* Set the statics written.  */
! 	      v_count = lto_input_uleb128 (ib);
! 	      for (j = 0; j < v_count; j++)
! 		{
! 		  unsigned int var_index = lto_input_uleb128 (ib);
! 		  tree v_decl = lto_file_decl_data_get_var_decl (file_data,
! 								 var_index);
! 		  add_static_var (v_decl);
! 		  bitmap_set_bit (l->statics_written, DECL_UID (v_decl));
! 		} 
  	    }
  
  	  lto_destroy_simple_input_block (file_data, 
--- 1112,1143 ----
  	      l = init_function_info (node);
  
  	      /* Set the statics read.  */
! 	      v_count = lto_input_sleb128 (ib);
! 	      if (v_count == -1)
! 	        l->calls_read_all = true;
! 	      else
! 		for (j = 0; j < (unsigned int)v_count; j++)
! 		  {
! 		    unsigned int var_index = lto_input_uleb128 (ib);
! 		    tree v_decl = lto_file_decl_data_get_var_decl (file_data,
! 								   var_index);
! 		    add_static_var (v_decl);
! 		    bitmap_set_bit (l->statics_read, DECL_UID (v_decl));
! 		  } 
  
  	      /* Set the statics written.  */
! 	      v_count = lto_input_sleb128 (ib);
! 	      if (v_count == -1)
! 	        l->calls_read_all = true;
! 	      else
! 		for (j = 0; j < (unsigned int)v_count; j++)
! 		  {
! 		    unsigned int var_index = lto_input_uleb128 (ib);
! 		    tree v_decl = lto_file_decl_data_get_var_decl (file_data,
! 								   var_index);
! 		    add_static_var (v_decl);
! 		    bitmap_set_bit (l->statics_written, DECL_UID (v_decl));
! 		  } 
  	    }
  
  	  lto_destroy_simple_input_block (file_data, 
*************** ipa_reference_read_summary (void)
*** 1141,1146 ****
--- 1149,1174 ----
  
  
  \f
+ /* Set READ_ALL/WRITE_ALL based on DECL flags.  */
+ static void
+ read_write_all_from_decl (tree decl, bool * read_all, bool * write_all)
+ {
+   int flags = flags_from_decl_or_type (decl);
+   if (flags & ECF_CONST)
+     ;
+   else if (flags & ECF_PURE)
+     *read_all = true;
+   else
+     {
+        /* TODO: To be able to produce sane results, we should also handle
+ 	  common builtins, in particular throw.
+ 	  Indirect calls hsould be only counted and as inliner is replacing them
+ 	  by direct calls, we can conclude if any indirect calls are left in body */
+       *read_all = true;
+       *write_all = true;
+     }
+ }
+ 
  /* Produce the global information by preforming a transitive closure
     on the local information that was produced by ipa_analyze_function
     and ipa_analyze_variable.  */
*************** propagate (void)
*** 1173,1178 ****
--- 1201,1207 ----
        ipa_reference_global_vars_info_t node_g = 
  	XCNEW (struct ipa_reference_global_vars_info_d);
        ipa_reference_local_vars_info_t node_l;
+       struct cgraph_edge *e;
        
        bool read_all;
        bool write_all;
*************** propagate (void)
*** 1193,1198 ****
--- 1222,1236 ----
        read_all = node_l->calls_read_all;
        write_all = node_l->calls_write_all;
  
+       /* When function is overwrittable, we can not assume anything.  */
+       if (cgraph_function_body_availability (node) <= AVAIL_OVERWRITABLE)
+         read_write_all_from_decl (node->decl, &read_all, &write_all);
+ 
+       for (e = node->callees; e; e = e->next_callee) 
+         if (cgraph_function_body_availability (e->callee) <= AVAIL_OVERWRITABLE)
+           read_write_all_from_decl (e->callee->decl, &read_all, &write_all);
+ 
+ 
        /* If any node in a cycle is calls_read_all or calls_write_all
  	 they all are. */
        w_info = (struct ipa_dfs_info *) node->aux;
*************** propagate (void)
*** 1201,1206 ****
--- 1239,1253 ----
  	{
  	  ipa_reference_local_vars_info_t w_l = 
  	    get_reference_vars_info (w)->local;
+ 
+ 	  /* When function is overwrittable, we can not assume anything.  */
+ 	  if (cgraph_function_body_availability (w) <= AVAIL_OVERWRITABLE)
+ 	    read_write_all_from_decl (w->decl, &read_all, &write_all);
+ 
+ 	  for (e = w->callees; e; e = e->next_callee) 
+ 	    if (cgraph_function_body_availability (e->callee) <= AVAIL_OVERWRITABLE)
+ 	      read_write_all_from_decl (e->callee->decl, &read_all, &write_all);
+ 
  	  read_all |= w_l->calls_read_all;
  	  write_all |= w_l->calls_write_all;
  
Index: testsuite/gcc.c-torture/compile/pr40556.c
===================================================================
*** testsuite/gcc.c-torture/compile/pr40556.c	(revision 0)
--- testsuite/gcc.c-torture/compile/pr40556.c	(revision 0)
***************
*** 0 ****
--- 1,22 ----
+ struct A {};
+ 
+ struct A foo()
+ {
+   return foo();
+ }
+ 
+ void bar()
+ {
+   foo();
+ }
+ struct A {};
+ 
+ struct A foo()
+ {
+   return foo();
+ }
+ 
+ void bar()
+ {
+   foo();
+ }
Index: ipa-pure-const.c
===================================================================
*** ipa-pure-const.c	(revision 153448)
--- ipa-pure-const.c	(working copy)
*************** check_call (funct_state local, gimple ca
*** 330,341 ****
    /* When not in IPA mode, we can still handle self recursion.  */
    if (!ipa && callee_t == current_function_decl)
      local->looping = true;
!   /* The callee is either unknown (indirect call) or there is just no
!      scannable code for it (external call) .  We look to see if there
!      are any bits available for the callee (such as by declaration or
!      because it is builtin) and process solely on the basis of those
!      bits. */
!   else if (avail <= AVAIL_OVERWRITABLE || !ipa)
      {
        if (possibly_throws && flag_non_call_exceptions)
          {
--- 330,340 ----
    /* When not in IPA mode, we can still handle self recursion.  */
    if (!ipa && callee_t == current_function_decl)
      local->looping = true;
!   /* Either calle is unknown or we are doing local analysis.
!      Look to see if there are any bits available for the callee (such as by
!      declaration or because it is builtin) and process solely on the basis of
!      those bits. */
!   else if (!ipa || !callee_t)
      {
        if (possibly_throws && flag_non_call_exceptions)
          {
*************** analyze_function (struct cgraph_node *fn
*** 492,504 ****
    funct_state l;
    basic_block this_block;
  
-   if (cgraph_function_body_availability (fn) <= AVAIL_OVERWRITABLE)
-     {
-       if (dump_file)
-         fprintf (dump_file, "Function is not available or overwrittable; not analyzing.\n");
-       return NULL;
-     }
- 
    l = XCNEW (struct funct_state_d);
    l->pure_const_state = IPA_CONST;
    l->state_previously_known = IPA_NEITHER;
--- 491,496 ----
*************** end:
*** 609,615 ****
  static void
  add_new_function (struct cgraph_node *node, void *data ATTRIBUTE_UNUSED)
  {
!  if (cgraph_function_body_availability (node) <= AVAIL_OVERWRITABLE)
     return;
    /* There are some shared nodes, in particular the initializers on
       static declarations.  We do not need to scan them more than once
--- 601,607 ----
  static void
  add_new_function (struct cgraph_node *node, void *data ATTRIBUTE_UNUSED)
  {
!  if (cgraph_function_body_availability (node) < AVAIL_OVERWRITABLE)
     return;
    /* There are some shared nodes, in particular the initializers on
       static declarations.  We do not need to scan them more than once
*************** generate_summary (void)
*** 686,697 ****
  
    /* Process all of the functions. 
  
!      We do NOT process any AVAIL_OVERWRITABLE functions, we cannot
!      guarantee that what we learn about the one we see will be true
!      for the one that overrides it.
!   */
    for (node = cgraph_nodes; node; node = node->next)
!     if (cgraph_function_body_availability (node) > AVAIL_OVERWRITABLE)
        set_function_state (node, analyze_function (node, true));
  
    pointer_set_destroy (visited_nodes);
--- 678,689 ----
  
    /* Process all of the functions. 
  
!      We process AVAIL_OVERWRITABLE functions.  We can not use the results
!      by default, but the info can be used at LTO with -fwhole-program or
!      when function got clonned and the clone is AVAILABLE.  */
! 
    for (node = cgraph_nodes; node; node = node->next)
!     if (cgraph_function_body_availability (node) >= AVAIL_OVERWRITABLE)
        set_function_state (node, analyze_function (node, true));
  
    pointer_set_destroy (visited_nodes);
*************** propagate (void)
*** 878,883 ****
--- 870,881 ----
  
  	  if (w_l->looping)
  	    looping = true;
+ 	  if (cgraph_function_body_availability (w) == AVAIL_OVERWRITABLE)
+ 	    {
+ 	      looping |= w_l->looping_previously_known;
+ 	      if (pure_const_state < w_l->state_previously_known)
+ 	        pure_const_state = w_l->state_previously_known;
+ 	    }
  
  	  if (pure_const_state == IPA_NEITHER)
  	    break;
*************** propagate (void)
*** 901,906 ****
--- 899,918 ----
  		  if (y_l->looping)
  		    looping = true;
  		}
+ 	      else
+ 	        {
+ 		  int flags = flags_from_decl_or_type (y->decl);
+ 
+ 		  if (flags & ECF_LOOPING_CONST_OR_PURE)
+ 		    looping = true;
+ 		  if (flags & ECF_CONST)
+ 		    ;
+ 		  else if ((flags & ECF_PURE) && pure_const_state == IPA_CONST)
+ 		    pure_const_state = IPA_PURE;
+ 		  else
+ 		    pure_const_state = IPA_NEITHER, looping = true;
+ 
+ 		}
  	    }
  	  w_info = (struct ipa_dfs_info *) w->aux;
  	  w = w_info->next_cycle;
*************** propagate (void)
*** 988,994 ****
  	  struct cgraph_edge *e;
  	  funct_state w_l = get_function_state (w);
  
! 	  if (w_l->can_throw)
  	    can_throw = true;
  
  	  if (can_throw)
--- 1000,1007 ----
  	  struct cgraph_edge *e;
  	  funct_state w_l = get_function_state (w);
  
! 	  if (w_l->can_throw
! 	      || cgraph_function_body_availability (w) == AVAIL_OVERWRITABLE)
  	    can_throw = true;
  
  	  if (can_throw)
*************** propagate (void)
*** 1008,1013 ****
--- 1021,1028 ----
  		      && e->can_throw_external)
  		    can_throw = true;
  		}
+ 	      else if (e->can_throw_external && !TREE_NOTHROW (y->decl))
+ 	        can_throw = true;
  	    }
  	  w_info = (struct ipa_dfs_info *) w->aux;
  	  w = w_info->next_cycle;
*************** propagate (void)
*** 1046,1052 ****
  	  free (node->aux);
  	  node->aux = NULL;
  	}
!       if (cgraph_function_body_availability (node) > AVAIL_OVERWRITABLE)
  	free (get_function_state (node));
      }
    
--- 1061,1067 ----
  	  free (node->aux);
  	  node->aux = NULL;
  	}
!       if (cgraph_function_body_availability (node) >= AVAIL_OVERWRITABLE)
  	free (get_function_state (node));
      }
    
*************** local_pure_const (void)
*** 1109,1123 ****
          fprintf (dump_file, "Function called in recursive cycle; ignoring\n");
        return 0;
      }
! 
!   l = analyze_function (cgraph_node (current_function_decl), false);
!   if (!l)
      {
        if (dump_file)
          fprintf (dump_file, "Function has wrong visibility; ignoring\n");
        return 0;
      }
  
    switch (l->pure_const_state)
      {
      case IPA_CONST:
--- 1124,1139 ----
          fprintf (dump_file, "Function called in recursive cycle; ignoring\n");
        return 0;
      }
!   if (cgraph_function_body_availability (cgraph_node (current_function_decl))
!       <= AVAIL_OVERWRITABLE)
      {
        if (dump_file)
          fprintf (dump_file, "Function has wrong visibility; ignoring\n");
        return 0;
      }
  
+   l = analyze_function (cgraph_node (current_function_decl), false);
+ 
    switch (l->pure_const_state)
      {
      case IPA_CONST:

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2009-10-22 10:27 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-22 10:42 Fix ipa-pure-const and ipa-reference for LTO 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).