[Beignet] [PATCH] Fix a segmentation fault.

Zhigang Gong zhigang.gong at linux.intel.com
Tue Apr 7 22:46:30 PDT 2015


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