From patchwork Fri Feb 18 03:58:37 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jie Zhang X-Patchwork-Id: 83531 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 C6E78B70AA for ; Fri, 18 Feb 2011 14:58:51 +1100 (EST) Received: (qmail 6113 invoked by alias); 18 Feb 2011 03:58:50 -0000 Received: (qmail 6105 invoked by uid 22791); 18 Feb 2011 03:58:49 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, TW_QE, T_RP_MATCHES_RCVD 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; Fri, 18 Feb 2011 03:58:45 +0000 Received: (qmail 13588 invoked from network); 18 Feb 2011 03:58:42 -0000 Received: from unknown (HELO ?117.131.21.126?) (jie@127.0.0.2) by mail.codesourcery.com with ESMTPA; 18 Feb 2011 03:58:42 -0000 Message-ID: <4D5DEE6D.4040709@codesourcery.com> Date: Fri, 18 Feb 2011 11:58:37 +0800 From: Jie Zhang User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101213 Lightning/1.0b2 Icedove/3.1.7 MIME-Version: 1.0 To: Jeff Law CC: gcc-patches@gcc.gnu.org, Alexandre Oliva Subject: Re: Ignore clobber def when replacing register in WEB pass (was Re: Fix a bug in merging uninitialized refs into a single web) References: <4D245036.4040807@codesourcery.com> <4D24B3E1.1040500@redhat.com> <4D252724.7050200@codesourcery.com> <4D4ADB82.4070107@redhat.com> <4D4D3D16.5090405@codesourcery.com> <4D537233.70409@codesourcery.com> <4D5A9DA2.2030908@redhat.com> <4D5B7BAE.4070108@codesourcery.com> In-Reply-To: <4D5B7BAE.4070108@codesourcery.com> 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 On 02/16/2011 03:24 PM, Jie Zhang wrote: > On 02/15/2011 11:37 PM, Jeff Law wrote: >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 >> >> On 02/09/11 22:05, Jie Zhang wrote: >>> Hi Jeff, >>> >>> On 02/05/2011 08:05 PM, Jie Zhang wrote: >>>>>> Thanks for review. Yeah. I thought about ignoring the clobber at >>>>>> first. >>>>>> But later I found there was a bug in the code which merges >>>>>> uninitialized >>>>>> refs into a single web and fixing that bug should also fix the >>>>>> issue I >>>>>> encountered. So I just try to fix that bug which will be safer and >>>>>> easier for me. >>>>> So just so I'm certain I understand the problem. In the original >>>>> testcase a naked CLOBBER was the "set" that triggered the problem, but >>>>> this problem can occur for assignments to any uninitialized pseudo, >>>>> such >>>>> as in examples you provided below. >>>>> >>>>> When we see a set to an uninitialized pseudo, we're losing the saved >>>>> DF_REF_UID which allows us to combine all the uninitialized uses >>>>> into a >>>>> single web. Right? >>>>> >>>> Yes. My patch just prevents this losing. >>>> >>> While looking into PR 47622, which was caused by my patch, I found my >>> patch was wrong. For example >>> >>> reg 134<-- is uninitialized >>> use (reg 134)<-- use1 >>> set (reg 134)<-- def2 >>> use (reg 134)<-- use2 >>> >>> use1 forms a web, def2 and use2 form another web. In such case, we still >>> want (reg 134) in the second web to be renamed. But with my patch, it >>> will not. That bug I found in Alex's patch is *not* real and I have >>> reverted my patch. Sorry for the huge noise I caused. >>> >>> A new patch is attached, which just follows Jeff's suggestion to ignore >>> clobber in the web pass. Testing is still going on. Is it OK if the >>> testing is good? >> Is there some reason you don't just do something like >> >> if (NONDEBUG_INSN_P (insn)&& GET_CODE (insn) != CLOBBER) >> >> In the outer conditional? >> >> That's more typical of how I ignore naked clobbers. Otherwise don't you >> run the risk of ignoring a clobber which appears inside a normal insn? >> > I don't know why we should not ignore a clobber inside a normal insn. I > wrote the patch in that way was trying to ignore a clobber inside a > normal insn. > Anyway I have also tested the attached patch as your advice. Testing arm-none-linux-gnueabi on qemu shows no regressions. Also bootstrapped and regression tested natively on x86_64-unknown-linux-gnu. If you like this one better, I do not object. It seems safer than the previous one and also fixes the issue I concern. I also reported a new PR and use that PR for the test case file name. Regards, * web.c (web_main): Ignore naked clobber when replacing register. testsuite/ * gcc.dg/pr47763.c: New test. Index: testsuite/gcc.dg/pr47763.c =================================================================== --- testsuite/gcc.dg/pr47763.c (revision 0) +++ testsuite/gcc.dg/pr47763.c (revision 0) @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -funroll-loops -fdump-rtl-web" } */ + +foo() +{ +} + +/* { dg-final { scan-rtl-dump-not "Web oldreg" "web" } } */ +/* { dg-final { cleanup-rtl-dump "web" } } */ Index: web.c =================================================================== --- web.c (revision 169997) +++ web.c (working copy) @@ -377,7 +377,17 @@ web_main (void) FOR_BB_INSNS (bb, insn) { unsigned int uid = INSN_UID (insn); - if (NONDEBUG_INSN_P (insn)) + + if (NONDEBUG_INSN_P (insn) + /* Ignore naked clobber. For example, reg 134 in the second insn + of the following sequence will not be replaced. + + (insn (clobber (reg:SI 134))) + + (insn (set (reg:SI 0 r0) (reg:SI 134))) + + Thus the later passes can optimize them away. */ + && GET_CODE (PATTERN (insn)) != CLOBBER) { df_ref *use_rec; df_ref *def_rec;