[Patch] Splitting memory references during unrolling (resubmission)

[Steven Bosscher]

On Monday 09 August 2004 08:30, Revital Eres wrote:
> Hello,
>
> Following the fix for the unroller patch
> (http://gcc.gnu.org/ml/gcc-patches/2004-08/msg00397.html)
> I resubmit the patch for Splitting memory references during unrolling.
>
> Bootstrapped (with -funroll-all-loops) & regression tests on POWER4.
>
>
> 2004-08-09  Revital Eres  <eres@il.ibm.com>
>
>         * loop-unroll.c: (can_split_memory_ref): New function
>           for determine whether the current instruction
>           contains memory reference which uses an
>           induction variable.
>           (analyze_iv_to_split_insn): Expand the function to
>           analyze also instructions of memory references.

That would be:

	* loop-unroll.c: (can_split_memory_ref): New function
	for determine whether the current instruction
	contains memory reference which uses an
	induction variable.
	(analyze_iv_to_split_insn): Expand the function to
	analyze also instructions of memory references.

Note, different indentation

You should also send the patch as a context diff, cvs diff -c3p ...


> /* Determine whether INSN contains memory references
>    which uses an induction variable. */
>
> static struct iv_to_split *
> can_split_memory_ref (rtx insn)
> {

You make it sound like can_split_memory_ref is some sort of predicate
in the comment preceeding the function, but apparently it also returns
some relevant value.  Can you add to this comment a few more details
about what this function does and returns?

>   if ((REG_P (dest) || GET_MODE (dest) == SUBREG)
>       && GET_CODE (src) == MEM)

I believe you want GET_CODE here, not GET_MODE.  And when it is a
SUBREG, you probably want to make sure that SUBREG_REG is a REG.  And
you can use MEM_P now instead of GET_CODE (src) == MEM.
So the condition should be:

  if ((REG_P (dest)
       || (GET_MODE (dest) == SUBREG
	   && REG_P (SUBREG_REG (dest))))
      && MEM_P (src))

>   else if ((REG_P (src) || GET_MODE (src) == SUBREG)
> 	   && GET_CODE (dest) == MEM)
Same:

  if ((REG_P (src)
       || (GET_MODE (src) == SUBREG
	   && REG_P (SUBREG_REG (src))))
      && MEM_P (dest))


You also do some duplicate work here apparently:
>   if (iv_analyze (insn1, lop, &iv1)
>       && !iv1.first_special
>       && iv_analyze (insn2, rop, &iv2)
>       && !iv2.first_special
>       && iv1.step == const0_rtx
>       && iv2.mode == iv2.extend_mode)
>     {
>       if (iv2.step == NULL_RTX
(...)
>       return ivts;
>     }
>   /*  Memory reference is of the form:
>       induction variable + invariant */
>   if (iv_analyze (insn1, lop, &iv1)
>       && !iv1.first_special
>       && iv_analyze (insn2, rop, &iv2)
>       && !iv2.first_special
>       && iv2.step == const0_rtx
>       && iv1.mode == iv1.extend_mode)
>     {
>       if (iv1.step == NULL_RTX
(...)

As far as I can tell, those if-conditions are identical so you
don't have to evaluate it twice...


<  {
<    rtx set, dest;
<    struct rtx_iv iv;
<    struct iv_to_split *ivts;
<
<    /* For now we just split the basic induction variables.  Later this may be
<       extended for example by selecting also addresses of memory references.  */
<    set = single_set (insn);
<    if (!set)
<      return NULL;
(...)
---
> {
>   rtx set, dest;
>   struct rtx_iv iv;
>   struct iv_to_split *ivts;
>
>   set = single_set (insn);
>   if (!set)
>     return NULL;
>

Why did you remove the comment?
And why did you remove the abort in that hunk if iv_analyze fails?

Gr.
Steven