From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103178 invoked by alias); 18 May 2018 07:28:02 -0000 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 Received: (qmail 103168 invoked by uid 89); 18 May 2018 07:28:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=HX-Received:Fri X-HELO: mail-lf0-f66.google.com Received: from mail-lf0-f66.google.com (HELO mail-lf0-f66.google.com) (209.85.215.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 18 May 2018 07:27:59 +0000 Received: by mail-lf0-f66.google.com with SMTP id r2-v6so12481325lff.4 for ; Fri, 18 May 2018 00:27:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=FHpEOjApakbj9VnkHchgEA1O8v3bE/Aw9eZsN3I0yFc=; b=TwEbMu5Did2oARfcodqHzG3EQRNxQJmVVoZvUvRBbChTPBSv0qOmGhlBIDvqYBi6AA lVARWXP96cnGbErZYKceOq+aOJbBTA+FAFUH74RHyRnhODETI0bcdIxJZ/B1lBqcXGtX lDBdl7dbTjoY65iyMs/Oy/UBXfeVphNc+f/NhfwcQ5pA9fe88pOcMdRouKhO+z1OFmcs 6Lyi72OKOUu45o2jORcnM98npPNr9R2/bcn1fXsZLbYRfZh/c2jrOWBSI36Ew7TA5ywl 1Xw67XnfTbyuvMtV/w/9cYK9izMWhI5G/gLjxDxmqY92WwNrUCntFYGVjPSsTWtIFi1c x7hQ== X-Gm-Message-State: ALKqPwc8FTAEs0bQ4/S2EM4w6tOA95LbDx5r4mZdiNVCs3xlyoJ+svM5 fxZj+rhyIRAqRZQQhLxlEbYx7+siP2mm19YUsq4= X-Google-Smtp-Source: AB8JxZqEXjQ4sF2EqsYG5/z6iOz8dihQEvDqUkaveh3ToRtJmdxFwf8JqafNFcUh2BcvNLRPSAkZm26Yq0Bxa71GOlk= X-Received: by 2002:a2e:634e:: with SMTP id x75-v6mr5149152ljb.140.1526628476985; Fri, 18 May 2018 00:27:56 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Fri, 18 May 2018 07:29:00 -0000 Message-ID: Subject: Re: [PATCH GCC][4/6]Support regional coalesce and live range computation To: "Amker.Cheng" Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-05/txt/msg00872.txt.bz2 On Thu, May 17, 2018 at 5:49 PM Bin.Cheng wrote: > On Thu, May 17, 2018 at 3:04 PM, Richard Biener > wrote: > > On Tue, May 15, 2018 at 5:44 PM Bin.Cheng wrote: > > > >> On Fri, May 11, 2018 at 1:53 PM, Richard Biener > >> wrote: > >> > On Fri, May 4, 2018 at 6:23 PM, Bin Cheng wrote: > >> >> Hi, > >> >> Following Jeff's suggestion, I am now using existing tree-ssa-live.c > > and > >> >> tree-ssa-coalesce.c to compute register pressure, rather than inventing > >> >> another live range solver. > >> >> > >> >> The major change is to record region's basic blocks in var_map and use > > that > >> >> information in computation, rather than FOR_EACH_BB_FN. For now only > > loop > >> >> and function type regions are supported. The default one is function > > type > >> >> region which is used in out-of-ssa. Loop type region will be used in > > next > >> >> patch to compute information for a loop. > >> >> > >> >> Bootstrap and test on x86_64 and AArch64 ongoing. Any comments? > >> > > >> > I believe your changes to create_outofssa_var_map should be done > > differently > >> > by simply only calling it from the coalescing context and passing in the > >> > var_map rather than initializing it therein and returning it. > >> > > >> > This also means the coalesce_vars_p flag in the var_map structure looks > >> > somewhat out-of-place. That is, it looks you could do with many less > >> > changes if you refactored what calls what slightly? For example > >> > the extra arg to gimple_can_coalesce_p looks unneeded. > >> > > >> > Just as a note I do have a CFG helper pending that computes RPO order > >> > for SEME regions (attached). loops are SEME regions, so your RTYPE_SESE > >> > is somewhat odd - I guess RTYPE_LOOP exists only because of the > >> > convenience of passing in a loop * to the "constructor". I'd rather > >> > drop this region_type thing and always assume a SEME region - at least > >> > I didn't see anything in the patch that depends on any of the forms > >> > apart from the initial BB gathering. > > > >> Hi Richard, > > > >> Thanks for reviewing. I refactored tree-ssa-live.c and > >> tree-ssa-coalesce.c following your comments. > >> Basically I did following changes: > >> 1) Remove region_type and only support loop region live range computation. > >> Also I added one boolean field in var_map indicating whether we > >> are computing > >> loop region live range or out-of-ssa. > >> 2) Refactored create_outofssa_var_map into > > create_coalesce_list_for_region and > >> populate_coalesce_list_for_outofssa. Actually the original > >> function name doesn't > >> quite make sense because it has nothing to do with var_map. > >> 3) Hoist init_var_map up in call stack. Now it's called by caller > >> (out-of-ssa or live range) > >> and the returned var_map is passed to coalesce_* stuff. > >> 4) Move global functions to tree-outof-ssa.c and make them static. > >> 5) Save/restore flag_tree_coalesce_vars in order to avoid updating > >> checks on the flag. > > > >> So how is this one? Patch attached. > > > > A lot better. Few things I noticed: > > > > + map->bmp_bbs = BITMAP_ALLOC (NULL); > > > > use a bitmap_head member and bitmap_initialize (). > > > > + map->vec_bbs = new vec (); > > > > use a vec<> member and map->vec_bbs = vNULL; > > > > Both changes remove an unnecessary indirection. > > > > + map->outofssa_p = true; > > + basic_block bb; > > + FOR_EACH_BB_FN (bb, cfun) > > + { > > + bitmap_set_bit (map->bmp_bbs, bb->index); > > + map->vec_bbs->safe_push (bb); > > + } > > > > I think you can avoid populating the bitmap and return > > true unconditionally for outofssa_p in the contains function? > > Ah, you already do - so why populate the bitmap? > > > > +/* Return TRUE if region of the MAP contains basic block BB. */ > > + > > +inline bool > > +region_contains_p (var_map map, basic_block bb) > > +{ > > + if (bb == ENTRY_BLOCK_PTR_FOR_FN (cfun) > > + || bb == EXIT_BLOCK_PTR_FOR_FN (cfun)) > > + return false; > > + > > + if (map->outofssa_p) > > + return true; > > + > > + return bitmap_bit_p (map->bmp_bbs, bb->index); > > > > the entry/exit block check should be conditional in map->outofssa_p > > but I think we should never get the function called with those args > > so we can as well use a gcc_checking_assert ()? > > > > I think as followup we should try to get a BB order that > > is more suited for the dataflow problem. Btw, I was > > thinking about adding anoter "visited" BB flag that is guaranteed to > > be unset and free to be used by infrastructure. So the bitmap > > could be elided for a bb flag check (but we need to clear that flag > > at the end of processing). Not sure if it's worth to add a machinery > > to dynamically assign pass-specific flags... it would at least be > > less error prone. Sth to think about. > > > > So -- I think the patch is ok with the two indirections removed, > > the rest can be optimized as followup. > Hi, > This is the updated patch. I moved checks on ENTRY/EXIT blocks under > outofssa_p, > as well as changed vec_bbs into object. Note bmp_bbs is kept in > pointer so that we > can avoid allocating memory in out-of-ssa case. > Bootstrap and test ongoing. Is it OK? Yes. Thanks, Richard. > Thanks, > bin > > > > Thanks, > > Richard. > > > > > >> Thanks, > >> bin > >> 2018-05-15 Bin Cheng > > > >> * tree-outof-ssa.c (tree-ssa.h, tree-dfa.h): Include header files. > >> (create_default_def, for_all_parms): Moved from tree-ssa-coalesce.c. > >> (parm_default_def_partition_arg): Ditto. > >> (set_parm_default_def_partition): Ditto. > >> (get_parm_default_def_partitions): Ditto and make it static. > >> (get_undefined_value_partitions): Ditto and make it static. > >> (remove_ssa_form): Refactor call to init_var_map here. > >> * tree-ssa-coalesce.c (build_ssa_conflict_graph): Support live range > >> computation for loop region. > >> (coalesce_partitions, compute_optimized_partition_bases): Ditto. > >> (register_default_def): Delete. > >> (for_all_parms, create_default_def): Move to tree-outof-ssa.c. > >> (parm_default_def_partition_arg): Ditto. > >> (set_parm_default_def_partition): Ditto. > >> (get_parm_default_def_partitions): Ditto and make it static. > >> (get_undefined_value_partitions): Ditto and make it static. > >> (coalesce_with_default, coalesce_with_default): Update comment. > >> (create_coalesce_list_for_region): New func factored out from > >> create_outofssa_var_map. > >> (populate_coalesce_list_for_outofssa): New func factored out from > >> create_outofssa_var_map and coalesce_ssa_name. > >> (create_outofssa_var_map): Delete. > >> (coalesce_ssa_name): Refactor to support live range computation. > >> * tree-ssa-coalesce.h (coalesce_ssa_name): Change decl. > >> (get_parm_default_def_partitions): Delete. > >> (get_undefined_value_partitions): Ditto. > >> * tree-ssa-live.c (init_var_map, delete_var_map): Support live range > >> computation for loop region. > >> (new_tree_live_info, loe_visit_block): Ditto. > >> (live_worklist, set_var_live_on_entry): Ditto. > >> (calculate_live_on_exit, verify_live_on_entry): Ditto. > >> * tree-ssa-live.h (struct _var_map): New fields. > >> (init_var_map): Change decl. > >> (region_contains_p): New.