Remove redundant checks added by loop unrolling

[Philip Reames]

On 12/07/2014 07:57 PM, Benjamin Poulain wrote:
> I looked into this today.
>
> Following your advice, I made something local to loop unrolling (patch 
> attached). What this does is look if the previous block already checks 
> for a trip count of zero.
Given the canonicalization introduced by loop rotation, this might be 
sufficient for many interesting cases.  You're basically taking 
advantage of the fact you know this is being inserted in a loop preheader.
>
> This seems a bit hackish, it works for basic for() and while() loops. 
> Is there an analysis pass I could use to track properties of values? I 
> thought ScalarEvolution could help but it seems to only track that 
> value inside the loop.
There is LazyValueInfo which is used by JumpThreading, but I suspect 
that's too expensive for this case.  There's also isKnownNonNull in 
ValueTracking, but that's not path sensitive.  (It doesn't hurt to 
leverage that anyway.)

What you have seems like a reasonable starting point.  If you want to 
put it up on phabricator, I have a couple of small comments, but that's 
about it.  Opening a new thread is also a good way to get other eyes on 
the revised patch.

Structure wise, I think the following would be clearer for the actual test:
Value *LCmp = B.CreateIsNotNull(ModVal, "lcmp.mod");
Value *OverflowCheck = IsKnownNonZero(TripCount, PH) ? 
ConstantInt::getFalse(i1) : B.CreateIsNull(TripCount, "lcmp.overflow");
Value *BranchVal = B.CreateOr(OverflowCheck, LCmp, "lcmp.or");


>
> Benjamin
>
> On 12/2/14, 3:42 PM, Philip Reames wrote:
>> Benjamin,
>>
>> This doesn't really feel like the right approach to the problem.  
>> Adding another run of GVN after every unrolling opportunity is a 
>> fairly heavy weight hammer.  Have you looked to see if there's a 
>> simple change you could make to the unroller itself to handle this 
>> case?  I haven't looked at the code, but my suspicion is there would be.
>>
>> Philip
>>
>> On 11/29/2014 12:59 PM, Benjamin Poulain wrote:
>>> Hi,
>>>
>>> I noticed the optimized code generated for loops often include 
>>> redundant checks that the size is not zero.
>>>
>>> For example, code like this:
>>>   unsigned total = 0;
>>>   for (unsigned i = 0; i < size; ++i) { ...
>>>
>>> when optimized, generate the following prologue:
>>>   entry:
>>>     %cmp4 = icmp eq i32 %size, 0
>>>     br i1 %cmp4, label %for.end, label %for.body.lr.ph
>>>
>>>   for.body.lr.ph:                                   ; preds = %entry
>>>    %0 = add i32 %size, -1
>>>     %xtraiter = and i32 %size, 3
>>>     %lcmp.mod = icmp ne i32 %xtraiter, 0
>>>     %lcmp.overflow = icmp eq i32 %size, 0
>>>     %lcmp.or = or i1 %lcmp.overflow, %lcmp.mod
>>>     br i1 %lcmp.or, label %for.body.prol, label %for.body.lr.ph.split
>>>
>>> Notice the redundant test for "icmp eq i32 %size, 0". When compiled 
>>> to target, we have one redundant check per loop that can never be true.
>>>
>>> The extra check for size==0 comes from LoopUnroll. It is never 
>>> optimized out because LoopUnrollPass is run after the passes that 
>>> could eliminate redundant conditions.
>>>
>>> I have attached a patch that fixes the problem. For every pass of 
>>> LoopUnroll, I added a GVN pass to remove every redundant conditions 
>>> and branches. My patch is without test, I need guidance on how to 
>>> properly test this.
>>>
>>> Benjamin
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141215/90beffa4/attachment.html>