[PATCH] Make disabled-optimization warning more informative; increase default max-gcse-memory

[PATCH] Make disabled-optimization warning more informative; increase default max-gcse-memory

[Bradley Lucier]

[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]

This patch (a) removes an exact copy of is_too_expensive from cprop.c, 
(b) renames is_too_expensive in gcse.c to 
gcse_or_cprop_is_too_expensive, (c) expands the warning in 
gcse_or_cprop_is_too_expensive to say how much --param max-gcse-memory 
needs to be increased, and (d) increases the default max-gcse-memory 
from 50MB to 128MB.

The expanded warning allowed me to see how much memory really was needed 
to apply gcse to some of my routines, and 128MB fixes my problem.  The 
limit has been 50MB for over 10 years, I think we can up it a bit now.

Bootstrapped and checked, default languages, no regressions.

Brad

	* gcc/cprop.c (is_too_expensive): Remove.
	(gcse.h): Include.
	(one_cprop_pass): Call gcse_or_cprop_is_too_expensive, not
	is_too_expensive.
	* gcc/gcse.h (gcse_or_cprop_is_too_expensive): Declare.
	* gcc/gcse.c (is_too_expensive): Rename to ...
	(gcse_or_cprop_is_too_expensive): ... this.
	Expand warning to add required size of max-gcse-memory.
	(one_pre_gcse_pass): Use it.
	(one_code_hoisting_pass): Use it.
	* gcc/params.def (max-gcse-memory): Increase from 50MB to 128MB.



[-- Attachment #2: gcse.patch --]
[-- Type: text/x-patch, Size: 5349 bytes --]

Index: gcc/gcse.c
===================================================================
--- gcc/gcse.c	(revision 230136)
+++ gcc/gcse.c	(working copy)
@@ -510,7 +510,6 @@
 static void update_ld_motion_stores (struct gcse_expr *);
 static void clear_modify_mem_tables (void);
 static void free_modify_mem_tables (void);
-static bool is_too_expensive (const char *);
 
 #define GNEW(T)			((T *) gmalloc (sizeof (T)))
 #define GCNEW(T)		((T *) gcalloc (1, sizeof (T)))
@@ -2565,7 +2564,7 @@
 
   /* Return if there's nothing to do, or it is too expensive.  */
   if (n_basic_blocks_for_fn (cfun) <= NUM_FIXED_BLOCKS + 1
-      || is_too_expensive (_("PRE disabled")))
+      || gcse_or_cprop_is_too_expensive (_("PRE disabled")))
     return 0;
 
   /* We need alias.  */
@@ -3493,7 +3492,7 @@
 
   /* Return if there's nothing to do, or it is too expensive.  */
   if (n_basic_blocks_for_fn (cfun) <= NUM_FIXED_BLOCKS + 1
-      || is_too_expensive (_("GCSE disabled")))
+      || gcse_or_cprop_is_too_expensive (_("GCSE disabled")))
     return 0;
 
   doing_code_hoisting_p = true;
@@ -3957,9 +3956,13 @@
 /* Return true if the graph is too expensive to optimize. PASS is the
    optimization about to be performed.  */
 
-static bool
-is_too_expensive (const char *pass)
+bool
+gcse_or_cprop_is_too_expensive (const char *pass)
 {
+  unsigned int memory_request = n_basic_blocks_for_fn (cfun)
+    * SBITMAP_SET_SIZE (max_reg_num ())
+    * sizeof (SBITMAP_ELT_TYPE);
+  
   /* Trying to perform global optimizations on flow graphs which have
      a high connectivity will take a long time and is unlikely to be
      particularly useful.
@@ -3981,13 +3984,12 @@
 
   /* If allocating memory for the dataflow bitmaps would take up too much
      storage it's better just to disable the optimization.  */
-  if ((n_basic_blocks_for_fn (cfun)
-       * SBITMAP_SET_SIZE (max_reg_num ())
-       * sizeof (SBITMAP_ELT_TYPE)) > MAX_GCSE_MEMORY)
+  if (memory_request > MAX_GCSE_MEMORY)
     {
       warning (OPT_Wdisabled_optimization,
-	       "%s: %d basic blocks and %d registers",
-	       pass, n_basic_blocks_for_fn (cfun), max_reg_num ());
+	       "%s: %d basic blocks and %d registers; increase --param max-gcse-memory above %d",
+	       pass, n_basic_blocks_for_fn (cfun), max_reg_num (),
+	       memory_request);
 
       return true;
     }
