From patchwork Thu Sep 13 11:41:41 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Bruel X-Patchwork-Id: 183616 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 6D46D2C0040 for ; Thu, 13 Sep 2012 21:42:49 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1348141370; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Message-ID:Date:From:User-Agent:MIME-Version: To:Cc:Subject:References:In-Reply-To:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=k42bzgr1SakyBJm7tSBSgJXesBQ=; b=yO8tHz65FRgQRoBJQ8rk14dFiJE6nqp4zc02wl3rZ2tyLmE6zHGBxiHbsxl1J3 N48I8IUS5FjVAK1QSkPWsHAxLc9Nbx5G7BDL/keKQP1YMnyYQJzWNTT/NXJSj/s0 xFAbdc3pXHTepkqPpRDwm4F7HB9CVVW+rdA9k1z7sls6o= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Cc:Subject:References:In-Reply-To:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=iRKrjWE+mo2OdLGjPRzFgYHsdUqWWMFUZoYJiLYOYnXY40XBMb44R4eif/+WpG EAiFR0783C45YNdZpWLG7p1H0fM/y9qd64JhtXx8hEnM38BjbGgPE09kiJV9Hd7j uhPWNn3mWICX7ud1LU+IO+GzCvXLNVnG0Ayb2+9mxdqDw=; Received: (qmail 7299 invoked by alias); 13 Sep 2012 11:42:29 -0000 Received: (qmail 7097 invoked by uid 22791); 13 Sep 2012 11:42:23 -0000 X-SWARE-Spam-Status: No, hits=-4.1 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL X-Spam-Check-By: sourceware.org Received: from eu1sys200aog109.obsmtp.com (HELO eu1sys200aog109.obsmtp.com) (207.126.144.127) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 13 Sep 2012 11:42:06 +0000 Received: from beta.dmz-eu.st.com ([164.129.1.35]) (using TLSv1) by eu1sys200aob109.postini.com ([207.126.147.11]) with SMTP ID DSNKUFHGhgtY9CEgnPMNWtorJAZ0TdcXWuZM@postini.com; Thu, 13 Sep 2012 11:42:05 UTC Received: from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id D8C17134; Thu, 13 Sep 2012 11:41:42 +0000 (GMT) Received: from Webmail-eu.st.com (safex1hubcas1.st.com [10.75.90.14]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 912842BA3; Thu, 13 Sep 2012 11:41:42 +0000 (GMT) Received: from [164.129.122.89] (164.129.122.89) by webmail-eu.st.com (10.75.90.13) with Microsoft SMTP Server (TLS) id 8.3.245.1; Thu, 13 Sep 2012 13:41:41 +0200 Message-ID: <5051C675.6060803@st.com> Date: Thu, 13 Sep 2012 13:41:41 +0200 From: Christian Bruel User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:14.0) Gecko/20120713 Thunderbird/14.0 MIME-Version: 1.0 To: Kaz Kojima Cc: "gcc-patches@gcc.gnu.org" Subject: Re: [SH] Add simple_return pattern References: <504DF050.1070800@st.com> <20120911.100526.500442496.kkojima@rr.iij4u.or.jp> In-Reply-To: <20120911.100526.500442496.kkojima@rr.iij4u.or.jp> X-IsSubscribed: yes 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 Hi Kaz, The failure turned out to be issues with the profile count and handling or region partitioning. So, I prefer to handle those separately, For now, I disable shrink-wrap when partitioning, even if the problem seems to have disappeared with the more constrained heuristics. This is probably latent also on other targets BTW. I added a sh_can_use_simple_return_p function that makes the heuristic refinements more convenient. For instance, measured that shrink-wrap is generally not good when optimizing for size because we might introduce new return instructions or split blocks to avoid the epilogue, that is still in the code somewhere anyway. Cycle-accurate benchmarks show a few very small improvements (there and there, about max 2%. accordingly, the prologue is rarely in the critical path...) but no regression. Manual assembly peering of CSiBE show that the transformation are decent. Checked with all assertions this time, Candidate for trunk. Many thanks Christian On 09/11/2012 03:05 AM, Kaz Kojima wrote: > Christian Bruel wrote: >> This patch implements the simple_return pattern to enable -fshrink-wrap >> on SH. It also clean up some redundancies for expand_epilogue (called >> twice from the "return" and "epilogue" patterns and the >> sh_expand_prologue parameter type. >> >> No regressions with sh-superh-elf and sh4-linux gcc testsuites. > > With the patch + revision 191106, I've got a new failure: > > FAIL: gcc.dg/tree-prof/bb-reorg.c compilation, -fprofile-use -D_PROFILE_USE (internal compiler error) > > for sh4-unknown-linux-gnu. My testsuite/gcc/gcc.log says > > /exp/ldroot/dodes/xsh-gcc/gcc/xgcc -B/exp/ldroot/dodes/xsh-gcc/gcc/ /exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.dg/tree-prof/bb-reorg.c -fno-diagnostics-show-caret -O2 -freorder-blocks-and-partition -fprofile-use -D_PROFILE_USE -lm -o /exp/ldroot/dodes/xsh-gcc/gcc/testsuite/gcc/bb-reorg.x02 > /exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.dg/tree-prof/bb-reorg.c: In function 'main': > /exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.dg/tree-prof/bb-reorg.c:38:1: error: EDGE_CROSSING missing across section boundary > /exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.dg/tree-prof/bb-reorg.c:38:1: internal compiler error: verify_flow_info failed > Please submit a full bug report, > > Regards, > kaz > 2012-09-12 Christian Bruel PR target/54546 * config/sh/sh-protos.h (sh_need_epilogue): Delete. (sh_can_use_simple_return_p): Declare. * config/sh/sh.c (sh_can_use_simple_return_p): Define. (sh_need_epilogue, sh_need_epilogue_known): Delete. (sh_output_function_epilogue): Remove sh_need_epilogue_known. * config/sh/sh.md (simple_return, return): Define. (epilogue): Use inline return rtl. (sh_expand_epilogue): Cleanup parameters boolean type. * config/sh/iterators.md (any_return): New iterator. Index: config/sh/sh-protos.h =================================================================== --- config/sh/sh-protos.h (revision 191129) +++ config/sh/sh-protos.h (working copy) @@ -117,7 +117,6 @@ extern rtx get_fpscr_rtx (void); extern int sh_media_register_for_return (void); extern void sh_expand_prologue (void); extern void sh_expand_epilogue (bool); -extern bool sh_need_epilogue (void); extern void sh_set_return_address (rtx, rtx); extern int initial_elimination_offset (int, int); extern bool fldi_ok (void); @@ -155,4 +154,5 @@ extern int sh2a_get_function_vector_number (rtx); extern bool sh2a_is_function_vector_call (rtx); extern void sh_fix_range (const char *); extern bool sh_hard_regno_mode_ok (unsigned int, enum machine_mode); +extern bool sh_can_use_simple_return_p (void); #endif /* ! GCC_SH_PROTOS_H */ Index: config/sh/sh.c =================================================================== --- config/sh/sh.c (revision 191129) +++ config/sh/sh.c (working copy) @@ -7899,24 +7899,6 @@ sh_expand_epilogue (bool sibcall_p) emit_use (gen_rtx_REG (SImode, PR_REG)); } -static int sh_need_epilogue_known = 0; - -bool -sh_need_epilogue (void) -{ - if (! sh_need_epilogue_known) - { - rtx epilogue; - - start_sequence (); - sh_expand_epilogue (0); - epilogue = get_insns (); - end_sequence (); - sh_need_epilogue_known = (epilogue == NULL ? -1 : 1); - } - return sh_need_epilogue_known > 0; -} - /* Emit code to change the current function's return address to RA. TEMP is available as a scratch register, if needed. */ @@ -7996,7 +7978,6 @@ static void sh_output_function_epilogue (FILE *file ATTRIBUTE_UNUSED, HOST_WIDE_INT size ATTRIBUTE_UNUSED) { - sh_need_epilogue_known = 0; } static rtx @@ -12959,4 +12940,34 @@ sh_init_sync_libfuncs (void) init_sync_libfuncs (UNITS_PER_WORD); } +/* Return true if it is appropriate to emit `ret' instructions in the + body of a function. */ + +bool +sh_can_use_simple_return_p (void) +{ + HARD_REG_SET live_regs_mask; + int d; + + if (! reload_completed || frame_pointer_needed) + return false; + + /* Moving prologue around does't reduce the size. */ + if (optimize_function_for_size_p (cfun)) + return false; + + /* Can't optimize CROSSING_JUMPS for now. */ + if (flag_reorder_blocks_and_partition) + return false; + + /* Finally, allow for pr save. */ + d = calc_live_regs (&live_regs_mask); + + if (rounded_frame_size (d) > 4) + return false; + + return true; + +} + #include "gt-sh.h" Index: config/sh/iterators.md =================================================================== --- config/sh/iterators.md (revision 191129) +++ config/sh/iterators.md (working copy) @@ -34,3 +34,6 @@ (define_mode_attr disp04 [(QI "K04") (HI "K05")]) (define_mode_attr disp12 [(QI "K12") (HI "K13")]) +;; Return codes. +(define_code_iterator any_return [return simple_return]) + Index: config/sh/sh.md =================================================================== --- config/sh/sh.md (revision 191129) +++ config/sh/sh.md (working copy) @@ -9280,7 +9280,7 @@ label: [(return)] "" { - sh_expand_epilogue (1); + sh_expand_epilogue (true); if (TARGET_SHCOMPACT) { rtx insn, set; @@ -10099,9 +10099,13 @@ label: } [(set_attr "type" "load_media")]) +(define_expand "simple_return" + [(simple_return)] + "sh_can_use_simple_return_p ()") + (define_expand "return" [(return)] - "reload_completed && ! sh_need_epilogue ()" + "reload_completed && epilogue_completed" { if (TARGET_SHMEDIA) { @@ -10117,8 +10121,8 @@ label: } }) -(define_insn "*return_i" - [(return)] +(define_insn "*_i" + [(any_return)] "TARGET_SH1 && ! (TARGET_SHCOMPACT && (crtl->args.info.call_cookie & CALL_COOKIE_RET_TRAMP (1))) @@ -10244,19 +10248,12 @@ label: (define_expand "prologue" [(const_int 0)] "" -{ - sh_expand_prologue (); - DONE; -}) + "sh_expand_prologue (); DONE;") (define_expand "epilogue" [(return)] "" -{ - sh_expand_epilogue (0); - emit_jump_insn (gen_return ()); - DONE; -}) + "sh_expand_epilogue (false);") (define_expand "eh_return" [(use (match_operand 0 "register_operand" ""))]