From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12590 invoked by alias); 31 Oct 2016 15:35:19 -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 12579 invoked by uid 89); 31 Oct 2016 15:35:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.4 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=Hx-languages-length:2003, H*r:8.13.8 X-HELO: gate.crashing.org Received: from gate.crashing.org (HELO gate.crashing.org) (63.228.1.57) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 31 Oct 2016 15:35:18 +0000 Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.13.8) with ESMTP id u9VFZBVG012845; Mon, 31 Oct 2016 10:35:11 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id u9VFZAnQ012813; Mon, 31 Oct 2016 10:35:10 -0500 Date: Mon, 31 Oct 2016 15:35:00 -0000 From: Segher Boessenkool To: Steven Bosscher Cc: GCC Patches Subject: Re: [PATCH] bb-reorder: Improve compgotos pass (PR71785) Message-ID: <20161031153504.GA3328@gate.crashing.org> References: <3d8d886a7e9d7bcf5bee9867e2f4849a210fd976.1477843149.git.segher@kernel.crashing.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg02490.txt.bz2 On Mon, Oct 31, 2016 at 04:09:48PM +0100, Steven Bosscher wrote: > On Sun, Oct 30, 2016 at 8:10 PM, Segher Boessenkool wrote: > > This patch solves this problem by simply running the duplicate_computed_gotos > > pass again, as long as it does any work. The patch looks much bigger than > > it is, because I factored out two routines to simplify the control flow. > > It's made the patch a bit difficult to read. Condensing it a bit... Well, it would have a goto crossing a loop, or two gotos crossing each other, otherwise. I should have done it as two patches I guess (first factor, then change). > > + for (;;) > > { > > + if (n_basic_blocks_for_fn (fun) <= NUM_FIXED_BLOCKS + 1) > > + return 0; > > This test should not be needed in the loop. This pass can never > collapse the function to a single basic block. Yeah maybe, but that relies on quite a few assumptions. This is strictly an optimisation anyway, will move it outside the loop. > > + basic_block bb; > > + FOR_EACH_BB_FN (bb, fun) > > + { > > + /* Build the reorder chain for the original order of blocks. */ > > + if (bb->next_bb != EXIT_BLOCK_PTR_FOR_FN (fun)) > > + bb->aux = bb->next_bb; > > + } > > > > + duplicate_computed_gotos_find_candidates (fun, candidates, max_size); > > > > + bool changed = false; > > + if (!bitmap_empty_p (candidates)) > > + changed = duplicate_computed_gotos_do_duplicate (fun, candidates); > > > > + if (changed) > > + fixup_partitions (); > > + > > + cfg_layout_finalize (); > > I don't think you have to go into/out-of cfglayout mode for each iteration. Yeah probably. I was too lazy :-) It needs the cleanup_cfg every iteration though, I was afraid that interacts. > > /* Merge the duplicated blocks into predecessors, when possible. */ > > + if (changed) > > + cleanup_cfg (0); > > + else > > + break; > > } > > Maybe a gcc_assert that the loop doesn't iterate more often than num_edges? Good plan (num blocks even). Thanks, Segher