Index: gcc/gcse.h
===================================================================
--- gcc/gcse.h	(revision 230136)
+++ gcc/gcse.h	(working copy)
@@ -40,5 +40,6 @@
 #endif
 
 void gcse_c_finalize (void);
+extern bool gcse_or_cprop_is_too_expensive (const char *);
 
 #endif
Index: gcc/cprop.c
===================================================================
--- gcc/cprop.c	(revision 230136)
+++ gcc/cprop.c	(working copy)
@@ -39,6 +39,7 @@
 #include "tree-pass.h"
 #include "dbgcnt.h"
 #include "cfgloop.h"
+#include "gcse.h"
 
 \f
 /* An obstack for our working variables.  */
@@ -1724,47 +1725,6 @@
   return changed;
 }
 \f
-/* Return true if the graph is too expensive to optimize.  PASS is the
-   optimization about to be performed.  */
-
-static bool
-is_too_expensive (const char *pass)
-{
-  /* Trying to perform global optimizations on flow graphs which have
-     a high connectivity will take a long time and is unlikely to be
-     particularly useful.
-
-     In normal circumstances a cfg should have about twice as many
-     edges as blocks.  But we do not want to punish small functions
-     which have a couple switch statements.  Rather than simply
-     threshold the number of blocks, uses something with a more
-     graceful degradation.  */
-  if (n_edges_for_fn (cfun) > 20000 + n_basic_blocks_for_fn (cfun) * 4)
-    {
-      warning (OPT_Wdisabled_optimization,
-	       "%s: %d basic blocks and %d edges/basic block",
-	       pass, n_basic_blocks_for_fn (cfun),
-	       n_edges_for_fn (cfun) / n_basic_blocks_for_fn (cfun));
-
-      return true;
-    }
-
-  /* If allocating memory for the cprop bitmap would take up too much
-     storage it's better just to disable the optimization.  */
-  if ((n_basic_blocks_for_fn (cfun)
-       * SBITMAP_SET_SIZE (max_reg_num ())
-       * sizeof (SBITMAP_ELT_TYPE)) > MAX_GCSE_MEMORY)
-    {
-      warning (OPT_Wdisabled_optimization,
-	       "%s: %d basic blocks and %d registers",
-	       pass, n_basic_blocks_for_fn (cfun), max_reg_num ());
-
-      return true;
-    }
-
-  return false;
-}
-\f
 /* Main function for the CPROP pass.  */
 
 static int
@@ -1775,7 +1735,7 @@
 
   /* Return if there's nothing to do, or it is too expensive.  */
   if (n_basic_blocks_for_fn (cfun) <= NUM_FIXED_BLOCKS + 1
-      || is_too_expensive (_ ("const/copy propagation disabled")))
+      || gcse_or_cprop_is_too_expensive (_ ("const/copy propagation disabled")))
     return 0;
 
   global_const_prop_count = local_const_prop_count = 0;
Index: gcc/params.def
===================================================================
--- gcc/params.def	(revision 230136)
+++ gcc/params.def	(working copy)
@@ -218,7 +218,7 @@
 DEFPARAM(PARAM_MAX_GCSE_MEMORY,
 	 "max-gcse-memory",
 	 "The maximum amount of memory to be allocated by GCSE.",
-	 50 * 1024 * 1024, 0, 0)
+	 128 * 1024 * 1024, 0, 0)
 
 /* The GCSE optimization of an expression will avoided if the ratio of
    insertions to deletions is greater than this value.  */

