diff mbox

bb partitioning vs optimize_function_for_speed_p

Message ID 4E5EAA70.8000107@codesourcery.com
State New
Headers show

Commit Message

Bernd Schmidt Aug. 31, 2011, 9:41 p.m. UTC 
On 08/29/11 18:02, Jeff Law wrote:
> On 08/26/11 08:47, Bernd Schmidt wrote:
>> In rest_of_reorder_blocks, we avoid reordering if 
>> !optimize_function_for_speed_p. However, we still call 
>> insert_section_bounary_note, which can cause problems because now, if
>> we have a sequence of HOT-COLD-HOT blocks, the second set of HOT
>> blocks will end up in the cold section. This causes assembler
>> failures when using exception handling (subtracting labels from
>> different sections).
> 
>> Unfortunately, the only way I have of reproducing it is to apply a 
>> 67-patch quilt tree backporting the preliminary shrink-wrapping
>> patches to gcc-4.6; then we get
> 
>> FAIL: g++.dg/tree-prof/partition2.C compilation,  -Os  -fprofile-use
> 
>> However, the problem is reasonably obvious. Bootstrapped and
>> currently testing in the aforementioned 4.6 tree. Ok for trunk after
>> testing there?
> OK after testing.

Thanks. Committed, but on second thought something like the below is
probably cleaner; it also avoids partitioning if we're not going to
reorder blocks. Tested along with the shrink-wrapping patches on
i686-linux and mips64-elf.


Bernd
* bb-reorder.c (insert_section_boundary_note): Don't check
	optimize_function_for_speed_p.
	(gate_handle_partition_blocks): Do it here instead.
	(gate_handle_reorder_blocks): Move preliminary checks here ...
	(rest_of_handle_reorder_blocks): ... from here.

Comments

Jeff Law Sept. 1, 2011, 4:48 a.m. UTC | #1 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/31/11 15:41, Bernd Schmidt wrote:
> On 08/29/11 18:02, Jeff Law wrote:
>> On 08/26/11 08:47, Bernd Schmidt wrote:
>>> In rest_of_reorder_blocks, we avoid reordering if 
>>> !optimize_function_for_speed_p. However, we still call 
>>> insert_section_bounary_note, which can cause problems because
>>> now, if we have a sequence of HOT-COLD-HOT blocks, the second set
>>> of HOT blocks will end up in the cold section. This causes
>>> assembler failures when using exception handling (subtracting
>>> labels from different sections).
>> 
>>> Unfortunately, the only way I have of reproducing it is to apply
>>> a 67-patch quilt tree backporting the preliminary
>>> shrink-wrapping patches to gcc-4.6; then we get
>> 
>>> FAIL: g++.dg/tree-prof/partition2.C compilation,  -Os
>>> -fprofile-use
>> 
>>> However, the problem is reasonably obvious. Bootstrapped and 
>>> currently testing in the aforementioned 4.6 tree. Ok for trunk
>>> after testing there?
>> OK after testing.
> 
> Thanks. Committed, but on second thought something like the below is 
> probably cleaner; it also avoids partitioning if we're not going to 
> reorder blocks. Tested along with the shrink-wrapping patches on 
> i686-linux and mips64-elf.
That looks OK too.
Jeff

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJOXw67AAoJEBRtltQi2kC78icH/jJAqu9jwiNIRZUgtbCu5f9i
gD7S0+BNY/mIooZoQXl6trD9K5Xuzb7Y78DAWR+Zlz10SewyD0+EMWjm+w+z00gb
UKLZSH/Qx0sgKXKWw4k72yzPVKCA62D6XskCD7gNA1hbxTp9V+ino31FE0RxaFLM
/MpcJqJaKJ72s5kHJq3MRlldY4rcfVWyFHxkTaGgMml3C61TO53o6ZIsC3xiFzUp
2hs/SUq4rJRMV5DMDZiupkcJ7yUpEGLFa5xMQnd8JCKYk4rHkDdP2NGgko5/3szW
J7xvjg4EFHkC0befzKyAVWF4eGyUmWUfog/+npvfsSM3rjS1mv126oABAnkwQsU=
=CHim
-----END PGP SIGNATURE-----
diff mbox

Patch

Index: gcc/bb-reorder.c
===================================================================
--- gcc/bb-reorder.c	(revision 178389)
+++ gcc/bb-reorder.c	(working copy)
@@ -1965,8 +1965,7 @@  insert_section_boundary_note (void)
   rtx new_note;
   int first_partition = 0;
 
-  if (!flag_reorder_blocks_and_partition
-      || !optimize_function_for_speed_p (cfun))
+  if (!flag_reorder_blocks_and_partition)
     return;
 
   FOR_EACH_BB (bb)
@@ -2296,7 +2295,17 @@  gate_handle_reorder_blocks (void)
 {
   if (targetm.cannot_modify_jumps_p ())
     return false;
-  return (optimize > 0);
+  /* Don't reorder blocks when optimizing for size because extra jump insns may
+     be created; also barrier may create extra padding.
+
+     More correctly we should have a block reordering mode that tried to
+     minimize the combined size of all the jumps.  This would more or less
+     automatically remove extra jumps, but would also try to use more short
+     jumps instead of long jumps.  */
+  if (!optimize_function_for_speed_p (cfun))
+    return false;
+  return (optimize > 0
+	  && (flag_reorder_blocks || flag_reorder_blocks_and_partition));
 }
 
 
@@ -2310,19 +2319,8 @@  rest_of_handle_reorder_blocks (void)
      splitting possibly introduced more crossjumping opportunities.  */
   cfg_layout_initialize (CLEANUP_EXPENSIVE);
 
-  if ((flag_reorder_blocks || flag_reorder_blocks_and_partition)
-      /* Don't reorder blocks when optimizing for size because extra jump insns may
-	 be created; also barrier may create extra padding.
-
-	 More correctly we should have a block reordering mode that tried to
-	 minimize the combined size of all the jumps.  This would more or less
-	 automatically remove extra jumps, but would also try to use more short
-	 jumps instead of long jumps.  */
-      && optimize_function_for_speed_p (cfun))
-    {
-      reorder_basic_blocks ();
-      cleanup_cfg (CLEANUP_EXPENSIVE);
-    }
+  reorder_basic_blocks ();
+  cleanup_cfg (CLEANUP_EXPENSIVE);
 
   FOR_EACH_BB (bb)
     if (bb->next_bb != EXIT_BLOCK_PTR)
@@ -2362,6 +2360,9 @@  gate_handle_partition_blocks (void)
      arises.  */
   return (flag_reorder_blocks_and_partition
           && optimize
+	  /* See gate_handle_reorder_blocks.  We should not partition if
+	     we are going to omit the reordering.  */
+	  && optimize_function_for_speed_p (cfun)
 	  && !DECL_ONE_ONLY (current_function_decl)
 	  && !user_defined_section_attribute);
 }