Remove redundant checks added by loop unrolling

[Hal Finkel]

----- Original Message -----
> From: "Philip Reames" <listmail at philipreames.com>
> To: "Benjamin Poulain" <benjamin at webkit.org>, llvm-commits at cs.uiuc.edu
> Sent: Monday, December 15, 2014 7:38:41 PM
> Subject: Re: Remove redundant checks added by loop unrolling
> 
> 
> 
> 
> 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.

Hi Benjamin,

Maybe I'm not reading this correctly, but couldn't your IsKnownNonZero function be replaced by a call to ScalarEvolutions's isLoopEntryGuardedByCond function?

 -Hal

> 
> 
> 
> 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
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory