Re: [RFC] Variable Expansion Optimization

[Zdenek Dvorak <rakdver at atrey dot karlin dot mff dot cuni dot cz>]

Hello,

> Here is a diff of loop-unroll.c against msg00397.html.
> Hope it is helpful.

the patch seems reasonable (once it includes some check for register
pressure); some remarks:

< /* Information about variable to expand.  */
< struct var_to_expand
< {
<   rtx insn;		           /* The insn in that the variable expansion occurs.  */
<   rtx reg;                         /* The accumolator var which is expanded. */
                                                 ^ typo (repeated on other places as well)

<   varray_type var_expansions; /* The copies of the var was expanded.  */ 
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
				   This sentence does not make sense.

<   enum rtx_code op;                /* The type of the accumolation - addition, subsraction 
                                                             ^                       ^ typos
<                                       or multiplication.  */


<   htab_t insns_to_split;           /* A hashtable of insns to split.  */
<   htab_t insns_with_var_to_expand; /* A hashtable of insns to apply variable 
<                                       expansion.  */
<   unsigned first_new_block;        /* The first basic block that was
<                                       duplicated.  */
<   basic_block *loop_exit;          /* The loop exit basic block.  */
<   basic_block *loop_preheader;     /* The loop preheader basic block.  */
    ^^^^^^^^^^^^^  basic_block, not basic_block *. 

< static struct var_to_expand *analyze_insn_to_exapnd_var (struct loop*, rtx);
                                               ^^^^^^ typo

< referenced_in_one_insn_in_loop_p (struct loop *loop, rtx reg)

The way the function is used might cause quadratic behavior.  You might
consider counting the number of references for all registers at once instead.
You forget to free the loop body array at the end of function.

< analyze_insn_to_exapnd_var (struct loop *loop, rtx insn)
<   /* Record the insn to split.  */
                          ^^^^^ expand

<   if (flag_variable_expansion_in_unroller
<       && n_edges == 1
<       && fast_math_flags_set_p ())

You might want to use loop->single_exit instead of finding all the
exits (and if you from some reasons do not do it, at least free the
edges array).

<   if (!validate_change (insn, &SET_DEST (set), new_reg, 0))
<     return;
<   
<   if (!XEXP (SET_SRC (set), 0))
<     abort ();
<   
<   if (validate_change (insn, &XEXP (SET_SRC (set), 0), new_reg, 0))
<     return;
<   else
<     {
<       /* Restore the original state.  */
<       if (!validate_change (insn, &SET_DEST (set), ve->reg, 0))
<         abort ();
<     }

You should use validate_change with in_group=1 and apply_change_group.

< static int
< combine_var_copies_in_loop_exit (void **slot, void *place_p)

As a minor optimization, you might consider generating the combination
as a balanced binary tree instead of chain, which would be a bit better
for scheduling.

Zdenek