From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21807 invoked by alias); 11 Oct 2012 23:04:09 -0000 Received: (qmail 21793 invoked by uid 22791); 11 Oct 2012 23:04:06 -0000 X-SWARE-Spam-Status: No, hits=-5.1 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-lb0-f175.google.com (HELO mail-lb0-f175.google.com) (209.85.217.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 11 Oct 2012 23:03:56 +0000 Received: by mail-lb0-f175.google.com with SMTP id y2so1716309lbk.20 for ; Thu, 11 Oct 2012 16:03:55 -0700 (PDT) Received: by 10.112.8.133 with SMTP id r5mr973600lba.63.1349996635004; Thu, 11 Oct 2012 16:03:55 -0700 (PDT) MIME-Version: 1.0 Received: by 10.112.42.5 with HTTP; Thu, 11 Oct 2012 16:03:34 -0700 (PDT) In-Reply-To: <50766d69.a853420a.2475.fffffbafSMTPIN_ADDED@mx.google.com> References: <50655073.e54c420a.651e.ffffac0fSMTPIN_ADDED@mx.google.com> <506AE4F1.5030807@redhat.com> <50766d69.a853420a.2475.fffffbafSMTPIN_ADDED@mx.google.com> From: Steven Bosscher Date: Thu, 11 Oct 2012 23:19:00 -0000 Message-ID: Subject: Re: [PATCH RFA] Implement register pressure directed hoist pass To: Bin Cheng Cc: Jeff Law , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-10/txt/msg01170.txt.bz2 On Thu, Oct 11, 2012 at 8:50 AM, Bin Cheng wrote: > + /* x+y won't be hoisted without defaultly enabled "-fira-hoist-pressure", defaulty comment. > + kinds of code motion(including code hoisting) in a unified way. needs space between "motion" and "(". > + flow graph, given if it can reach BB unimpared. Stop the search if the s/given if/if/ > +should_hoist_expr_to_dom (basic_block expr_bb, struct expr *expr, > + basic_block bb, sbitmap visited, int distance, > + int *bb_size, enum reg_class pressure_class, > + int *nregs, bitmap hoisted_bbs) At least BB_SIZE and DISTANCE are not documented (also not before your patch). While there, can you document these please? > + becasue hoisting constant expr aggresively results in worse code > + as observed. */ s/becasue/because/ s/aggresively/aggressively/ Not sure what you mean with "as observed". Observed where/how? > - visited = XCNEWVEC (char, last_basic_block); > + visited = sbitmap_alloc (last_basic_block); > + sbitmap_zero (visited); Send a separate patch for this change please. Really. Reviewing patches is much easier if you post things one change at a time. > + The code hoisting pass hoists multiple computations of expression > + to dominator basic block, like from b2/b3 to b1 as depicted below: "The code hoisting pass can hoist multiple computations of the same expression along dominated paths to a dominating basic block, like from ..." > + Unfortunately code hoisting generally extend live range of output s/extend/extends the/ s/of output/of an output/ > + from rtx cost of corresponding expression and it's used to control s/of corresponding/of the corresponding/ Similarly elsewhere in comments. > + of shrinked live range of input pseudo register when hoisting an s/range/ranges/ s/register/registers/ After all this is only possible, AFAICT, if there's more than one input register. I'll leave all the algorithmic stuff to Jeff... Ciao! Steven