From patchwork Wed Jun 23 20:22:08 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxim Kuvyrkov X-Patchwork-Id: 56711 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 911DFB6F16 for ; Thu, 24 Jun 2010 06:22:28 +1000 (EST) Received: (qmail 18678 invoked by alias); 23 Jun 2010 20:22:25 -0000 Received: (qmail 18543 invoked by uid 22791); 23 Jun 2010 20:22:22 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, 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; Wed, 23 Jun 2010 20:22:12 +0000 Received: (qmail 11637 invoked from network); 23 Jun 2010 20:22:10 -0000 Received: from unknown (HELO ?172.16.1.24?) (maxim@127.0.0.2) by mail.codesourcery.com with ESMTPA; 23 Jun 2010 20:22:10 -0000 Message-ID: <4C226CF0.1060408@codesourcery.com> Date: Thu, 24 Jun 2010 00:22:08 +0400 From: Maxim Kuvyrkov User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.10) Gecko/20100512 Thunderbird/3.0.5 MIME-Version: 1.0 To: Jeff Law CC: gcc-patches Subject: Re: 0007-Add-open-ended-comments.patch References: <4C18F225.2040509@codesourcery.com> <4C18F4F4.606@codesourcery.com> <4C190767.7070204@redhat.com> In-Reply-To: <4C190767.7070204@redhat.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 6/16/10 9:18 PM, Jeff Law wrote: >> This patch adds several open-ended comments in gcse.c. I'll be happy >> if anyone can answer some of them, in which case I'll check in the >> answers, rather than questions :). >> >> Thank you, >> > @@ -3431,7 +3431,9 @@ process_insert_insn (struct expr *expr) > > For PRE, we want to verify that the expr is either transparent > or locally anticipatable in the target block. This check makes > - no sense for code hoisting. */ > + no sense for code hoisting. > + ??? We always call this function with (PRE == 0), which makes the checks > + useless. */ > See pre_edge_insert and search for EDGE_ABNORMAL. > > That code went through several iterations and may no longer be > necessary. So we really should extend the existing comment before the > call to insert_insn_end_basic_block from pre_edge_insert. I tried making pre_edge_insert to call insert_insn_end_basic_block with (pre == 1), and that failed with segmentation fault due to antloc being NULL. Anyway, the attached patch removes unused checks. OK to check in? diff --git a/gcc/gcse.c b/gcc/gcse.c index ff8dbc2..b95be91 100644 --- a/gcc/gcse.c +++ b/gcc/gcse.c @@ -491,7 +491,7 @@ static void free_pre_mem (void); static void compute_pre_data (void); static int pre_expr_reaches_here_p (basic_block, struct expr *, basic_block); -static void insert_insn_end_basic_block (struct expr *, basic_block, int); +static void insert_insn_end_basic_block (struct expr *, basic_block); static void pre_insert_copy_insn (struct expr *, rtx); static void pre_insert_copies (void); static int pre_delete (void); @@ -3453,14 +3453,10 @@ process_insert_insn (struct expr *expr) /* Add EXPR to the end of basic block BB. - This is used by both the PRE and code hoisting. - - For PRE, we want to verify that the expr is either transparent - or locally anticipatable in the target block. This check makes - no sense for code hoisting. */ + This is used by both the PRE and code hoisting. */ static void -insert_insn_end_basic_block (struct expr *expr, basic_block bb, int pre) +insert_insn_end_basic_block (struct expr *expr, basic_block bb) { rtx insn = BB_END (bb); rtx new_insn; @@ -3487,12 +3483,6 @@ insert_insn_end_basic_block (struct expr *expr, basic_block bb, int pre) #ifdef HAVE_cc0 rtx note; #endif - /* It should always be the case that we can put these instructions - anywhere in the basic block with performing PRE optimizations. - Check this. */ - gcc_assert (!NONJUMP_INSN_P (insn) || !pre - || TEST_BIT (antloc[bb->index], expr->bitmap_index) - || TEST_BIT (transp[bb->index], expr->bitmap_index)); /* If this is a jump table, then we can't insert stuff here. Since we know the previous real insn must be the tablejump, we insert @@ -3529,15 +3519,7 @@ insert_insn_end_basic_block (struct expr *expr, basic_block bb, int pre) /* Keeping in mind targets with small register classes and parameters in registers, we search backward and place the instructions before the first parameter is loaded. Do this for everyone for consistency - and a presumption that we'll get better code elsewhere as well. - - It should always be the case that we can put these instructions - anywhere in the basic block with performing PRE optimizations. - Check this. */ - - gcc_assert (!pre - || TEST_BIT (antloc[bb->index], expr->bitmap_index) - || TEST_BIT (transp[bb->index], expr->bitmap_index)); + and a presumption that we'll get better code elsewhere as well. */ /* Since different machines initialize their parameter registers in different orders, assume nothing. Collect the set of all @@ -3634,7 +3616,7 @@ pre_edge_insert (struct edge_list *edge_list, struct expr **index_map) now. */ if (eg->flags & EDGE_ABNORMAL) - insert_insn_end_basic_block (index_map[j], bb, 0); + insert_insn_end_basic_block (index_map[j], bb); else { insn = process_insert_insn (index_map[j]); @@ -4685,7 +4667,7 @@ hoist_code (void) if (!insn_inserted_p) { - insert_insn_end_basic_block (index_map[i], bb, 0); + insert_insn_end_basic_block (index_map[i], bb); insn_inserted_p = 1; } }