This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH GCC]Vectorize possible infinite loops by versioning


On Wed, Jul 20, 2016 at 10:27 PM, Jeff Law <law@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.cheng@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.cheng@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
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]