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
