[Beignet] [PATCH] GBE: We need use exiting block here.

[Zhigang Gong]

On Thu, Feb 12, 2015 at 01:21:37AM +0000, Song, Ruiling wrote:
> LLVM 3.6 requires the second parameter to getSmallConstantTripCount() need to be exiting block.
> See code below.
> unsigned ScalarEvolution::getSmallConstantTripCount(Loop *L,
>                                                     BasicBlock *ExitingBlock) {
>   assert(ExitingBlock && "Must pass a non-null exiting block!");
>   assert(L->isLoopExiting(ExitingBlock) &&
>          "Exiting block must actually branch out of the loop!");
> 
> I checked LoopUnrollPass in llvm. The logic below maybe better than my modification:
>   BasicBlock *ExitingBlock = L->getLoopLatch();
>   if (!ExitingBlock || !L->isLoopExiting(ExitingBlock))
>     ExitingBlock = L->getExitingBlock();
>   if (ExitingBlock) {
>     TripCount = SE->getSmallConstantTripCount(L, ExitingBlock);
>     TripMultiple = SE->getSmallConstantTripMultiple(L, ExitingBlock);
>   }
I agree the above logic seems better to me too.

Thanks,
Zhigang Gong.

> 
> > -----Original Message-----
> > From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com]
> > Sent: Wednesday, February 11, 2015 4:15 PM
> > To: Song, Ruiling
> > Cc: beignet at lists.freedesktop.org
> > Subject: Re: [Beignet] [PATCH] GBE: We need use exiting block here.
> > 
> > What the assertion does it hit with LLVM 3.6?
> > 
> > The original purpose to use latch block is:
> > 
> >   // Find "latch trip count". UnrollLoop assumes that control cannot exit
> >   // via the loop latch on any iteration prior to TripCount. The loop may exit
> >   // early via an earlier branch.
> > 
> > So if you use an exiting block here, you change the original logic, and as you
> > may know, one loop may have many exiting block, and some exiting block
> > may exit very early, so you may get a wrong currTripCount for the current
> > loop if you change to use an exiting block here.
> > 
> > On Wed, Feb 11, 2015 at 04:38:33PM +0800, Ruiling Song wrote:
> > > According to the API explanation, we should use exiting block instead
> > > of latch block. llvm 3.6 place an assert on this.
> > >
> > > Signed-off-by: Ruiling Song <ruiling.song at intel.com>
> > > ---
> > >  backend/src/llvm/llvm_unroll.cpp |   12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/backend/src/llvm/llvm_unroll.cpp
> > > b/backend/src/llvm/llvm_unroll.cpp
> > > index f0ad4b4..38bfa3e 100644
> > > --- a/backend/src/llvm/llvm_unroll.cpp
> > > +++ b/backend/src/llvm/llvm_unroll.cpp
> > > @@ -177,19 +177,19 @@ namespace gbe {
> > >        bool handleParentLoops(Loop *L, LPPassManager &LPM) {
> > >          Loop *currL = L;
> > >          ScalarEvolution *SE = &getAnalysis<ScalarEvolution>();
> > > -        BasicBlock *latchBlock = currL->getLoopLatch();
> > > +        BasicBlock *ExitBlock = currL->getExitingBlock();
> > >          unsigned currTripCount = 0;
> > >          bool shouldUnroll = true;
> > > -        if (latchBlock)
> > > -          currTripCount = SE->getSmallConstantTripCount(L,
> > latchBlock);
> > > +        if (ExitBlock)
> > > +          currTripCount = SE->getSmallConstantTripCount(L,
> > > + ExitBlock);
> > >
> > >          while(currL) {
> > >            Loop *parentL = currL->getParentLoop();
> > >            unsigned parentTripCount = 0;
> > >            if (parentL) {
> > > -            BasicBlock *parentLatchBlock = parentL->getLoopLatch();
> > > -            if (parentLatchBlock)
> > > -              parentTripCount =
> > SE->getSmallConstantTripCount(parentL, parentLatchBlock);
> > > +            BasicBlock *parentExitBlock = parentL->getExitingBlock();
> > > +            if (parentExitBlock)
> > > +              parentTripCount =
> > > + SE->getSmallConstantTripCount(parentL, parentExitBlock);
> > >            }
> > >            if ((parentTripCount != 0 && currTripCount /
> > parentTripCount > 16) ||
> > >                (currTripCount > 32)) {
> > > --
> > > 1.7.10.4
> > >
> > > _______________________________________________
> > > Beignet mailing list
> > > Beignet at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/beignet
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet