[polly] r246161 - Do not detect Scops with only one loop.

[Tobias Grosser via llvm-commits]

On 08/28/2015 09:10 AM, Johannes Doerfert wrote:
> On 08/28, Tobias Grosser wrote:
>> On 08/28/2015 08:53 AM, Johannes Doerfert wrote:
>>> On 08/28, Tobias Grosser via llvm-commits wrote:
>>>> On 08/27/2015 06:55 PM, Tobias Grosser via llvm-commits wrote:
>>>>> Author: grosser
>>>>> Date: Thu Aug 27 11:55:18 2015
>>>>> New Revision: 246161
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=246161&view=rev
>>>>> Log:
>>>>> Do not detect Scops with only one loop.
>>>>>
>>>>> If a region does not have more than one loop, we do not identify it as
>>>>> a Scop in ScopDetection. The main optimizations Polly is currently performing
>>>>> (tiling, preparation for outer-loop vectorization and loop fusion) are unlikely
>>>>> to have a positive impact on individual loops. In some cases, Polly's run-time
>>>>> alias checks or conditional hoisting may still have a positive impact, but those
>>>>> are mostly enabling transformations which LLVM already performs for individual
>>>>> loops. As we do not focus on individual loops, we leave them untouched to not
>>>>> introduce compile time regressions and execution time noise. This results in
>>>>> good compile time reduction (oourafft: -73.99%, smg2000: -56.25%).
>>>>>
>>>>> Contributed-by: Pratik Bhatu <cs12b1010 at iith.ac.in>
>>>>>
>>>>> Reviewers: grosser
>>>>>
>>>>> Differential Revision: http://reviews.llvm.org/D12268
>>>>
>>>> This change significantly improves compile-time performance on LNT without
>>>> causing any execution-time regressions:
>>>>
>>>> clang -O3 -polly - Before vs. After:
>>>> 	
>>>> http://llvm.org/perf/db_default/v4/nts/38436
>>>>
>>>> clang -O3 vs. clang -O3 -polly (before):
>>>>
>>>> http://llvm.org/perf/db_default/v4/nts/38423?compare_to=38469
>>>>
>>>> clang -O3 vs. clang -O3 -polly (after):
>>>>
>>>> http://llvm.org/perf/db_default/v4/nts/38436?compare_to=38469
>>>>
>>>> We see 78 compile time improvements with 14 of them showing more than 25%
>>>> percent compile time reduction. The overall benchmarks which show an
>>>> increase in compile time due to Polly are reduced from 140 to 111.
>>> While this sounds good, I think we do miss too many SCoPs with this
>>> patch. Especially ones that have two or more affine loops but the region
>>> entry is just not part of them.
>>
>> Yes, I also realized we block too many loops here. Polybench's 3mm kernel
>> is e.g. split into three kernels now. :(
>>
>>> To this end I think we need to change
>>> the way we count loops. My proposal:
>>>
>>>    Instead of a boolean Context.hasAffineLoop we keep a integer that we
>>>    increase for each affine loop. In the end we do not check if
>>>    hasAffineLoop is true or false but the integer is bigger than 1.
>>>    This will catch all affine loops in the region.
>>
>>> @Tobias, @Pratik:
>>>    Do you agree that we might miss SCoPs with multiple loops?
>>>    Do you think my proposal is OK or do you see any potential for
>>>    improvement?
>>
>>
>> Right. That would definitely work. However, we only have this number after
>> verifying that the full scop is indeed a scop. My hope would be that we
>> could bail out even before running scop-detection over the offending region.
>>
>> Maybe by iterating over the outermost loops and checking if more of them
>> are contained in the region?
> It's not as easy in general I think. Maybe building a mapping from
> regions to loops when we iterate over __all__ loops. That should work
> but outermost loops will probably not be precise.

What about the following:

Algorithm:

   loopNum = 0;

   entryNodeLoop = LI->getLoopFor(R->getEntry)

   if loop in R
     children = loop->getParent()->children
   else if loop not in R:
     children = loop->children
   else // no loop at all
     children = LI->getoutermostloops

   for child in children
     if (R->contains(child)) {
       loopNum++;
       if (child->subloops > 0)
         loopNum++;

       if (loopNum >= 2)
         return true;
     }
   return false;

Best,
Tobias