This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Array prefetch patch take 3
> On Tue, Dec 11, 2001 at 07:19:46PM +0100, Jan Hubicka wrote:
> > + /* We don't want to return 1 if X is a MEM that contains a register
> > + within REG_SET_REG. */
> > +
> > +
> > + if ((GET_CODE (x) == MEM) && rtx_equal_p (d->mem_address, XEXP (x, 0)))
> > + d->mem_write = 1;
>
> The comment doesn't match the code. Cut and paste-o?
Yes.
>
> > + /* Like rtx_equal_p, but attempts to swap commutative operands. This is
> > + important to get some addresses combined. Later more sophisticated
> > + transformations can be added when necesary.
>
> Is this _really_ needed? I'd hope that commutative operands were
> canonicalized. If the problem case is merely reg+reg, perhaps we
> can instead extend swap_commutative_operands_p to look at REGNO
> as well.
It is a while since I was working on the patch, but I remember I had specific
testcase in my hands before writting the function. It is because the expressions
are not stricly canonized, since they come from BIV/GIV simplification code.
It was the reg+reg problem. If we are thinking about this patch for 3.1, I
would preffer this sollution over swap_commutative_operands_p change. It took
me over week to stabilize the tree after swap_commutative_operands_p patch, as
some targets depends on operands to not swap for insns to match and virtual
register ellimination relies on validate_replace_rtx to work.
I will try to investigate the issue futher on the cfg-branch and later send for
mainline. Sounds sane?
>
> > + static int
> > + remove_constant_addition (x)
>
> Must be HOST_WIDE_INT to avoid truncating the value.
Good catch - I've replaced some other places too.
>
> > + "Prefetch: ignoring loop - not enought iterations.\n");
>
> typo.
>
> > + /* Attempt to calculate the number of bytes fetched by the loop. */
> > + if (LOOP_INFO (loop)->n_iterations
> > + && LOOP_INFO (loop)->n_iterations < 200)
> > + info[i].total_bytes = info[i].stride * LOOP_INFO (loop)->n_iterations;
>
> Why the 200? Yes, I can see that you'd like to not overflow, but
> perhaps
>
> if (LOOP_INFO (loop)->n_iterations
> && (0xffffffff / info[i].stride) >= LOOP_INFO (loop)->n_iterations)
OK.
We later do something special only when total_bytes is "small", but your test appear
cleaner.
I am testing updated patch I will send shortly.
Honza
>
>
>
> r~