Re: Selective scheduling pass - middle end changes [1/1]

[Ian Lance Taylor <iant@google.com>]

Andrey Belevantsev <abel@ispras.ru> writes:

> 2008-06-03  Andrey Belevantsev  <abel@ispras.ru>
> 	    Dmitry Melnik  <dm@ispras.ru>
> 	    Dmitry Zhurikhin  <zhur@ispras.ru>
> 	    Alexander Monakov  <amonakov@ispras.ru>
> 	    Maxim Kuvyrkov  <maxim@codesourcery.com>
> 	
> 	* 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