diff mbox series

[lto] ipcp don't propagate where not needed

Message ID bq4rylugldgdfkoy3nba4jm6weqxm6rrsir7yp4fiakgffhzq4@styehyzvtg7q
State New
Headers
Series [lto] ipcp don't propagate where not needed |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Michal Jireš Oct. 24, 2024, 12:55 p.m. UTC 
  This patch disables propagation of ipcp information into lto partitions
where all instances of the node are marked to be inlined.

Motivation:
Incremental LTO needs stable values between compilations to be
effective. This requirement fails with following example:

void heavily_used_debug_function(int);
...
heavily_used_debug_function(__LINE__);

Ipcp creates long list of all __LINE__ arguments, and then
propagates it with every function clone, even though for inlined
functions this information is not useful.

gcc/ChangeLog:

	* ipa-prop.cc (write_ipcp_transformation_info): Disable
	  uneeded value propagation.
	* lto-cgraph.cc (lto_symtab_encoder_encode): Default values.
	(lto_symtab_encoder_always_inlined_p): New.
	(lto_set_symtab_encoder_not_always_inlined): New.
	(add_node_to): Set always inlined.
	* lto-streamer.h (struct lto_encoder_entry): New field.
	(lto_symtab_encoder_always_inlined_p): New.
---
 gcc/ipa-prop.cc    | 12 +++++++++---
 gcc/lto-cgraph.cc  | 41 +++++++++++++++++++++++++++++++++++++----
 gcc/lto-streamer.h |  4 ++++
 3 files changed, 50 insertions(+), 7 deletions(-)

Comments

Jan Hubicka Oct. 24, 2024, 2:16 p.m. UTC | #1 
> This patch disables propagation of ipcp information into lto partitions
> where all instances of the node are marked to be inlined.
> 
> Motivation:
> Incremental LTO needs stable values between compilations to be
> effective. This requirement fails with following example:
> 
> void heavily_used_debug_function(int);
> ...
> heavily_used_debug_function(__LINE__);
> 
> Ipcp creates long list of all __LINE__ arguments, and then
> propagates it with every function clone, even though for inlined
> functions this information is not useful.
> 
> gcc/ChangeLog:
> 
> 	* ipa-prop.cc (write_ipcp_transformation_info): Disable
> 	  uneeded value propagation.
> 	* lto-cgraph.cc (lto_symtab_encoder_encode): Default values.
> 	(lto_symtab_encoder_always_inlined_p): New.
> 	(lto_set_symtab_encoder_not_always_inlined): New.
> 	(add_node_to): Set always inlined.
> 	* lto-streamer.h (struct lto_encoder_entry): New field.
> 	(lto_symtab_encoder_always_inlined_p): New.
OK
> ---
>  gcc/ipa-prop.cc    | 12 +++++++++---
>  gcc/lto-cgraph.cc  | 41 +++++++++++++++++++++++++++++++++++++----
>  gcc/lto-streamer.h |  4 ++++
>  3 files changed, 50 insertions(+), 7 deletions(-)
> 
> diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
> index 032358fde22..ef83ce3edb6 100644
> --- a/gcc/ipa-prop.cc
> +++ b/gcc/ipa-prop.cc
> @@ -5404,9 +5404,15 @@ write_ipcp_transformation_info (output_block *ob, cgraph_node *node,
>        streamer_write_bitpack (&bp);
>      }
>  
> -  streamer_write_uhwi (ob, vec_safe_length (ts->m_vr));
> -  for (const ipa_vr &parm_vr : ts->m_vr)
> -    parm_vr.streamer_write (ob);
> +  /* If all instances of this node are inlined, ipcp info is not useful.  */
> +  if (!lto_symtab_encoder_always_inlined_p (encoder, node))
> +    {
> +      streamer_write_uhwi (ob, vec_safe_length (ts->m_vr));
> +      for (const ipa_vr &parm_vr : ts->m_vr)
> +	parm_vr.streamer_write (ob);
> +    }
> +  else
> +    streamer_write_uhwi (ob, 0);
>  }
>  

