[Beignet] [PATCH] Fix a segmentation fault.

Yang, Rong R rong.r.yang at intel.com
Wed Apr 8 20:13:14 PDT 2015


Some of these register is request as scalar ir::register and retype to uniform GenRegister.
These temp registers are all used as send's address or other payload, the send payload need continuous register, so I change  these registers to scalar register.

> -----Original Message-----
> From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com]
> Sent: Wednesday, April 8, 2015 13:47
> To: Yang, Rong R
> Cc: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] Fix a segmentation fault.
> 
> On Tue, Mar 31, 2015 at 04:39:03PM +0800, Yang Rong wrote:
> > There is a segmentation fault in function isSrcDstDiffSpan, when src's
> > hstrde is not GEN_HORIZONTAL_STRIDE_0 but dst's hstride is
> GEN_HORIZONTAL_STRIDE_0.
> >
> > This is wrong state, and the LoadInstruction using GenRegister::udxgrf
> > with simd is 1, will introduce this state, when dst is scalar. Use sel.selReg
> instead of GenRegister::udxgrf.
> 
> Nice catch, but the patch will always use non-uniform temporary register.
> Could you refine it to only use non-uniform registers on this segfault case.
> For other normal cases, they can still use simd1 mode which has better
> performance.
> 
> Thanks,
> Zhigang Gong.
> 
> >
> > Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> > ---
> >  backend/src/backend/gen_insn_selection.cpp | 18 ++++++++----------
> >  1 file changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/backend/src/backend/gen_insn_selection.cpp
> > b/backend/src/backend/gen_insn_selection.cpp
> > index 7f9c95a..058d22b 100644
> > --- a/backend/src/backend/gen_insn_selection.cpp
> > +++ b/backend/src/backend/gen_insn_selection.cpp
> > @@ -3069,7 +3069,7 @@ namespace gbe
> >
> >        GenRegister dst = GenRegister::retype(sel.selReg(insn.getValue(0)),
> GEN_TYPE_F);
> >        // get dword based address
> > -      GenRegister addrDW = GenRegister::udxgrf(simdWidth,
> sel.reg(FAMILY_DWORD));
> > +      GenRegister addrDW = sel.selReg(sel.reg(FAMILY_DWORD),
> > + ir::TYPE_U32);
> >
> >        sel.push();
> >          if (sel.isScalarReg(addr.reg())) { @@ -3116,9 +3116,9 @@
> > namespace gbe
> >                          uint8_t bti) const
> >      {
> >        using namespace ir;
> > -        Register tmpReg = sel.reg(FAMILY_DWORD, simdWidth == 1);
> > -        GenRegister tmpAddr = GenRegister::udxgrf(simdWidth,
> sel.reg(FAMILY_DWORD));
> > -        GenRegister tmpData = GenRegister::udxgrf(simdWidth, tmpReg);
> > +        Register tmpReg = sel.reg(FAMILY_DWORD);
> > +        GenRegister tmpAddr = sel.selReg(sel.reg(FAMILY_DWORD),
> ir::TYPE_U32);
> > +        GenRegister tmpData = sel.selReg(tmpReg, ir::TYPE_U32);
> >          // Get dword aligned addr
> >          sel.push();
> >            if (simdWidth == 1) {
> > @@ -3154,8 +3154,6 @@ namespace gbe
> >      {
> >        using namespace ir;
> >        const uint32_t valueNum = insn.getValueNum();
> > -      const uint32_t simdWidth = sel.isScalarReg(insn.getValue(0)) ?
> > -                                 1 : sel.ctx.getSimdWidth();
> >        RegisterFamily family = getFamily(insn.getValueType());
> >
> >        vector<GenRegister> dst(valueNum); @@ -3170,7 +3168,7 @@
> > namespace gbe
> >        vector<Register> tmpReg(tmpRegNum);
> >        for(uint32_t i = 0; i < tmpRegNum; i++) {
> >          tmpReg[i] = sel.reg(FAMILY_DWORD);
> > -        tmp2[i] = tmp[i] = GenRegister::udxgrf(simdWidth, tmpReg[i]);
> > +        tmp2[i] = tmp[i] = sel.selReg(tmpReg[i], ir::TYPE_U32);
> >        }
> >
> >        readDWord(sel, tmp, tmp2, address, tmpRegNum, bti); @@ -3254,9
> > +3252,9 @@ namespace gbe
> >          vector<GenRegister> tmp2(effectDataNum + 1);
> >          vector<GenRegister> effectData(effectDataNum);
> >          for(uint32_t i = 0; i < effectDataNum + 1; i++)
> > -          tmp2[i] = tmp[i] = GenRegister::udxgrf(simdWidth,
> sel.reg(FAMILY_DWORD));
> > +          tmp2[i] = tmp[i] = sel.selReg(sel.reg(FAMILY_DWORD),
> > + ir::TYPE_U32);
> >
> > -        GenRegister alignedAddr = GenRegister::udxgrf(simdWidth,
> sel.reg(FAMILY_DWORD));
> > +        GenRegister alignedAddr = sel.selReg(sel.reg(FAMILY_DWORD),
> > + ir::TYPE_U32);
> >          sel.push();
> >            if (simdWidth == 1)
> >              sel.curr.noMask = 1;
> > @@ -3465,7 +3463,7 @@ namespace gbe
> >        } else {
> >          const GenRegister value = sel.selReg(insn.getValue(0));
> >          GBE_ASSERT(insn.getValueNum() == 1);
> > -        const GenRegister tmp = GenRegister::udxgrf(simdWidth,
> sel.reg(FAMILY_DWORD));
> > +        const GenRegister tmp = sel.selReg(sel.reg(FAMILY_DWORD),
> > + ir::TYPE_U32);
> >          if (elemSize == GEN_BYTE_SCATTER_WORD) {
> >            sel.MOV(tmp, GenRegister::retype(value, GEN_TYPE_UW));
> >          } else if (elemSize == GEN_BYTE_SCATTER_BYTE) {
> > --
> > 1.8.3.2
> >
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list