On Tue, Jun 27, 2017 at 6:46 PM, Bin.Cheng <amker.ch...@gmail.com> wrote: > On Tue, Jun 27, 2017 at 3:59 PM, Richard Biener > <richard.guent...@gmail.com> wrote: >> On June 27, 2017 4:27:17 PM GMT+02:00, "Bin.Cheng" <amker.ch...@gmail.com> >> wrote: >>>On Tue, Jun 27, 2017 at 1:58 PM, Richard Biener >>><richard.guent...@gmail.com> wrote: >>>> On Fri, Jun 23, 2017 at 12:10 PM, Bin.Cheng <amker.ch...@gmail.com> >>>wrote: >>>>> On Mon, Jun 12, 2017 at 6:02 PM, Bin Cheng <bin.ch...@arm.com> >>>wrote: >>>>>> Hi, >>>>>> I was asked by upstream to split the loop distribution patch into >>>small ones. >>>>>> It is hard because data structure and algorithm are closely coupled >>>together. >>>>>> Anyway, this is the patch series with smaller patches. Basically I >>>tried to >>>>>> separate data structure and bug-fix changes apart with one as the >>>main patch. >>>>>> Note I only made necessary code refactoring in order to separate >>>patch, apart >>>>>> from that, there is no change against the last version. >>>>>> >>>>>> This is the first patch introducing new internal function >>>IFN_LOOP_DIST_ALIAS. >>>>>> GCC will distribute loops under condition of this function call. >>>>>> >>>>>> Bootstrap and test on x86_64 and AArch64. Is it OK? >>>>> Hi, >>>>> I need to update this patch fixing an issue in >>>>> vect_loop_dist_alias_call. The previous patch fails to find some >>>>> IFN_LOOP_DIST_ALIAS calls. >>>>> >>>>> Bootstrap and test in series. Is it OK? >>>> >>>> So I wonder if we really need to track ldist_alias_id or if we can do >>>sth >>>Yes, it is needed because otherwise we probably falsely trying to >>>search for IFN_LOOP_DIST_ALIAS for a normal (not from distribution) >>>loop. >>> >>>> more "general", like tracking a copy_of or origin and then directly >>>> go to nearest_common_dominator (loop->header, copy_of->header) >>>> to find the controlling condition? >>>I tend to not record any pointer in loop structure, it can easily go >>>dangling for a across passes data structure. >> >> I didn't mean to record a pointer, just rename your field and make it more >> general. The common dominator thing shod still work, no? > I might not be following. If we record the original loop->num in the > renamed field, nearest_common_dominator can't work because we don't > have basic blocks to start the call? The original loop could be > eliminated at several points, for example, instantly after > distribution, or folded in vectorizer for other loops distributed from > the original loop. > BTW, setting the copy_of/origin field in loop_version is not enough > for this use case, all generated loops (actually, except the versioned > loop) from distribution need to be set.
Of course it would need to be set for all distributed loops. I'm not sure "loop vanishes" is the case to optimize for though. If the loop is still available then origin->header should work as BB. If not then can't we conclude, for the purpose of IFN_LOOP_DIST_ALIAS, that the whole region must be dead? We still have to identify it of course, but it means we can fold stray IFN_LOOP_DIST_ALIAS calls left over after vectorization to whatever we like? >> >> As far as memory usage >>>is concerned. I actually don't need a whole integer to record the >>>loop num. I can simply restrict number of distributions in one >>>function to at most 256, and record such id in a char field in struct >>>loop? Does this sounds better? >> >> As said, tracking loop origin sounds useful anyway so I'd rather add and use >> that somehow. > To be honest, I don't know. the current field works like a unique > index of distribution operation. The original loop could be destroyed > at different points thus no longer exists, this makes the recorded > copy_of/origin less meaningful? I think we talked about prologue and epilogue loops to be easier identifiable as so (and as to what "main" loop). So lets say we have one "origin" field and accompaning flags "origin_is_loop_dist_alias_version", "origin_is_main_loop_of_prologue", etc.? I can't think of the case where origin would be two things at the same time (you can always walk up the origin tree). Vanishing origins could also be cleared btw, or we can make the new origin the origin of the vanishing origin... Richard. > Thanks, > bin >> >>>Thanks, >>>bin >>>> >>>> That said "ldist_alias_id" is a bit too narrow of purpose to "waste" >>>> an int inside struct loop? I'd set copy_of/origi in loop_version for >>>example. >>>> 'origin' would probably be better given the ldist cases aren't really >>>> full "copies". >>>> >>>> fold_loop_dist_alias_call should re-use / rename >>>fold_loop_vectorized_call >>>> by just passing folded_value to it. >>>> >>>> Richard. >>>> >>>>> Thanks, >>>>> bin >>>>>> >>>>>> Thanks, >>>>>> bin >>>>>> 2017-06-07 Bin Cheng <bin.ch...@arm.com> >>>>>> >>>>>> * cfgloop.h (struct loop): New field ldist_alias_id. >>>>>> * cfgloopmanip.c (lv_adjust_loop_entry_edge): Comment >>>change. >>>>>> * internal-fn.c (expand_LOOP_DIST_ALIAS): New function. >>>>>> * internal-fn.def (LOOP_DIST_ALIAS): New. >>>>>> * tree-vectorizer.c (vect_loop_dist_alias_call): New >>>function. >>>>>> (fold_loop_dist_alias_call): New function. >>>>>> (vectorize_loops): Fold IFN_LOOP_DIST_ALIAS call depending >>>on >>>>>> successful vectorization or not. >>