Re: [PATCH] Make disabled-optimization warning more informative; increase default max-gcse-memory

[Bernd Schmidt]

> The expanded warning allowed me to see how much memory really was needed
> to apply gcse to some of my routines, and 128MB fixes my problem.  The
> limit has been 50MB for over 10 years, I think we can up it a bit now.
>   {
> +  unsigned int memory_request = n_basic_blocks_for_fn (cfun)
> +    * SBITMAP_SET_SIZE (max_reg_num ())
> +    * sizeof (SBITMAP_ELT_TYPE);
> +

Formatting (wrap in parens to get it indented). Could also move the 
initialization before the use.

Otherwise ok.


Bernd

Re: [PATCH] Make disabled-optimization warning more informative; increase default max-gcse-memory

[Bradley Lucier]

[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]

On 11/12/2015 11:57 AM, Bernd Schmidt wrote:
>> The expanded warning allowed me to see how much memory really was needed
>> to apply gcse to some of my routines, and 128MB fixes my problem.  The
>> limit has been 50MB for over 10 years, I think we can up it a bit now.
>>   {
>> +  unsigned int memory_request = n_basic_blocks_for_fn (cfun)
>> +    * SBITMAP_SET_SIZE (max_reg_num ())
>> +    * sizeof (SBITMAP_ELT_TYPE);
>> +
>
> Formatting (wrap in parens to get it indented).

Fixed.

>
> Otherwise ok.

Thanks.

Here's the reformatted patch (but without retesting).

I don't have check-in privileges.

Brad

	* gcc/cprop.c (is_too_expensive): Remove.
	(gcse.h): Include.
	(one_cprop_pass): Call gcse_or_cprop_is_too_expensive, not
	is_too_expensive.
	* gcc/gcse.h (gcse_or_cprop_is_too_expensive): Declare.
	* gcc/gcse.c (is_too_expensive): Rename to ...
	(gcse_or_cprop_is_too_expensive): ... this.
	Expand warning to add required size of max-gcse-memory.
	(one_pre_gcse_pass): Use it.
	(one_code_hoisting_pass): Use it.
	* gcc/params.def (max-gcse-memory): Increase from 50MB to 128MB.


[-- Attachment #2: gcse.patch --]
[-- Type: text/x-patch, Size: 5353 bytes --]

Index: gcc/gcse.c
===================================================================
--- gcc/gcse.c	(revision 230136)
+++ gcc/gcse.c	(working copy)
@@ -510,7 +510,6 @@
 static void update_ld_motion_stores (struct gcse_expr *);
 static void clear_modify_mem_tables (void);
 static void free_modify_mem_tables (void);
-static bool is_too_expensive (const char *);
 
 #define GNEW(T)			((T *) gmalloc (sizeof (T)))
 #define GCNEW(T)		((T *) gcalloc (1, sizeof (T)))
@@ -2565,7 +2564,7 @@
 
   /* Return if there's nothing to do, or it is too expensive.  */
   if (n_basic_blocks_for_fn (cfun) <= NUM_FIXED_BLOCKS + 1
-      || is_too_expensive (_("PRE disabled")))
+      || gcse_or_cprop_is_too_expensive (_("PRE disabled")))
     return 0;
 
   /* We need alias.  */
@@ -3493,7 +3492,7 @@
 
   /* Return if there's nothing to do, or it is too expensive.  */
   if (n_basic_blocks_for_fn (cfun) <= NUM_FIXED_BLOCKS + 1
-      || is_too_expensive (_("GCSE disabled")))
+      || gcse_or_cprop_is_too_expensive (_("GCSE disabled")))
     return 0;
 
   doing_code_hoisting_p = true;
@@ -3957,9 +3956,13 @@
 /* Return true if the graph is too expensive to optimize. PASS is the
    optimization about to be performed.  */
 
-static bool
-is_too_expensive (const char *pass)
+bool
+gcse_or_cprop_is_too_expensive (const char *pass)
 {
+  unsigned int memory_request = (n_basic_blocks_for_fn (cfun)
+				 * SBITMAP_SET_SIZE (max_reg_num ())
+				 * sizeof (SBITMAP_ELT_TYPE));
+  
   /* Trying to perform global optimizations on flow graphs which have
      a high connectivity will take a long time and is unlikely to be
      particularly useful.
@@ -3981,13 +3984,12 @@
 
   /* If allocating memory for the dataflow bitmaps would take up too much
      storage it's better just to disable the optimization.  */
-  if ((n_basic_blocks_for_fn (cfun)
-       * SBITMAP_SET_SIZE (max_reg_num ())
-       * sizeof (SBITMAP_ELT_TYPE)) > MAX_GCSE_MEMORY)
+  if (memory_request > MAX_GCSE_MEMORY)
     {
       warning (OPT_Wdisabled_optimization,
-	       "%s: %d basic blocks and %d registers",
-	       pass, n_basic_blocks_for_fn (cfun), max_reg_num ());
+	       "%s: %d basic blocks and %d registers; increase --param max-gcse-memory above %d",
+	       pass, n_basic_blocks_for_fn (cfun), max_reg_num (),
+	       memory_request);
 
       return true;
     }
Index: gcc/gcse.h
===================================================================
--- gcc/gcse.h	(revision 230136)
+++ gcc/gcse.h	(working copy)
@@ -40,5 +40,6 @@
 #endif
 
 void gcse_c_finalize (void);
+extern bool gcse_or_cprop_is_too_expensive (const char *);
 
 #endif
Index: gcc/cprop.c
===================================================================
--- gcc/cprop.c	(revision 230136)
+++ gcc/cprop.c	(working copy)
@@ -39,6 +39,7 @@
 #include "tree-pass.h"
 #include "dbgcnt.h"
 #include "cfgloop.h"
+#include "gcse.h"
 
 \f
 /* An obstack for our working variables.  */
@@ -1724,47 +1725,6 @@
   return changed;
 }
 \f
