[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