On Fri, Jun 30, 2017 at 12:37 PM, Bin.Cheng <amker.ch...@gmail.com> wrote: > On Wed, Jun 28, 2017 at 8:29 AM, Richard Biener > <richard.guent...@gmail.com> wrote: >> 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). > Hi, > Here is the updated patch working in this way. There is still one > problem with this method. Considering one distributed loop is > if-converted later, the orig loop for if-converted distributed loop is > different. Though we can update orig_loop_num, it's inaccurate and > one consequence is we need to walk up dominance tree till entry_block. > Note if orig_loop_num is not shared, we can stop once basic block goes > beyond outer loop. > I didn't introduce flags in this patch, which can be done along with > solving the above problem. > >> >> Vanishing origins could also be cleared btw, or we can make the new origin >> the origin of the vanishing origin... > Yes, I did this in distribution patch, so that dominance walking > routine can be simplified. > > Bootstrap and test. Is it OK?
Ok. Thanks, Richard. > Thanks, > bin > > 2017-06-27 Bin Cheng <bin.ch...@arm.com> > > * cfgloop.h (struct loop): Add comment. New field orig_loop_num. > * 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 (fold_loop_vectorized_call): Rename to ... > (fold_loop_internal_call): ... this. > (vect_loop_dist_alias_call): New function. > (set_uid_loop_bbs): Call fold_loop_internal_call. > (vectorize_loops): Fold IFN_LOOP_VECTORIZED and IFN_LOOP_DIST_ALIAS > internal calls.