-/* Return true if the graph is too expensive to optimize.  PASS is the
-   optimization about to be performed.  */
-
-static bool
-is_too_expensive (const char *pass)
-{
-  /* Trying to perform global optimizations on flow graphs which have
-     a high connectivity will take a long time and is unlikely to be
-     particularly useful.
-
-     In normal circumstances a cfg should have about twice as many
-     edges as blocks.  But we do not want to punish small functions
-     which have a couple switch statements.  Rather than simply
-     threshold the number of blocks, uses something with a more
-     graceful degradation.  */
-  if (n_edges_for_fn (cfun) > 20000 + n_basic_blocks_for_fn (cfun) * 4)
-    {
-      warning (OPT_Wdisabled_optimization,
-	       "%s: %d basic blocks and %d edges/basic block",
-	       pass, n_basic_blocks_for_fn (cfun),
-	       n_edges_for_fn (cfun) / n_basic_blocks_for_fn (cfun));
-
-      return true;
-    }
-
-  /* If allocating memory for the cprop bitmap would take up too much
-     storage it's better just to disable the optimization.  */
-  if ((n_basic_blocks_for_fn (cfun)
-       * SBITMAP_SET_SIZE (max_reg_num ())
-       * sizeof (SBITMAP_ELT_TYPE)) > MAX_GCSE_MEMORY)
-    {
-      warning (OPT_Wdisabled_optimization,
-	       "%s: %d basic blocks and %d registers",
-	       pass, n_basic_blocks_for_fn (cfun), max_reg_num ());
-
-      return true;
-    }
-
-  return false;
-}
-\f
 /* Main function for the CPROP pass.  */
 
 static int
