[Beignet] [PATCH] GBE: fix a zero/one's liveness bug.

[Yang, Rong R]

LGTM, pushed, thanks.

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Zhigang Gong
> Sent: Friday, October 9, 2015 9:06
> To: Gong, Zhigang
> Cc: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] GBE: fix a zero/one's liveness bug.
> 
> Ping for review.
> 
> On Tue, Sep 22, 2015 at 03:45:29PM +0800, Zhigang Gong wrote:
> > Ping for review.
> > Thanks.
> >
> > On Mon, Sep 14, 2015 at 03:50:00PM +0800, Zhigang Gong wrote:
> > > This is a long standing bug, and is exposed by my latest register
> > > allocation refinement patchset. ir::ocl::zero and ir::ocl::one are
> > > global registers, we have to compute its liveness information
> > > carefully, not just get a local interval ID.
> > >
> > > Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
> > > ---
> > >  backend/src/backend/gen_reg_allocation.cpp | 29
> > > +++++++++++++++++++++++++++++
> > >  1 file changed, 29 insertions(+)
> > >
> > > diff --git a/backend/src/backend/gen_reg_allocation.cpp
> > > b/backend/src/backend/gen_reg_allocation.cpp
> > > index bf2ac2b..f440747 100644
> > > --- a/backend/src/backend/gen_reg_allocation.cpp
> > > +++ b/backend/src/backend/gen_reg_allocation.cpp
> > > @@ -179,6 +179,8 @@ namespace gbe
> > >      SpilledRegs spilledRegs;
> > >      /*! register which could be spilled.*/
> > >      SpillCandidateSet spillCandidate;
> > > +    /*! BBs last instruction ID map */
> > > +    map<const ir::BasicBlock *, int32_t> bbLastInsnIDMap;
> > >      /* reserved registers for register spill/reload */
> > >      uint32_t reservedReg;
> > >      /*! Current vector to expire */ @@ -505,6 +507,7 @@ namespace
> > > gbe
> > >      // policy is to spill the allocate flag which live to the last time end point.
> > >
> > >      // we have three flags we use for booleans f0.0 , f1.0 and f1.1
> > > +    set<const ir::BasicBlock *> liveInSet01;
> > >      for (auto &block : *selection.blockList) {
> > >        // Store the registers allocated in the map
> > >        map<ir::Register, uint32_t> allocatedFlags; @@ -674,6 +677,7
> > > @@ namespace gbe
> > >              sel0->src(0) = GenRegister::uw1grf(ir::ocl::one);
> > >              sel0->src(1) = GenRegister::uw1grf(ir::ocl::zero);
> > >              sel0->dst(0) = GET_FLAG_REG(insn);
> > > +            liveInSet01.insert(insn.parent->bb);
> > >              insn.append(*sel0);
> > >              // We use the zero one after the liveness analysis, we have to
> update
> > >              // the liveness data manually here.
> > > @@ -692,6 +696,30 @@ namespace gbe
> > >          }
> > >        }
> > >      }
> > > +
> > > +    // As we introduce two global variables zero and one, we have to
> > > +    // recompute its liveness information here!
> > > +    if (liveInSet01.size()) {
> > > +      set<const ir::BasicBlock *> liveOutSet01;
> > > +      set<const ir::BasicBlock *> workSet(liveInSet01.begin(),
> liveInSet01.end());
> > > +      while(workSet.size()) {
> > > +        for(auto bb : workSet) {
> > > +          for(auto predBB : bb->getPredecessorSet()) {
> > > +            liveOutSet01.insert(predBB);
> > > +            if (liveInSet01.contains(predBB))
> > > +              continue;
> > > +            liveInSet01.insert(predBB);
> > > +            workSet.insert(predBB);
> > > +          }
> > > +          workSet.erase(bb);
> > > +        }
> > > +      }
> > > +      int32_t maxID = 0;
> > > +      for(auto bb : liveOutSet01)
> > > +        maxID = std::max(maxID, bbLastInsnIDMap.find(bb)->second);
> > > +      intervals[ir::ocl::zero].maxID = std::max(intervals[ir::ocl::zero].maxID,
> maxID);
> > > +      intervals[ir::ocl::one].maxID = std::max(intervals[ir::ocl::one].maxID,
> maxID);
> > > +    }
> > >    }
> > >
> > >    IVAR(OCL_SIMD16_SPILL_THRESHOLD, 0, 16, 256); @@ -1127,6 +1155,7
> > > @@ namespace gbe
> > >
> > >        // All registers alive at the begining of the block must update their
> intervals.
> > >        const ir::BasicBlock *bb = block.bb;
> > > +      bbLastInsnIDMap.insert(std::make_pair(bb, lastID));
> > >        for (auto reg : ctx.getLiveIn(bb))
> > >          this->intervals[reg].minID =
> > > std::min(this->intervals[reg].minID, firstID);
> > >
> > > --
> > > 1.9.1
> > >
> > > _______________________________________________
> > > 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
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet