On Fri, Dec 8, 2017 at 12:46 PM, Bin Cheng <bin.ch...@arm.com> wrote: > Hi, > This simple patch makes interchange even more conservative for small loops > with constant initialized simple reduction. > The reason is undoing such reduction introduces new data reference and > cond_expr, which could cost too much in a small > loop. > Test gcc.target/aarch64/pr62178.c is fixed with this patch. Is it OK if test > passes?
Shouldn't we do this even for non-constant initialzied simple reduction? Because for any simple reduction we add two DRs that are not innermost, for constant initialized we add an additional cond-expr. So ... + /* Conservatively skip interchange in cases only have few data references + and constant initialized simple reduction since it introduces new data + reference as well as ?: operation. */ + if (num_old_inv_drs + num_const_init_simple_reduc * 2 >= datarefs.length ()) + return false; + can you, instead of carrying num_const_init_simple_reduc simply loop over m_reductions and classify them in this function accordingly? I think we want to cost non-constant-init reductions as well. The :? can eventually count for another DR for cost purposes. It looks like we do count the existing DRs for the reduction? Is that why you arrive at the num_const_init_simple_reduc * 2 figure? (one extra load plus one ?:) But we don't really know whether the DR was invariant in the outer loop (well, I suppose we could remember the DR in m_reductions). Note that the good thing is that the ?: has an invariant condition and thus vectorization can hoist the mask generation out of the vectorized loop which means it boils down to cheap operations. My gut feeling is that just looking at the number of memory references isn't a good indicator of profitability as the regular stmt workload has a big impact on profitability of vectorization. So no ack nor nack... Richard. > Thanks, > bin > 2017-12-08 Bin Cheng <bin.ch...@arm.com> > > * gimple-loop-interchange.cc (struct loop_cand): New field. > (loop_cand::loop_cand): Init new field in constructor. > (loop_cand::classify_simple_reduction): Record simple reduction > initialized with constant value. > (should_interchange_loops): New parameter. Skip interchange if loop > has few data references and constant intitialized simple reduction. > (tree_loop_interchange::interchange): Update call to above function. > (should_interchange_loop_nest): Ditto.