@@ -1775,7 +1735,7 @@
 
   /* Return if there's nothing to do, or it is too expensive.  */
   if (n_basic_blocks_for_fn (cfun) <= NUM_FIXED_BLOCKS + 1
-      || is_too_expensive (_ ("const/copy propagation disabled")))
+      || gcse_or_cprop_is_too_expensive (_ ("const/copy propagation disabled")))
     return 0;
 
   global_const_prop_count = local_const_prop_count = 0;
Index: gcc/params.def
===================================================================
--- gcc/params.def	(revision 230136)
+++ gcc/params.def	(working copy)
@@ -218,7 +218,7 @@
 DEFPARAM(PARAM_MAX_GCSE_MEMORY,
 	 "max-gcse-memory",
 	 "The maximum amount of memory to be allocated by GCSE.",
-	 50 * 1024 * 1024, 0, 0)
+	 128 * 1024 * 1024, 0, 0)
 
 /* The GCSE optimization of an expression will avoided if the ratio of
    insertions to deletions is greater than this value.  */

Re: [PATCH] Make disabled-optimization warning more informative; increase default max-gcse-memory

[Bradley Lucier]

On 11/12/2015 12:08 PM, Bradley Lucier wrote:
> On 11/12/2015 11:57 AM, Bernd Schmidt wrote:
>>> The expanded warning allowed me to see how much memory really was needed
>>> to apply gcse to some of my routines, and 128MB fixes my problem.  The
>>> limit has been 50MB for over 10 years, I think we can up it a bit now.
>>>   {
>>> +  unsigned int memory_request = n_basic_blocks_for_fn (cfun)
>>> +    * SBITMAP_SET_SIZE (max_reg_num ())
>>> +    * sizeof (SBITMAP_ELT_TYPE);
>>> +
>>
>> Formatting (wrap in parens to get it indented).
>
> Fixed.
>
>>
>> Otherwise ok.
>
> Thanks.
>
> Here's the reformatted patch (but without retesting).

To check reformatting, bootstrapped on x86_64-pc-linux-gnu with 
--enable-languages=c,c++ --disable-multilib.

Brad

Re: [PATCH] Make disabled-optimization warning more informative; increase default max-gcse-memory

[Jeff Law]

On 11/12/2015 10:08 AM, Bradley Lucier wrote:
> On 11/12/2015 11:57 AM, Bernd Schmidt wrote:
>>> The expanded warning allowed me to see how much memory really was needed
>>> to apply gcse to some of my routines, and 128MB fixes my problem.  The
>>> limit has been 50MB for over 10 years, I think we can up it a bit now.
>>>   {
>>> +  unsigned int memory_request = n_basic_blocks_for_fn (cfun)
>>> +    * SBITMAP_SET_SIZE (max_reg_num ())
>>> +    * sizeof (SBITMAP_ELT_TYPE);
>>> +
>>
>> Formatting (wrap in parens to get it indented).
>
> Fixed.
>
>>
>> Otherwise ok.
>
> Thanks.
>
> Here's the reformatted patch (but without retesting).
>
> I don't have check-in privileges.
>
> Brad
>
>      * gcc/cprop.c (is_too_expensive): Remove.
>      (gcse.h): Include.
>      (one_cprop_pass): Call gcse_or_cprop_is_too_expensive, not
>      is_too_expensive.
>      * gcc/gcse.h (gcse_or_cprop_is_too_expensive): Declare.
>      * gcc/gcse.c (is_too_expensive): Rename to ...
>      (gcse_or_cprop_is_too_expensive): ... this.
>      Expand warning to add required size of max-gcse-memory.
>      (one_pre_gcse_pass): Use it.
>      (one_code_hoisting_pass): Use it.
>      * gcc/params.def (max-gcse-memory): Increase from 50MB to 128MB.
Committed on your behalf.

jeff