From patchwork Wed Aug 31 21:41:04 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Schmidt X-Patchwork-Id: 112692 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 98CFAB6F70 for ; Thu, 1 Sep 2011 07:43:11 +1000 (EST) Received: (qmail 17631 invoked by alias); 31 Aug 2011 21:43:10 -0000 Received: (qmail 17623 invoked by uid 22791); 31 Aug 2011 21:43:09 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, TW_CF X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 31 Aug 2011 21:42:55 +0000 Received: (qmail 17118 invoked from network); 31 Aug 2011 21:42:54 -0000 Received: from unknown (HELO ?84.152.169.2?) (bernds@127.0.0.2) by mail.codesourcery.com with ESMTPA; 31 Aug 2011 21:42:54 -0000 Message-ID: <4E5EAA70.8000107@codesourcery.com> Date: Wed, 31 Aug 2011 23:41:04 +0200 From: Bernd Schmidt User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.18) Gecko/20110801 Lightning/1.0b3pre Thunderbird/3.1.11 MIME-Version: 1.0 To: Jeff Law CC: GCC Patches Subject: Re: bb partitioning vs optimize_function_for_speed_p References: <4E57B20F.7000307@codesourcery.com> <4E5BB7FF.7060609@redhat.com> In-Reply-To: <4E5BB7FF.7060609@redhat.com> Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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. 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); }