[PATCH, RFC] New target interface for vectorizer cost model

[William J. Schmidt]

On Tue, 2012-07-03 at 10:00 -0500, William J. Schmidt wrote:
> On Tue, 2012-07-03 at 15:59 +0200, Richard Guenther wrote:
> > On Tue, 3 Jul 2012, William J. Schmidt wrote:

<snip>

> > > > +@deftypefn {Target Hook} int TARGET_VECTORIZE_FINISH_COST (void *@var{})
> > > +This hook should complete calculations of the cost of vectorizing a loop 
> > > +or basic block, and return that cost as an integer.  It should also release
> > > +any target-specific data structures allocated by TARGET_VECTORIZE_INIT_COST.
> > > +The default returns the value of the accumulator and releases it.
> > > +@end deftypefn
> > 
> > Should return unsigned int I think.
> 
> So did I, until I started running into all the existing places where
> costs are signed for no good reason; it quickly became pretty bothersome
> to be casting back and forth.  I'll look at it again.  Probably all the
> mess will eventually go away if we can get rid of the existing cost
> fields.

This isn't as bad as I thought, fixing.
> 
> > 
> > > +@deftypefn {Target Hook} void TARGET_VECTORIZE_DESTROY_COST_DATA (void *@var{})
> > > +This hook should release any target-specific data structures allocated by
> > > +TARGET_VECTORIZE_INIT_COST.  The default releases the accumulator.
> > > +@end deftypefn
> > > +
> > 
> > Any reason this is not unified into one?  finish also destroys the data,
> > so are you merely saving time in the not vectorized case?
> 
> This interface is for all the exceptional exit paths where we need to
> free the memory but don't care to do the calculations associated with
> finish_cost, which may become more expensive over time.  For cleanliness
> of interface we could remove releasing the data from finish_cost's
> responsibilities.

Changing so finish_cost doesn't release the data.

<snip>

> > > +/* Opaque pointer to target-specific cost model data.  */
> > > +void *target_cost_data;
> > 
> > Put that into _loop_vec_info / _bb_vec_info?
> 
> I think it's problematic to do this.  If you look at record_stmt_cost,
> it needs to reference target_cost_data but currently has no way of
> knowing whether the vectorizer is processing a loop or a block.  So we'd
> need to pass the _{loop,bb}_vec_info around through a lot of interfaces
> (record_stmt_cost is called all over), or create some more global state.
> 
> The simplest thing would be for the target cost model to keep its state
> private so the rest of the vectorizer never sees target_cost_data.  This
> would give up on a certain level of "reentrancy" as we discussed, but
> I'm not convinced that's a practical issue.  Do we really need to allow
> the cost model to handle more than one loop and/or block at a time?  I'd
> rather just hide the state and simplify these issues.

Never mind all this.  I found since I had a stmt_info in
record_stmt_cost, this could be done without a lot of mess.  I'm working
on a revision that ties the lifetime of target_cost_data to that of the
_loop_vec_info or _bb_vec_info.  Much cleaner, thanks.

Thanks,
Bill