This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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~


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]