From patchwork Tue May 20 10:56:26 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 350647 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 8AD2C140081 for ; Tue, 20 May 2014 20:56:40 +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 :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=WD+raOWS3yuoTTGiMF MMifAkwzr5QRjRVQ9SevZLbQc3BAWyvxHH/6DUBigoroV1muSdvOWsgmq/AGks7Z T8MjxKPxibJQjBbbvzGLbxkSGXQUaDmOxoQo6uBQRtLZBCLCvywVTBUxiSbUbXJQ WyJK+r0LxgvyzOE75s5v2WY6I= 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 :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=97Ax4TFb64VdqVoNI64sa8wb 04U=; b=nriyGw9oB4GTB5S7HMAG6cL72PjQ6LmlVo1ZatK2BBsh5AF08rnULL99 R5u+QL04wHoCeliGg6yZfMqYyyePUDw7pqyY8WBCav4cleJb9qxFVekB9c6nK3e0 ZiTHwJwI9kxPvucYuM5k/VZ4RL7n9PHNF81gKjq4O5mN2LrZ+Y4= Received: (qmail 9492 invoked by alias); 20 May 2014 10:56:33 -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 9481 invoked by uid 89); 20 May 2014 10:56:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-we0-f171.google.com Received: from mail-we0-f171.google.com (HELO mail-we0-f171.google.com) (74.125.82.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 20 May 2014 10:56:30 +0000 Received: by mail-we0-f171.google.com with SMTP id w62so319590wes.2 for ; Tue, 20 May 2014 03:56:27 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.194.57.225 with SMTP id l1mr36600808wjq.25.1400583387125; Tue, 20 May 2014 03:56:27 -0700 (PDT) Received: by 10.194.119.193 with HTTP; Tue, 20 May 2014 03:56:26 -0700 (PDT) In-Reply-To: <20140520042958.GD8734@kam.mff.cuni.cz> References: <20140516172559.GF20755@kam.mff.cuni.cz> <53791418.3060501@codesourcery.com> <20140518205956.GG1828@kam.mff.cuni.cz> <53797044.2050704@codesourcery.com> <537AD329.7070502@codesourcery.com> <20140520042958.GD8734@kam.mff.cuni.cz> Date: Tue, 20 May 2014 12:56:26 +0200 Message-ID: Subject: Re: Eliminate write-only variables From: Richard Biener To: Jan Hubicka Cc: Sandra Loosemore , GCC Patches , Jakub Jelinek X-IsSubscribed: yes On Tue, May 20, 2014 at 6:29 AM, Jan Hubicka wrote: >> On 05/18/2014 08:45 PM, Sandra Loosemore wrote: >> >On 05/18/2014 02:59 PM, Jan Hubicka wrote: >> >>For cases like local-statics-7 your approach can be "saved" by adding >> >>simple IPA analysis >> >>to look for static vars that are used only by one function and keeping >> >>your DSE code active >> >>for them, so we can still get rid of this special case assignments >> >>during late compilation. >> >>I am however not quite convinced it is worth the effort - do you have >> >>some real world >> >>cases where it helps? >> > >> >Um, the well-known benchmark. ;-) >> >> I looked at the generated code for this benchmark and see that your >> patch is indeed not getting rid of all the pointless static >> variables, while ours is, so this is quite disappointing. I'm >> thinking now that we will still need to retain our patch at least >> locally, although we'd really like to get it on trunk if possible. > > Yours patch can really be improved by adding simple IPA analysis to work > out what variables are refernced by one function only instead of using > not-longer-that-relevant local static info. > As last IPA pass for each static variable with no address taken, look at all > references and see if they all come from one function or functions inlined to > it. >> >> Here is another testcase vaguely based on the same kind of >> signal-processing algorithm as the benchmark, but coded without >> reference to it. I have arm-none-eabi builds around for both 4.9.0 >> with our remove_local_statics patch applied, and trunk with your >> patch. With -O2, our optimization produces 23 instructions and gets >> rid of all 3 statics, yours produces 33 and only gets rid of 1 of >> them. >> >> Of course it's lame to use static variables in this way, but OTOH >> it's lame if GCC can't recognize them and optimize them away, too. >> >> -Sandra >> > >> void >> fir (int *coeffs, int coeff_length, int *input, int input_length, int *output) >> { >> static int *coeffp; >> static int *inputp; >> static int *outputp; > > Here inputp read is rmeoved only at 101.dceloop1 pass. That is really late. > coeffp is removed late, too. > >> int i, c, acc; >> >> for (i = 0; i < input_length; i++) >> { >> coeffp = coeffs; >> inputp = input + i; >> outputp = output + i; >> acc = 0; >> for (c = 0; c < coeff_length; c++) >> { >> acc += *coeffp * *inputp; >> coeffp++; >> inputp--; >> } > > It seems like AA can not work out the fact that inputp is unchanged until that > late. Richi, any ideas? Well, AA is perfectly fine - it's just that this boils down to a partial redundancy problem. The stores can be removed by DCE even with but I don't have a good way of checking the ??? prerequesite (even without taking its address the statics may be refered to by a) inline copies, b) ipa-split function parts, c) nested functions). I'm sure IPA references are not kept up-to-date. The last reads go away with PRE at the expense of two additional induction variables. If we know that a static variable does not have its address taken and is only refered to from a single function we can in theory simply rewrite it into SSA form during early opts (before inlining its body), for example by SRA. That isn't even restricted to function-local statics (for statics used in multiple functions but not having address-taken we can do the same with doing function entry / exit load / store). I think that would be a much better IPA enabled optimization than playing games with looking at individual stmts. Richard. > Honza >> *outputp = acc; >> } >> } >> >> > Index: gcc/tree-ssa-dce.c =================================================================== --- gcc/tree-ssa-dce.c (revision 210635) +++ gcc/tree-ssa-dce.c (working copy) @@ -278,10 +278,20 @@ mark_stmt_if_obviously_necessary (gimple break; case GIMPLE_ASSIGN: - if (TREE_CODE (gimple_assign_lhs (stmt)) == SSA_NAME - && TREE_CLOBBER_P (gimple_assign_rhs1 (stmt))) - return; - break; + { + tree lhs = gimple_assign_lhs (stmt); + if (TREE_CODE (lhs) == SSA_NAME + && TREE_CLOBBER_P (gimple_assign_rhs1 (stmt))) + return; + if (TREE_CODE (lhs) == VAR_DECL + && !TREE_ADDRESSABLE (lhs) + && TREE_STATIC (lhs) + && !TREE_PUBLIC (lhs) && !DECL_EXTERNAL (lhs) + /* ??? Make sure lhs is only refered to from cfun->decl. */ + && decl_function_context (lhs) == cfun->decl) + return; + break; + } default: break;