[Beignet] [PATCH] Fix a segmentation fault.
Zhigang Gong
zhigang.gong at linux.intel.com
Thu Apr 9 01:37:39 PDT 2015
The root cause is the temporary virtual register and GenRegister are not consistent to each other.
I just sent a new patch, yhe advantage of the new patch is, it could generate more efficient code
and use less registers.
With the old patch:
( 27) add(16) g118<1>:UD g8.2<0,1,0>:UD -g8.2<0,1,0>:UD { align1 WE_all 1H };
( 29) and(16) g116<1>:UD g118<8,8,1>:UD 0xfffffffcUD { align1 WE_all 1H Compacted };
( 30) send(16) g114<1>:UW g116<8,8,1>:UD
data (bti: 2, rgba: 14, SIMD16, legacy, Untyped Surface Read) mlen 2 rlen 2 { align1 WE_all 1H };
With the new patch:
( 27) add(1) g127.6<1>:UD g8.2<0,1,0>:UD -g8.2<0,1,0>:UD { align1 WE_all };
( 29) and(1) g127.5<1>:UD g127.6<0,1,0>:UD 0xfffffffcUD { align1 WE_all };
( 31) mov(16) g118<1>:UD g127.5<0,1,0>:UD { align1 WE_all 1H Compacted };
( 32) send(16) g116<1>:UW g118<8,8,1>:UD
data (bti: 2, rgba: 14, SIMD16, legacy, Untyped Surface Read) mlen 2 rlen 2 { align1 WE_all 1H };
Please check and review the new patch. Thanks.
On Thu, Apr 09, 2015 at 03:13:14AM +0000, Yang, Rong R wrote:
> 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
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list