From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29815 invoked by alias); 11 Jun 2008 01:04:04 -0000 Received: (qmail 29799 invoked by uid 22791); 11 Jun 2008 01:04:03 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.33.17) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 11 Jun 2008 01:03:43 +0000 Received: from zps78.corp.google.com (zps78.corp.google.com [172.25.146.78]) by smtp-out.google.com with ESMTP id m5B13Wti025722; Wed, 11 Jun 2008 02:03:33 +0100 Received: from smtp.corp.google.com (spacemonkey1.corp.google.com [192.168.120.115]) by zps78.corp.google.com with ESMTP id m5B13UNK016679 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Tue, 10 Jun 2008 18:03:31 -0700 Received: from localhost.localdomain.google.com (69-36-227-135.cust.layer42.net [69.36.227.135] (may be forged)) (authenticated bits=0) by smtp.corp.google.com (8.13.8/8.13.8) with ESMTP id m5B13PIq015552 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Tue, 10 Jun 2008 18:03:28 -0700 To: Andrey Belevantsev Cc: GCC Patches , Jim Wilson , Vladimir Makarov Subject: Re: Selective scheduling pass - middle end changes [1/1] References: <4845522C.3010006@ispras.ru> <4845528D.6050302@ispras.ru> From: Ian Lance Taylor Date: Wed, 11 Jun 2008 01:04:00 -0000 In-Reply-To: <4845528D.6050302@ispras.ru> (Andrey Belevantsev's message of "Tue\, 03 Jun 2008 18\:17\:49 +0400") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: 2008-06/txt/msg00645.txt.bz2 Andrey Belevantsev writes: > 2008-06-03 Andrey Belevantsev > Dmitry Melnik > Dmitry Zhurikhin > Alexander Monakov > Maxim Kuvyrkov > > * cfghooks.h (get_cfg_hooks, set_cfg_hooks): New prototypes. > > * cfghooks.c (get_cfg_hooks, set_cfg_hooks): New functions. > (make_forwarder_block): Update loop latch if we have redirected > the loop latch edge. > > * cfgloop.c (get_loop_body_in_custom_order): New function. > > * cfgloop.h (LOOPS_HAVE_FALLTHRU_PREHEADERS): New enum field. > (CP_FALLTHRU_PREHEADERS): Likewise. > (get_loop_body_in_custom_order): Declare. > > * cfgloopmanip.c (has_preds_from_loop): New. > (create_preheader): Honor CP_FALLTHRU_PREHEADERS. > Assert that the preheader edge will be fall thru when it is set. > > * cse.c (hash_rtx_cb): New. > (hash_rtx): Use it. > > * emit-rtl.c (add_insn, add_insn_after, add_insn_before, > emit_insn_after_1): Call insn_added hook. > > * genattr.c (main): Output maximal_insn_latency prototype. > > * genautomata.c (output_default_latencies): New. Factor its code from ... > (output_internal_insn_latency_func): ... here. > (output_internal_maximal_insn_latency_func): New. > (output_maximal_insn_latency_func): New. > > * hard-reg-set.h (UHOST_BITS_PER_WIDE_INT): Define unconditionally. > (struct hard_reg_set_iterator): New. > (hard_reg_set_iter_init, hard_reg_set_iter_set, > hard_reg_set_iter_next): New functions. > (EXECUTE_IF_SET_IN_HARD_REG_SET): New macro. > > * lists.c (remove_free_INSN_LIST_node, > remove_free_EXPR_LIST_node): New functions. > > * loop-init.c (loop_optimizer_init): When LOOPS_HAVE_FALLTHRU_PREHEADERS, > set CP_FALLTHRU_PREHEADERS when calling create_preheaders. > (loop_optimizer_finalize): Do not verify flow info after reload. > > * passes.c (init_optimization_passes): Move pass_compute_alignments > after pass_machine_reorg. > > * recog.c (validate_replace_rtx_1): New parameter simplify. > Default it to true. Update all uses. Factor out simplifying > code to ... > (simplify_while_replacing): ... this new function. > (validate_replace_rtx_part, > validate_replace_rtx_part_nosimplify): New. > > * recog.h (validate_replace_rtx_part, > validate_replace_rtx_part_nosimplify): Declare. > > * rtl.c (rtx_equal_p_cb): New. > (rtx_equal_p): Use it. > > * rtl.h (rtx_equal_p_cb, hash_rtx_cb): Declare. > (remove_free_INSN_LIST_NODE, remove_free_EXPR_LIST_node, > debug_bb_n_slim, debug_bb_slim, print_rtl_slim, > sel_sched_fix_param, insn_added): Likewise. > > * rtlhooks-def.h (RTL_HOOKS_INSN_ADDED): Define to NULL. > Add to RTL_HOOKS_INITIALIZER. ! if (jump != NULL) ! { ! /* If we redirected the loop latch edge, the JUMP block now acts like ! the new latch of the loop. */ ! if (current_loops != NULL ! && dummy->loop_father->header == dummy ! && dummy->loop_father->latch == e_src) ! dummy->loop_father->latch = jump; I think you need to check that dummy->loop_father != NULL before you dereference it. && !((flags & CP_SIMPLE_PREHEADERS) ! && !single_succ_p (single_entry->src)) ! && !((flags & CP_FALLTHRU_PREHEADERS ! && (JUMP_P (BB_END (single_entry->src)) ! || has_preds_from_loop (single_entry->src, loop))))) This code needs a comment. Actually, I think it would be better to break up the complex condition into three simpler conditions, ideally ones which can be understood without applying DeMorgan's law. Also, you need to update the comment on the function as a whole. *************** hash_rtx (const_rtx x, enum machine_mode *** 2237,2243 **** x = XEXP (x, 0); goto repeat; ! case USE: /* A USE that mentions non-volatile memory needs special handling since the MEM may be BLKmode which normally prevents an entry from being made. Pure calls are --- 2241,2247 ---- x = XEXP (x, 0); goto repeat; ! case USE: /* A USE that mentions non-volatile memory needs special handling since the MEM may be BLKmode which normally prevents an entry from being made. Pure calls are A whitespace change in the wrong direction. Please don't apply this bit. *************** hash_rtx (const_rtx x, enum machine_mode *** 2330,2343 **** goto repeat; } ! hash += hash_rtx (XEXP (x, i), 0, do_not_record_p, ! hash_arg_in_memory_p, have_reg_qty); break; case 'E': for (j = 0; j < XVECLEN (x, i); j++) ! hash += hash_rtx (XVECEXP (x, i, j), 0, do_not_record_p, ! hash_arg_in_memory_p, have_reg_qty); break; case 's': --- 2339,2355 ---- goto repeat; } ! hash += hash_rtx_cb (XEXP (x, i), 0, do_not_record_p, ! hash_arg_in_memory_p, ! have_reg_qty, cb); break; case 'E': for (j = 0; j < XVECLEN (x, i); j++) ! hash += hash_rtx_cb (XVECEXP (x, i, j), 0, ! do_not_record_p, ! hash_arg_in_memory_p, ! have_reg_qty, cb); break; case 's': The indentation seems to have gone wrong here. *** trunk/gcc/genautomata.c Mon Sep 17 10:03:51 2007 --- sel-sched-branch/gcc/genautomata.c Mon Apr 14 17:13:39 2008 *************** output_min_insn_conflict_delay_func (voi *** 8067,8079 **** fprintf (output_file, "}\n\n"); } - /* Output function `internal_insn_latency'. */ static void ! output_internal_insn_latency_func (void) { - decl_t decl; - struct bypass_decl *bypass; int i, j, col; const char *tabletype = "unsigned char"; /* Find the smallest integer type that can hold all the default --- 8067,8077 ---- fprintf (output_file, "}\n\n"); } static void ! output_default_latencies (void) { int i, j, col; + decl_t decl; const char *tabletype = "unsigned char"; /* Find the smallest integer type that can hold all the default Don't remove the comment on the function, correct it. + if (iter->bits) + goto next_bit; This goto seems unnecessarily confusing. A simple "break" should work here. *** trunk/gcc/passes.c Fri May 30 17:32:06 2008 --- sel-sched-branch/gcc/passes.c Fri May 23 18:48:33 2008 *************** init_optimization_passes (void) *** 770,780 **** NEXT_PASS (pass_split_before_regstack); NEXT_PASS (pass_stack_regs_run); } - NEXT_PASS (pass_compute_alignments); NEXT_PASS (pass_duplicate_computed_gotos); NEXT_PASS (pass_variable_tracking); NEXT_PASS (pass_free_cfg); NEXT_PASS (pass_machine_reorg); NEXT_PASS (pass_cleanup_barriers); NEXT_PASS (pass_delay_slots); NEXT_PASS (pass_split_for_shorten_branches); --- 770,780 ---- NEXT_PASS (pass_split_before_regstack); NEXT_PASS (pass_stack_regs_run); } NEXT_PASS (pass_duplicate_computed_gotos); NEXT_PASS (pass_variable_tracking); NEXT_PASS (pass_free_cfg); NEXT_PASS (pass_machine_reorg); + NEXT_PASS (pass_compute_alignments); NEXT_PASS (pass_cleanup_barriers); NEXT_PASS (pass_delay_slots); NEXT_PASS (pass_split_for_shorten_branches); This looks wrong. I don't think you can call pass_compute_alignments after calling pass_free_cfg. *************** extern void set_curr_insn_source_locatio *** 2305,2308 **** --- 2327,2332 ---- extern void set_curr_insn_block (tree); extern int curr_insn_locator (void); + #define insn_added (rtl_hooks.insn_added) + #endif /* ! GCC_RTL_H */ We have a lot of #define's like this because nobody wanted to clean up the existing code. For new code, I don't think we need to add the #define's. Just use rtl_hooks.insn_added. That said, I'm not sure I like insn_added very much. It seems like a relatively fragile hook, as it will be hard to detect cases when it is used incorrectly. Can you expand on why this is needed? For building data structures, why does it not suffice to use get_max_uid? What sorts of insns do you expect to see created here? Thanks. Ian