>    if (!encoder->map)
>      {
> -      lto_encoder_entry entry = {node, false, false, false};
> +      lto_encoder_entry entry = {node, false, false, true, false};
>  
>        ref = encoder->nodes.length ();
>        encoder->nodes.safe_push (entry);
> @@ -124,7 +124,7 @@ lto_symtab_encoder_encode (lto_symtab_encoder_t encoder,
>    size_t *slot = encoder->map->get (node);
>    if (!slot || !*slot)
>      {
> -      lto_encoder_entry entry = {node, false, false, false};
> +      lto_encoder_entry entry = {node, false, false, true, false};

I wonder if we should enjoy C++ and add constructor for the entry :)
> +bool
> +lto_symtab_encoder_always_inlined_p (lto_symtab_encoder_t encoder,
> +				     struct cgraph_node *node)

always_inlined kind of triggers connection to always_inline attribute.
Perhaps the flag can be called only_for_inlining
to avoid confussion...
> +{
> +  int index = lto_symtab_encoder_lookup (encoder, node);
> +  return encoder->nodes[index].always_inlined;
> +}
> +
> +/* Specify that the NODE and its clones are not always inlined.  */
> +
> +static void
> +lto_set_symtab_encoder_not_always_inlined (lto_symtab_encoder_t encoder,
> +					   struct cgraph_node *node)
> +{
> +  int index = lto_symtab_encoder_encode (encoder, node);
> +  gcc_checking_assert (encoder->nodes[index].node == node);
> +  encoder->nodes[index].always_inlined = false;
> +}
> +
>  /* Return TRUE if we should encode initializer of NODE (if any).  */
>  
>  bool
> @@ -799,15 +820,27 @@ output_refs (lto_symtab_encoder_t encoder)
>  
>  static void
>  add_node_to (lto_symtab_encoder_t encoder, struct cgraph_node *node,
> -	     bool include_body)
> +	     bool include_body, bool not_inlined)
>  {
>    if (node->clone_of)
> -    add_node_to (encoder, node->clone_of, include_body);
> +    add_node_to (encoder, node->clone_of, include_body, not_inlined);
>    if (include_body)
>      lto_set_symtab_encoder_encode_body (encoder, node);
> +  if (not_inlined)
> +    lto_set_symtab_encoder_not_always_inlined (encoder, node);
>    lto_symtab_encoder_encode (encoder, node);

This is probably originaly done to me, but this does unnecesary multiple
hash lookups.  Perhaps the flags (INLUDE_BODY, NOT_INLINED) can be
passed lto_symtab_encoder_encode?

OK whith those changes.  The patch is good idea regardless caching,
since lto summaries can trigger extra unnecesary streaming.

Thanks,
Honza
diff mbox series

Patch

diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
index 032358fde22..ef83ce3edb6 100644
--- a/gcc/ipa-prop.cc
+++ b/gcc/ipa-prop.cc
@@ -5404,9 +5404,15 @@  write_ipcp_transformation_info (output_block *ob, cgraph_node *node,
       streamer_write_bitpack (&bp);
     }
 
-  streamer_write_uhwi (ob, vec_safe_length (ts->m_vr));
-  for (const ipa_vr &parm_vr : ts->m_vr)
-    parm_vr.streamer_write (ob);
+  /* If all instances of this node are inlined, ipcp info is not useful.  */
+  if (!lto_symtab_encoder_always_inlined_p (encoder, node))
+    {
+      streamer_write_uhwi (ob, vec_safe_length (ts->m_vr));
+      for (const ipa_vr &parm_vr : ts->m_vr)
+	parm_vr.streamer_write (ob);
+    }
+  else
+    streamer_write_uhwi (ob, 0);
 }
 
 /* Stream in the aggregate value replacement chain for NODE from IB.  */
diff --git a/gcc/lto-cgraph.cc b/gcc/lto-cgraph.cc
index 53cc965bdfd..5c3e3076c8d 100644
--- a/gcc/lto-cgraph.cc
+++ b/gcc/lto-cgraph.cc
@@ -114,7 +114,7 @@  lto_symtab_encoder_encode (lto_symtab_encoder_t encoder,
 
   if (!encoder->map)
     {
-      lto_encoder_entry entry = {node, false, false, false};
+      lto_encoder_entry entry = {node, false, false, true, false};
 
       ref = encoder->nodes.length ();
       encoder->nodes.safe_push (entry);
@@ -124,7 +124,7 @@  lto_symtab_encoder_encode (lto_symtab_encoder_t encoder,
   size_t *slot = encoder->map->get (node);
   if (!slot || !*slot)
     {
-      lto_encoder_entry entry = {node, false, false, false};
+      lto_encoder_entry entry = {node, false, false, true, false};
       ref = encoder->nodes.length ();
       if (!slot)
         encoder->map->put (node, ref + 1);
@@ -190,6 +190,27 @@  lto_set_symtab_encoder_encode_body (lto_symtab_encoder_t encoder,
   encoder->nodes[index].body = true;
 }
 
+/* Return TRUE if the NODE and its clones are always inlined.  */
+
+bool
+lto_symtab_encoder_always_inlined_p (lto_symtab_encoder_t encoder,
+				     struct cgraph_node *node)
+{
+  int index = lto_symtab_encoder_lookup (encoder, node);
+  return encoder->nodes[index].always_inlined;
+}
+
+/* Specify that the NODE and its clones are not always inlined.  */
+
+static void
+lto_set_symtab_encoder_not_always_inlined (lto_symtab_encoder_t encoder,
+					   struct cgraph_node *node)
+{
+  int index = lto_symtab_encoder_encode (encoder, node);
+  gcc_checking_assert (encoder->nodes[index].node == node);
+  encoder->nodes[index].always_inlined = false;
+}
+
 /* Return TRUE if we should encode initializer of NODE (if any).  */
 
 bool
@@ -799,15 +820,27 @@  output_refs (lto_symtab_encoder_t encoder)
 
 static void
 add_node_to (lto_symtab_encoder_t encoder, struct cgraph_node *node,
-	     bool include_body)
+	     bool include_body, bool not_inlined)
 {
   if (node->clone_of)
-    add_node_to (encoder, node->clone_of, include_body);
+    add_node_to (encoder, node->clone_of, include_body, not_inlined);
   if (include_body)
     lto_set_symtab_encoder_encode_body (encoder, node);
+  if (not_inlined)
+    lto_set_symtab_encoder_not_always_inlined (encoder, node);
   lto_symtab_encoder_encode (encoder, node);
 }
 
+/* Add NODE into encoder as well as nodes it is cloned from.
+   Do it in a way so clones appear first.  */
+
+static void
+add_node_to (lto_symtab_encoder_t encoder, struct cgraph_node *node,
+	     bool include_body)
+{
+  add_node_to (encoder, node, include_body, include_body && !node->inlined_to);
+}
+
 /* Add all references in NODE to encoders.  */
 
 static void
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 397f5fc8d68..1a6bc9fa3a1 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -449,6 +449,8 @@  struct lto_encoder_entry
   unsigned int in_partition:1;
   /* Do we encode body in this partition?  */
   unsigned int body:1;
+  /* Do we always inline this node and its clones?  */
+  unsigned int always_inlined:1;
   /* Do we encode initializer in this partition?
      For example the readonly variable initializers are encoded to aid
      constant folding even if they are not in the partition.  */
@@ -914,6 +916,8 @@  void lto_symtab_encoder_delete (lto_symtab_encoder_t);
 bool lto_symtab_encoder_delete_node (lto_symtab_encoder_t, symtab_node *);
 bool lto_symtab_encoder_encode_body_p (lto_symtab_encoder_t,
 				       struct cgraph_node *);
+bool lto_symtab_encoder_always_inlined_p (lto_symtab_encoder_t,
+					  struct cgraph_node *);
 bool lto_symtab_encoder_in_partition_p (lto_symtab_encoder_t,
 					symtab_node *);
 void lto_set_symtab_encoder_in_partition (lto_symtab_encoder_t,