public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Bradley Lucier <lucier@math.purdue.edu>
To: Bernd Schmidt <bschmidt@redhat.com>, gcc-patches@gcc.gnu.org
Cc: Bradley Lucier <lucier@math.purdue.edu>
Subject: Re: [PATCH] Make disabled-optimization warning more informative; increase default max-gcse-memory
Date: Thu, 12 Nov 2015 17:08:00 -0000	[thread overview]
Message-ID: <5644C797.8010107@math.purdue.edu> (raw)
In-Reply-To: <5644C4F3.5030801@redhat.com>

[-- 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.  */

  reply	other threads:[~2015-11-12 17:08 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-12 16:44 Bradley Lucier
2015-11-12 16:57 ` Bernd Schmidt
2015-11-12 17:08   ` Bradley Lucier [this message]
2015-11-12 21:12     ` Bradley Lucier
2015-11-12 22:28     ` Jeff Law

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5644C797.8010107@math.purdue.edu \
    --to=lucier@math.purdue.edu \
    --cc=bschmidt@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).