From patchwork Mon Sep 1 08:13:18 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zhenqiang Chen X-Patchwork-Id: 384702 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 570241401AB for ; Mon, 1 Sep 2014 18:13:53 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:references:in-reply-to:subject:date:message-id :mime-version:content-type:content-transfer-encoding; q=dns; s= default; b=jrvsoF1f2+VFjteTKt5HoQkHI3o7N0ShWf0vvx3s8CwcfMDgf9Xwb fMA5/6zR3CtN6VNS24L27VMsfRWEdtUSCrIlkvloDGlFVrtWQzh4BDI3inOchKdc TiFcW4qS3k7seoqxydGBt3nZ2+mkUEkpKRHiNITnprIXe06zgR3tSI= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:references:in-reply-to:subject:date:message-id :mime-version:content-type:content-transfer-encoding; s=default; bh=okGJOqeeERgTo/2bwkLTvdjO0Xo=; b=hehFgn6OCnKDRUe47zrHA+6VPkRt N75/ccS1MlVyu0BRcijBB3i2AObHTf2tlR49ppP/M1ADU4ce2yQuwk0EbMOWux6Q 0cIGn7y2At7BWkp5F2QYQ45Aen4x8DKQciinj1rKcwsIBdPyysdx3BTAUkQ+84Nl JMJ6DYLc6MMe51M= Received: (qmail 4178 invoked by alias); 1 Sep 2014 08:13:40 -0000 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 Received: (qmail 4162 invoked by uid 89); 1 Sep 2014 08:13:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: service87.mimecast.com Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 01 Sep 2014 08:13:34 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Mon, 01 Sep 2014 09:13:31 +0100 Received: from shawin003 ([10.164.2.44]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 1 Sep 2014 09:13:31 +0100 From: "Zhenqiang Chen" To: "'Jeff Law'" Cc: References: <5400E866.1020204@redhat.com> In-Reply-To: <5400E866.1020204@redhat.com> Subject: RE: [PATCH, ira] Miss checks in split_live_ranges_for_shrink_wrap Date: Mon, 1 Sep 2014 16:13:18 +0800 Message-ID: <000001cfc5bc$9876f4d0$c964de70$@arm.com> MIME-Version: 1.0 X-MC-Unique: 114090109133114001 X-IsSubscribed: yes > -----Original Message----- > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches- > owner@gcc.gnu.org] On Behalf Of Jeff Law > Sent: Saturday, August 30, 2014 4:54 AM > To: Zhenqiang Chen; gcc-patches@gcc.gnu.org > Subject: Re: [PATCH, ira] Miss checks in split_live_ranges_for_shrink_wrap > > On 08/13/14 20:55, Zhenqiang Chen wrote: > > Hi, > > > > Function split_live_ranges_for_shrink_wrap has code > > > > if (!flag_shrink_wrap) > > return false; > > > > But flag_shrink_wrap is TRUE by default when optimize > 0 even if the > > port does not support shrink-wrap. To make sure shrink-wrap is > > enabled, "HAVE_simple_return" must be defined and > "HAVE_simple_return" > > must be TRUE. > > > > Please refer function.c and shrink-wrap.c on how shrink-wrap is > > enabled in thread_prologue_and_epilogue_insns. > > > > To make the check easy, the patch defines a MICRO: > > SUPPORT_SHRINK_WRAP_P and replace the uses in ira.c and ifcvt.c > > > > Bootstrap and no make check regression on X86-64. > > > > OK for trunk? > > > > Thanks! > > -Zhenqiang > > > > ChangeLog: > > 2014-08-14 Zhenqiang Chen > > > > * shrink-wrap.h: #define SUPPORT_SHRINK_WRAP_P. > > * ira.c: #include "shrink-wrap.h" > > (split_live_ranges_for_shrink_wrap): Use SUPPORT_SHRINK_WRAP_P. > > * ifcvt.c: #include "shrink-wrap.h" > > (dead_or_predicable): Use SUPPORT_SHRINK_WRAP_P. > So what's the motivation behind this patch? I can probably guess the > motivation, but I might guess wrong. Since you know the motivation, it's > best if you just tell everyone what it is. To split live-range of register, split_live_ranges_for_shrink_wrap will introduce additional register copies. If such copies can not be optimized by later optimizations, it will lead to code size and performance regression. My tests on ARM THUMB1 code size show lots of regressions due to additional register copies. Shrink-wrap is not enabled for ARM THUMB1, so I think split_live_ranges_for_shrink_wrap should not be called. > > > > testsuite/ChangeLog: > > 2014-08-14 Zhenqiang Chen > > > > * gcc.target/arm/split-live-ranges-for-shrink-wrap.c: New test. > Testcase wasn't included in the patchkit. > > From a pure bikeshedding standpoint "SUPPORT_SHRINK_WRAP_P" seems > poorly named. SHRINK_WRAPPING_ENABLED seems like a better name to > me. > > Can you repost with the testcase included, name change and basic > rationale behind why you want to make this change. I'm pretty sure > it'll be OK at that point. Thanks. Patch is updated according to your comments. -Zhenqiang ChangeLog: 2014-09-01 Zhenqiang Chen * shrink-wrap.h: #define SHRINK_WRAPPING_ENABLED. * ira.c: #include "shrink-wrap.h" (split_live_ranges_for_shrink_wrap): Use SHRINK_WRAPPING_ENABLED. * ifcvt.c: #include "shrink-wrap.h" (dead_or_predicable): Use SHRINK_WRAPPING_ENABLED. testsuite/ChangeLog: 2014-09-01 Zhenqiang Chen * gcc.target/arm/split-live-ranges-for-shrink-wrap.c: New test. +/* { dg-final { cleanup-rtl-dump "ira" } } */ diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index 94b96f3..d2af0f9 100644 --- a/gcc/ifcvt.c +++ b/gcc/ifcvt.c @@ -42,6 +42,7 @@ #include "df.h" #include "vec.h" #include "dbgcnt.h" +#include "shrink-wrap.h" #ifndef HAVE_conditional_move #define HAVE_conditional_move 0 @@ -4287,14 +4288,13 @@ dead_or_predicable (basic_block test_bb, basic_block merge_bb, if (NONDEBUG_INSN_P (insn)) df_simulate_find_defs (insn, merge_set); -#ifdef HAVE_simple_return /* If shrink-wrapping, disable this optimization when test_bb is the first basic block and merge_bb exits. The idea is to not move code setting up a return register as that may clobber a register used to pass function parameters, which then must be saved in caller-saved regs. A caller-saved reg requires the prologue, killing a shrink-wrap opportunity. */ - if ((flag_shrink_wrap && HAVE_simple_return && !epilogue_completed) + if ((SHRINK_WRAPPING_ENABLED && !epilogue_completed) && ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb == test_bb && single_succ_p (new_dest) && single_succ (new_dest) == EXIT_BLOCK_PTR_FOR_FN (cfun) @@ -4341,7 +4341,6 @@ dead_or_predicable (basic_block test_bb, basic_block merge_bb, } BITMAP_FREE (return_regs); } -#endif } no_body: diff --git a/gcc/ira.c b/gcc/ira.c index 7c18496..f4140e4 100644 --- a/gcc/ira.c +++ b/gcc/ira.c @@ -392,6 +392,7 @@ along with GCC; see the file COPYING3. If not see #include "lra.h" #include "dce.h" #include "dbgcnt.h" +#include "shrink-wrap.h" struct target_ira default_target_ira; struct target_ira_int default_target_ira_int; @@ -4781,7 +4782,7 @@ split_live_ranges_for_shrink_wrap (void) bitmap_head need_new, reachable; vec queue; - if (!flag_shrink_wrap) + if (!SHRINK_WRAPPING_ENABLED) return false; bitmap_initialize (&need_new, 0); diff --git a/gcc/shrink-wrap.h b/gcc/shrink-wrap.h index 66bd26d..afcfec3 100644 --- a/gcc/shrink-wrap.h +++ b/gcc/shrink-wrap.h @@ -46,6 +46,9 @@ extern edge get_unconverted_simple_return (edge, bitmap_head, extern void convert_to_simple_return (edge entry_edge, edge orig_entry_edge, bitmap_head bb_flags, rtx returnjump, vec unconverted_simple_returns); +#define SHRINK_WRAPPING_ENABLED (flag_shrink_wrap && HAVE_simple_return) +#else +#define SHRINK_WRAPPING_ENABLED false #endif #endif /* GCC_SHRINK_WRAP_H */ diff --git a/gcc/testsuite/gcc.target/arm/split-live-ranges-for-shrink-wrap.c b/gcc/testsuite/gcc.target/arm/split-live-ranges-for-shrink-wrap.c new file mode 100644 index 0000000..e36000b --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/split-live-ranges-for-shrink-wrap.c @@ -0,0 +1,14 @@ +/* { dg-do assemble } */ +/* { dg-options "-mthumb -Os -fdump-rtl-ira " } */ +/* { dg-require-effective-target arm_thumb1_ok } */ + +int foo (char *, char *, int); +int test (int d, char * out, char *in, int len) +{ + if (out != in) + foo (out, in, len); + return 0; +} +/* { dg-final { object-size text <= 20 } } */ +/* { dg-final { scan-rtl-dump-not "Split live-range of register" "ira" } } */