On Wed, Jul 20, 2016 at 10:27 PM, Jeff Law <l...@redhat.com> wrote: > On 06/28/2016 12:18 AM, Bin Cheng wrote: >> >> Hi, >> This patch improves vectorizer in order to handle possible infinite loops >> by versioning. Its changes fall in three categories. >> A) Changes in vect_get_loop_niters. AT the moment, it computes niter >> using number_of_executions_latch, in this way the assumption is discarded >> and loop not vectorized. To fix the issue, we need assumption information >> from niter analyzer and use that as a break condition to version the loop. >> This patch uses newly introduced interface >> number_of_iterations_exit_assumptions and passes assumptions all the way to >> vect_analyze_loop_form. The assumptions will be finally recorded in >> LOOP_VINFO_NITERS_ASSUMPTIONS. >> B) It sets and clears flag LOOP_F_ASSUMPTIONS for loop. The flag is >> important because during checking if a loop can be vectorized (with >> versioning), all data references need to be analyzed by assuming >> LOOP_VINFO_NITERS_ASSUMPTIONS is TRUE. Otherwise it's very likely address >> expression of data reference won't be identified as SCEV and vectorization >> would fail. With this flag set to TRUE, niter analyzer will bypass >> assumptions recorded LOOP_VINFO_NITERS_ASSUMPTIONS. I also keep this flag >> for versioned loop because the assumption is guaranteed to be TRUE after >> versioning. For now, I didn't copy these flags in copy_loop_info, but I >> think this can be done so that the flags can be inherited by peeled pre/post >> loop. Maybe in follow up patches. Also it's possible to turn other bool >> fields into flags in the future? >> C) This patch uses existing infrastructure to version a loop against >> LOOP_VINFO_NITERS_ASSUMPTIONS, just like for alignment or alias check. The >> change is straightforward, however, I did refactoring to versioning related >> macros hoping the code would be cleaner. >> >> Bootstrap and test along with previous niter patches on x86_64 and >> AArch64. Is it OK? > Hi Jeff, Thanks for looking at this.
> So I have one high level concern -- how (if at all) does this interact with > Ilya's changes to vectorize loop tails that are just about through the > review process? Though I haven't look into details of tail-vect patch set, I would not expect interference between them. Versioning in this patch is before tail-vect exactly like existing versioning for alias/alignment. > > Related -- I see that you throw away the SCEV/iteration knowledge then > analyze the loop using the given assumptions, then eventually throw that > information away. Which sounds generally reasonable -- except for one > potential issue -- does anything still want to look at the original > SCEV/iteration information (that we've lost)? I'm assuming no since you > didn't try to restore it and we pass the testsuite with your change. The niter/scev information discarded are buffered data. The only impact is on compile time, that is, scev/niter information will be computed from scratch rather than queried from buffered (hash) data-structure, if following customer need these information. For successful versioning, I think this is kind of inevitable because scev/niter information of versioned loop IS different to the original loop. > > >> >> Thanks, >> bin >> >> 2016-06-27 Bin Cheng <bin.ch...@arm.com> >> >> PR tree-optimization/57558 >> * tree-vect-loop-manip.c (vect_create_cond_for_niters_checks): New >> function. >> (vect_loop_versioning): Support versioning with niter assumptions. >> * tree-vect-loop.c (tree-ssa-loop.h): Include new header file. >> (vect_get_loop_niters): New parameter. Reimplement to support >> assumptions in loop niter info. >> (vect_analyze_loop_form_1, vect_analyze_loop_form): Ditto. >> (new_loop_vec_info): Init LOOP_VINFO_NITERS_ASSUMPTIONS. >> (vect_estimate_min_profitable_iters): Use >> LOOP_REQUIRES_VERSIONING. >> * tree-vectorizer.c (vect_free_loop_info_assumptions): New >> function. >> (vectorize_loops): Free loop niter info for loops with flag >> LOOP_F_ASSUMPTIONS set. >> * tree-vectorizer.h (struct _loorefactoredp_vec_info): New field > > Typo in the ChangeLog entry. At this moment, prerequisite patch set for this one is ongoing review/rework, I will update this patch after finishing prerequisites. > >> num_iters_assumptions. >> (LOOP_VINFO_NITERS_ASSUMPTIONS): New macro. >> (LOOP_REQUIRES_VERSIONING_FOR_NITERS): New macro. >> (LOOP_REQUIRES_VERSIONING): New macro. >> (vect_free_loop_info_assumptions): New decl. >> >> gcc/testsuite/ChangeLog >> 2016-06-27 Bin Cheng <bin.ch...@arm.com> >> >> * gcc.dg/vect/pr57558-1.c: New test. >> * gcc.dg/vect/pr57558-2.c: New test. > > I was rather surprised at how simple supporting this case was. THe two high > level questions above are the only things I'm worried about. Let's figure > out the answers to those questions before we OK this for the trunk. Yeah, it reuses existing versioning code in vectorizer. Major part of work is in niter/scev to support analyzing loops whose counter may overflow/wrap. Thanks, bin > Jeff >
